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
Grammar change to prevent shift/reduce problem with varargslist #46293
Comments
I wrote a translator from the CFG used in the Grammar file into a form for PLY. I varargslist: ((fpdef ['=' test] ',')* This grammar definition is ambiguous until the presence/lack of a "*". PLY state 469
! shift/reduce conflict for NAME resolved as shift.
COMMA .) ! NAME [ reduce using rule 32 (varargslist_star -> fpdef EQUAL test
My fix was to use this definition when I did the translation. varargslist: ((fpdef ['=' test] (',' fpdef ['=' test])* So far I've not found a functional difference between these two definitions, and By making this change it would be easier for the handful of people who write |
I don't have any objection in principle, but I would like to see the |
I've been working from the Grammar file from CVS for 2.6 ... I thought. "svn log" says it hasn't changed since 2007-05-19, when except/as was What did I miss? |
Sorry, I misread slightly (or else just got confused as to which of the |
+!. I had to do a very similar refactoring recently when trying to fix the parser module to understand keyword-only arguments and annotations. Taking keyword-only arguments into account, I think the replacement needs to be: varargslist: (vfpdef ['=' test] (',' vfpdef ['=' test])* [',' and the analogous replacement needs to be made in the production for 'typedargslist', too. BTW, I'm a bit surprised that the grammar doesn't allow for trailing commas after keyword-only arguments: that is, def f(a, b,): ... is fine, but
def f(*, a, b,): ... is a SyntaxError |
Here's a patch. I'm assuming that this doesn't violate the moratorium, since it's not so much a change to the grammar itself as a modification to the way that it's expressed. |
That seems more like an oversight than a deliberate restriction. Does (And agreed the moratorium doesn't apply). |
No, "def f(, a,): ..." is still a SyntaxError with the patch. I'd think that allowing that extra comma *would be a violation of the moratorium, though maybe it's debatable. Perhaps worth bringing up on python-dev? |
Good point - I just wouldn't worry about it in that case. |
Ah, now I have to apologise: after some discussion on #python-dev IRC, I'm afraid I've (perhaps foolishly) rejected this advice. :) See python-dev discussion starting at http://mail.python.org/pipermail/python-dev/2010-July/101636.html I'll hold off on the modification in this issue until that discussion resolves. |
Applied in r82837. I'll open a separate issue for the trailing commas. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: