daniellansun commented on PR #2921: URL: https://github.com/apache/groovy/pull/2921#issuecomment-5637141960
Thanks Paul — that's a much closer reading than mine was, and I agree with the recommendation. Master only for now; I would not put this in the 6.0 RC line. The 1.6× figure is parse-only, and a grammar/ATN change is the wrong kind of surprise during RC. A backport after some settling time (and a corpus AST sweep if we do go there) sounds right. On the three deltas: 1. **Supplementary-plane capitalization.** You are right that `isUpperCase(LA(-1))` after a surrogate pair only saw the low surrogate, so everything in that plane used to lex as `Identifier`. Using the decoded code point is a real parse change for `className`. I think the new behaviour is the one we want, but it is not “strictly performance”. I have extracted `lastConsumedCodePoint()` so that mistake is harder to reintroduce, and added tests (Deseret capital vs small, type parameter vs command, `isInvalidLocalVariableDeclaration` on the code point). Follow-up: `e27a4dabbd`. 2. **`elementValues` gate.** Glad that checked out. I added a sweep over `identifier ∪ keywords` (including `MODULE`) so the BitSet cannot drift silently, plus a note on those two parser rules. 3. **Invalid-octal caret.** Agreed it was a latent counter bug. The follow-up reports from the token start (same pattern as unclosed comments), so a suffix or a second `08` in the file cannot shift the caret. Tests cover `08`, `08L`, `08 09`, and `errorIgnored`. Happy to drop or reword any of that if you would rather keep the old supplementary-plane classification for this PR and treat the code-point fix separately. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
