Introduce ArgOrKeyword to keep call parameter order#7302
Merged
Conversation
Member
Author
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
MichaReiser
reviewed
Sep 12, 2023
| } | ||
| for keyword in &arguments.keywords { | ||
| visitor.visit_keyword(keyword); | ||
| for arg_or_keyword in arguments.args_and_keywords() { |
Member
There was a problem hiding this comment.
The ordering in this visitor should be the same as the execution order at runtime.
Member
Author
There was a problem hiding this comment.
Input:
def f(*args, **kwargs):
pass
def g(x):
print(x)
return x
f(*g([1]), a=g(2), *g([3]), **g({"4": 5}))Output:
[1]
[3]
2
{'4': 5}😵💫
Member
Author
There was a problem hiding this comment.
thanks for making me check
Contributor
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
454aa03 to
f4037d9
Compare
MichaReiser
approved these changes
Sep 12, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The
ast::Argumentsfor call argument are split into positional arguments (args) and keywords arguments (keywords). We currently assume that call consists of first args and then keywords, which is generally the case, but not always:The consequence is accidentally reordering arguments (#7268).
Summary
Arguments::args_and_keywordsreturns an iterator of anArgOrKeywordenum that yields args and keywords in the correct order. I've fixed the obviousargsandkeywordsusages, but there might be some cases with wrong assumptions remaining.Test Plan
The generator got new test cases, otherwise the stacked PR (#7268) which uncovered this.