Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow any mapping after ** in calls #44767

Closed
abalkin opened this issue Mar 23, 2007 · 11 comments
Closed

Allow any mapping after ** in calls #44767

abalkin opened this issue Mar 23, 2007 · 11 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@abalkin
Copy link
Member

abalkin commented Mar 23, 2007

BPO 1686487
Nosy @birkenfeld, @abalkin
Files
  • star-star-mapping.patch: svn diff against revision 54869
  • star-star-mapping-msg.patch: diff against revision 54926
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2007-05-21.20:34:31.000>
created_at = <Date 2007-03-23.03:27:50.000>
labels = ['interpreter-core']
title = 'Allow any mapping after ** in calls'
updated_at = <Date 2007-05-21.20:34:31.000>
user = 'https://github.com/abalkin'

    bugs.python.org fields:

    activity = <Date 2007-05-21.20:34:31.000>
actor = 'georg.brandl'
assignee = 'none'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2007-03-23.03:27:50.000>
creator = 'belopolsky'
dependencies = []
files = ['7900', '7901']
hgrepos = []
issue_num = 1686487
keywords = ['patch']
message_count = 11.0
messages = ['52300', '52301', '52302', '52303', '52304', '52305', '52306', '52307', '52308', '52309', '52310']
nosy_count = 3.0
nosy_names = ['georg.brandl', 'belopolsky', 'zseil']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1686487'
versions = ['Python 2.6']
    @abalkin
    Copy link
    Member Author

    abalkin commented Mar 23, 2007

    Python allows arbitrary sequences after * in calls, but an expression following ** must be a (subclass of) dict. The attached patch makes Python accept arbitrary mappings after **.

    @abalkin abalkin closed this as completed Mar 23, 2007
    @abalkin abalkin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 23, 2007
    @abalkin abalkin closed this as completed Mar 23, 2007
    @abalkin abalkin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 23, 2007
    @birkenfeld
    Copy link
    Member

    Two flaws:

    • what if kwdict is NULL? Shouldn't happen, but the old version raises an error in this case.
    • if PyDict_Update fails, tmpdict is leaked.

    Please ask on python-dev whether to include this feature.

    @abalkin
    Copy link
    Member Author

    abalkin commented Mar 23, 2007

    I've fixed the flaws in star-star-mapping-1.patch . I will ask on python-dev about the feature. If the proposal is accepted, I will probably refactor the code to avoid using the second goto label.
    File Added: star-star-mapping-1.patch

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 18, 2007

    I have asked on python-dev almost a month ago, but did not get any response:

    http://mail.python.org/pipermail/python-dev/2007-March/072321.html

    I have recently posted another message that got no response as well:

    http://mail.python.org/pipermail/python-dev/2007-April/072653.html

    Is it possible that my messages are not being delivered to the list?

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 18, 2007

    Georg,

    Thanks for reposting my proposal. Since BDFL is +1 on the idea, I feel motivated to polish the patch. In your early comment you noted that kwdict == NULL check may be superficial. I agree. It does not look like it is possible that ext_do_call is called with CALL_FLAG_KW set and a null pointer at the top of the stack. It looks like this may only happen if the stack is corrupted by a misbehaving c extension. In this case, however, throwing a type error is too gentle. PyErr_BadInternalCall() seems to be more appropriate if anything at all.

    I propose to eliminate the null check for kwdict alltogether. This would be consistent with the way stararg is handled later in this function. I am going to run the testsuit without the null check and will post a revised patch shortly.

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 18, 2007

    The proposed patch handles ** arguments similar to the way * arguments are treated. If the expression following ** evaluates to an object that is not a dictionary, the new code attempts to update a new dictionary with that object. This procedure is closely similar to what PySequence_Tuple does with * arguments. (Python API does not provide PyMapping_Dict.) If new dictionary is succesfully created and updated, it replaces the original ** argument. If that attempt fails because kwdict is not a mapping (does not have a 'keys' attribute, a type error is raised explaining that a mapping is expected. All other errors, e.g. MemoryError are percolated up.

    (I am am removing the previous versions of the patch.)
    File Added: star-star-mapping.patch

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 18, 2007

    File Added: star-star-mapping.patch

    @zseil
    Copy link
    Mannequin

    zseil mannequin commented Apr 24, 2007

    The patch looks good to me. The only thing that could
    be improved is the error message. It would be nice if
    it contained the information about kwdict's type.

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 24, 2007

    zseil: It would be nice if it contained the information about kwdict's type.

    I agree, the new patch adds type information to both * and ** error messages.
    File Added: star-star-mapping-msg.patch

    @abalkin
    Copy link
    Member Author

    abalkin commented May 20, 2007

    What else needs to be done for the patch to go in? Guido OKed the feature and zseil gave a favorable code review. Is there any reason not to apply it?

    @birkenfeld
    Copy link
    Member

    I think it just needs someone to commit it. ;) See revision 55495.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @birkenfeld @abalkin