-
Notifications
You must be signed in to change notification settings - Fork 33
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
Update to the latest ASDL #93
Conversation
@@ -10,20 +10,22 @@ module Python | |||
|
|||
stmt = FunctionDef(identifier name, arguments args, | |||
stmt* body, expr* decorator_list, expr? returns, | |||
string? type_comment) | |||
string? type_comment, type_param* type_params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR: @youknowone do you know if RustPython parses out type_comment
s or if we could remove this (unused) field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't parse it now. So removing it is okay right now.
To let RustPython keep the field for AST, changing to the type to empty tuple ()
will be better than entirely removing it.
Since type_comment is a part of Python3 spec, I thought it is expected to be implemented. CPython parser has an option flag to parse it or not.
@youknowone this should be ready now if you could approve the workflow again |
@youknowone this looks good to me. I'll go ahead and merge it by tomorrow evening (around now). Let me know if you object and I'll then defer merging too you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Uses to latest ASDL to generated code for handling of new generic type syntax. Note this does not otherwise attempt to add handling for the new syntax, the new fields should always be empty.
Required for #82