Skip to content

Separate positional and keyword arguments in CallExpression#1167

Merged
slozier merged 1 commit intoIronLanguages:masterfrom
BCSharp:args_kwargs_split
Apr 11, 2021
Merged

Separate positional and keyword arguments in CallExpression#1167
slozier merged 1 commit intoIronLanguages:masterfrom
BCSharp:args_kwargs_split

Conversation

@BCSharp
Copy link
Copy Markdown
Member

@BCSharp BCSharp commented Apr 11, 2021

Code refactoring of CallExpression.

It came up when I was looking into keyword arguments for metaclasses (#1163), which most likely will have an impact on CallExpression as it is now. So before starting with metaclasses, I wanted to get this one out of the way, since it brings a public interface change.

Copy link
Copy Markdown
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I thought about doing this but was too lazy to go through with it. 😄

Not specifically an issue with this PR, but one thing that was bugging me while I was working on this code was the Arg construct. It doesn't seem particularly useful, especially now that we have split arguments. For example args could just be a list of expressions and we look for StarredExpression. Also GetArgumentInfo is terrible since it creates an Argument whenever call it on an argument that's not simple (and most of the time we're calling it just to check the Kind). Anyway, enough of my rant. 😺

@slozier slozier merged commit 1e9cffd into IronLanguages:master Apr 11, 2021
@BCSharp
Copy link
Copy Markdown
Member Author

BCSharp commented Apr 12, 2021

Thanks for the rant! Now the Arg construct has started to bug me too... 😄

@BCSharp BCSharp deleted the args_kwargs_split branch April 12, 2021 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants