Python: A few more parser fixes #17822
Conversation
This is primarily useful for ensuring that errors where a node does not have an appropriate context set in `python.tsg` actually have an effect on the pass/fail status of the parser tests. Previously, these would just be logged to stdout, but test could still succeed when there were errors present. Also fixes one of the logging lines in `tsg_parser.py` to be more consistent with the others.
That is, the `*T` in `def foo(*args : *T): ...`. This is apparently a piece of syntax we did not support correctly until now. In terms of the grammar, we simply add `list_splat` as a possible alternative for `type` (which could previously only be an `expression`). We also update `python.tsg` to not specify `expression` those places (as the relevant stanzas will then not work for `list_splat`s). This syntax is not supported by the old parser, hence we only add a new parser test for it.
Turns out, `except*` is actually not a token on its own according to the Python grammar. This means it's legal to write `except *foo: ...`, which we previously would consider a syntax error. To fix it, we simply break up the `except*` into two separate tokens.
Surprisingly, the new parser did not support these constructs (and the relevant test was missing this case), so on files that required the new parser we were unable to parse this construct. To fix it, we add `list_pattern` (not to be confused with `pattern_list`) as a `tree-sitter-python` node that results in a `List` node in the AST.
A somewhat complicated solution that necessitated adding a new custom function to `tsg-python`. See the comments in `python.tsg` for why this was necessary.
There was a problem hiding this comment.
Thanks for fixing these up. I feel the pain regarding having to compensate for tree-sitters matching in Rust code...
I have two comments, but I am not sure if they are worth addressing right away:
-
The contract for the logger seems a little brittle. Ideally, we would have an
end_of_test-signal that would reset the error count and then checking could be idempotent. -
I think we recently needed an "either list splat or expression" (we would look for certain fields to capture this), can we now use
typeinstead?
You're probably right. The current setup is a bit awkward in that it uses the same logger for all of the tests. Ideally we would create a new one for each test. Seeing as we basically never modify these tests, however, I don't think it's a high priority. (Also, I'm not entirely clear about what kind of signalling you have in mind. I guess one way would be to implement it as a suitable context handler.)
I think what you may be remembering is our use of fields to distinguish between plain That said, in principle we could use |
A mixed bag of parser fixes for issues seen on
python/cpython. See the individual commits for more information.Pull Request checklist
All query authors
.qhelp. See the documentation in this repository.Internal query authors only
.ql,.qll, or.qhelpfiles. See the documentation (internal access required).