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

Refactor rules for class_def and arguments #135

Merged
merged 4 commits into from Jan 6, 2020

Conversation

lysnikolaou
Copy link
Collaborator

Closes #122.

@lysnikolaou lysnikolaou changed the title WIP: Refactor rules for class_def and arguments rules WIP: Refactor rules for class_def and arguments Jan 5, 2020
'''
class C(A, x=2, *[B, C], y=3):
pass
'''),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we are missing a case with *args and **kwargs at the same time, which is valid in current Python:

>>> class A(x,y,*something,**more_something):   pass

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh yes, thats right! Another problem is that this currently allows generator expressions in the arguments, which is not valid Python, if I'm not mistaken.

Edit: Never mind my comment about genexprs, I had already fixed that, but forgot about it. Also, I added one more test case.

@lysnikolaou lysnikolaou changed the title WIP: Refactor rules for class_def and arguments Refactor rules for class_def and arguments Jan 5, 2020
@lysnikolaou
Copy link
Collaborator Author

@pablogsal @gvanrossum Is it okay if I push a PR that builds up on this for issues #126 and #136?

@gvanrossum
Copy link
Collaborator

gvanrossum commented Jan 6, 2020 via email

Copy link
Collaborator

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

This is a very subtle area! I've deleted more comments than I've left...

data/simpy.gram Outdated Show resolved Hide resolved
data/simpy.gram Outdated
seq_extract_starred_exprs(p, a),
seq_delete_starred_exprs(p, a),
EXTRA) }
| a=posarg b=[',' c=args { c }] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like we can get rid of posarg and instead write a=named_expression here -- the case where posarg matches starred_expression is already handled by the first alternative.

(Things are still weird -- one confusing thing is that starred_expression must start with *, while named_expression may or may not contain a := operator. But I don't think we can simplify further, since the resolution of "ambiguities" is quite subtle for this rule.)

docs/helpers.md Outdated Show resolved Hide resolved
docs/helpers.md Outdated Show resolved Hide resolved
pegen/pegen.c Outdated Show resolved Hide resolved
@lysnikolaou lysnikolaou merged commit 7591b3c into we-like-parsers:master Jan 6, 2020
@lysnikolaou lysnikolaou deleted the class-def-args branch January 6, 2020 21:32
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.

Simpy refactorings: class_def and arguments
3 participants