[spec_parser] Add isNoSkip() to disambiguate metadata from record types#62385
[spec_parser] Add isNoSkip() to disambiguate metadata from record types#62385modulovalue wants to merge 1 commit intodart-lang:mainfrom
Conversation
|
Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at: https://dart-review.googlesource.com/c/sdk/+/471863 Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly. Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR). |
|
Is this something we want to land @eernstg ? |
|
Yes, I just haven't gotten around to it yet. |
2b0dfe9 to
689e037
Compare
|
https://dart-review.googlesource.com/c/sdk/+/471863 has been updated with the latest commits from this pull request. |
1 similar comment
|
https://dart-review.googlesource.com/c/sdk/+/471863 has been updated with the latest commits from this pull request. |
|
@modulovalue @eernstg friendly ping, I am retriaging PRs and noticed that this one is fairly stale. |
|
Thanks for the heads up, @mraleph! I'm afraid there is no known solution to this issue, though. The treatment of whitespace probably cannot be expressed in the grammar unless we re-shape the entire grammar to know about whitespace everywhere, which would be a massive change. This is not an attractive option because it surely causes the grammar to be less readable, and more different from the grammar in the language specification documents, and the main purpose of the spec parser is that it bridges the gap between the specification documents and the actual code. ANTLRv4 doesn't support syntactic predicates, so we can't use the straightforward "customized look-ahead" technique that we used with ANTLRv3. The ANTLRv4 parser generator does support semantic predicates, but they cannot be used to guide the parsing process: the only outcomes of evaluating a semantic predicate (apart from side effects, if any) is (1) the evaluation yields true, and parsing proceeds as if the predicate had not been there, or (2) the evaluation yields false, in which case the parsing fails and a syntax error is reported. So we probably can't use a semantic predicate to distinguish the two syntactic situations of interest here. (Exceptions: A semantic predicate can be used to guide the recognition done by lexical rules, but that doesn't suffice here. Also, I think semantic predicates can guide parsing when they occur at the front of the alternative, but not when they occur anywhere after the first non-terminal, and I don't think that'll suffice for this purpose, either. I did try that.) |
|
@modulovalue, do you see a way to do it? |
|
@eernstg I did suggest an alternative and the patch should be live in the CL. I might've forgotten to check a box as it appears you have not received a notification.
You mentioned it briefly, but I found the following during my investigation, see https://github.com/antlr/antlr4/blob/master/doc/predicates.md#predicates-in-lexer-rules:
Predicates appear to be supported, but since ANTLR is LL based, predicates need to be at the start of a production. With this behavior be can do a "predictive parse" to implement the ambiguity resolution strategy in the ANTLR grammar. Let me know what you think! |
This change adds a semantic predicate to the metadatum rule that checks whether there is whitespace between the constructor designation and the opening parenthesis of arguments.
Without this fix:
With this fix:
The isNoSkip() function compares character positions to detect if any tokens (whitespace, comments) were skipped between the previous and current visible tokens.
Bug: #62384