Skip to content

[spec_parser] Add isNoSkip() to disambiguate metadata from record types#62385

Open
modulovalue wants to merge 1 commit intodart-lang:mainfrom
modulovalue:metadata-record-disambiguation
Open

[spec_parser] Add isNoSkip() to disambiguate metadata from record types#62385
modulovalue wants to merge 1 commit intodart-lang:mainfrom
modulovalue:metadata-record-disambiguation

Conversation

@modulovalue
Copy link
Copy Markdown
Contributor

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:

  • "@foo () f() {}" parses "()" as metadata arguments (incorrect)

With this fix:

  • "@foo() f() {}" parses "()" as metadata arguments (no whitespace)
  • "@foo () f() {}" parses "@foo" as metadata, "()" as record return type

The isNoSkip() function compares character positions to detect if any tokens (whitespace, comments) were skipped between the previous and current visible tokens.

Bug: #62384

@copybara-service
Copy link
Copy Markdown

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).

@mraleph
Copy link
Copy Markdown
Member

mraleph commented Jan 23, 2026

Is this something we want to land @eernstg ?

@eernstg
Copy link
Copy Markdown
Member

eernstg commented Jan 23, 2026

Yes, I just haven't gotten around to it yet.

@modulovalue modulovalue force-pushed the metadata-record-disambiguation branch from 2b0dfe9 to 689e037 Compare February 2, 2026 21:05
@copybara-service
Copy link
Copy Markdown

https://dart-review.googlesource.com/c/sdk/+/471863 has been updated with the latest commits from this pull request.

1 similar comment
@copybara-service
Copy link
Copy Markdown

https://dart-review.googlesource.com/c/sdk/+/471863 has been updated with the latest commits from this pull request.

@mraleph
Copy link
Copy Markdown
Member

mraleph commented Mar 23, 2026

@modulovalue @eernstg friendly ping, I am retriaging PRs and noticed that this one is fairly stale.

@eernstg
Copy link
Copy Markdown
Member

eernstg commented Mar 23, 2026

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.)

@eernstg
Copy link
Copy Markdown
Member

eernstg commented Mar 23, 2026

@modulovalue, do you see a way to do it?

@modulovalue
Copy link
Copy Markdown
Contributor Author

modulovalue commented Mar 23, 2026

@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.

ANTLRv4 doesn't support syntactic predicates, so we can't use the straightforward "customized look-ahead" technique that we used with ANTLRv3.

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:

In parser rules, predicates must appear on the left edge of alternatives to aid in alternative prediction.

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants