-
Notifications
You must be signed in to change notification settings - Fork 218
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
[Lark-PR-3] SpecialVarRef removed and merged with Name #1534
base: main
Are you sure you want to change the base?
Conversation
8643cb5
to
a0831cf
Compare
c59710d
to
b3f1512
Compare
@@ -160,13 +160,6 @@ def enter_index_slice(self, node: ast.IndexSlice) -> None: | |||
is_range: bool, | |||
""" | |||
|
|||
def enter_special_var_ref(self, node: ast.SpecialVarRef) -> None: |
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.
Can we have old SpecialVarRef to have a parent of atom trailer?
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.
I checked these changes. There is a comment I added. @marsninja please check it
b3f1512
to
99e9cab
Compare
// | ||
// module: STRING? toplevel_stmt* | ||
// | ||
module: (toplevel_stmt (tl_stmt_with_doc | toplevel_stmt)*)? |
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.
module: STRING? (STRING? toplevel_stmt)*
It could replace
module: (toplevel_stmt (tl_stmt_with_doc | toplevel_stmt)*)?
| STRING (tl_stmt_with_doc | toplevel_stmt)*
tl_stmt_with_doc: STRING toplevel_stmt
So that we can get rid of tl_stmt_with_doc
node in parser
If you want to keep that node
we can go with
module: STRING? tl_stmt_with_doc*
tl_stmt_with_doc: (STRING? toplevel_stmt)
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 can't do that cause there is ambiguity since we have STRING? STRING?
and the lark reduce doesn't know if the first string should be module dostring or class docstring,
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.
Yeah
Let's use this
module: STRING? tl_stmt_with_doc
tl_stmt_with_doc: (STRING? toplevel_stmt)*
I hope it fixes
Description
Depends on : #1523
SpecialVarRef
was removed and merged withName
In terms of function and use, there are almost no difference between name and special var ref (almost every where the logic is the same and copied), now they're merged.named_ref
renamed toname
in lark rule.