Python: Allow use of match as an identifier#19895
Conversation
This previously only worked in certain circumstances. In particular, assignments such as `match[1] = ...` or even just `match[1]` would fail to parse correctly. Fixing this turned out to be less trivial than anticipated. Consider the fact that ``` match [1]: case (...) ``` can either look the start of a `match` statement, or it could be a type ascription, ascribing the value of `case(...)` (a call) to the item at index 1 of `match`. To fix this, then, we give `match` the identifier and `match` the statement the same precendence in the grammar, and additionally also mark a conflict between `match_statement` and `primary_expression`. This causes the conflict to be resolved dynamically, and seems to do the right thing in all cases.
IdrissRio
left a comment
IdrissRio
left a comment
There was a problem hiding this comment.
LGTM 👍
Quick (non-blocking) question: would it make sense to run DCA to check that none of the existing projects fail due to some subtle corner case?
I guess that if anything unexpected happens, it would still be caught in the nightly run.
Oh yes, I completely forgot to do that. We usually do that for any change, no matter how small (unless it literally doesn't touch any code at all), so thanks for the reminder! |
IdrissRio
left a comment
IdrissRio
left a comment
There was a problem hiding this comment.
Two projects failed in DCA:
- Free_cad: Exit code 99 due to Java heap out-of-memory during query evaluation.
- ddnet: failed with
ECONNRESETduring artifact finalization. To me, this looks like a transient network issue, unrelated to the changes.
I will do a DCA re-run to make sure what we see is just noise, even though thefree_cadfailure might be related to the changes in this PR.
Alsostage-timingseems a bit off.
I'll block this to prevent the merge while we wait for @tausbn’s input.
FreeCAD has been OOMing for a while (one of the quality queries we ported away from points-to recently had some changes that caused this -- see #19641). It's unlikely to be related to these changes.
Agreed.
I had a look at the stage timings, and I don't think there's cause for concern. A bunch of queries got 6 seconds slower, and a bunch of queries got 6-10 seconds faster (mostly on
Let's wait and see if the |
This previously only worked in certain circumstances. In particular, assignments such as
match[1] = ...or even justmatch[1]would fail to parse correctly.Fixing this turned out to be less trivial than anticipated. Consider the fact that
can either look the start of a
matchstatement, or it could be a type ascription, ascribing the value ofcase(...)(a call) to the item at index 1 ofmatch.To fix this, then, we give
matchthe identifier andmatchthe statement the same precendence in the grammar, and additionally also mark a conflict betweenmatch_statementandprimary_expression. This causes the conflict to be resolved dynamically, and seems to do the right thing in all cases.