Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lexer: report missing error on ~ #16194

Merged
merged 1 commit into from
Oct 28, 2023
Merged

Conversation

auduchinok
Copy link
Member

@auduchinok auduchinok commented Oct 27, 2023

Fixes #16135.

Previously, the parser would fail on 'reserved' identifiers : ~ and unfinished backticked identifiers. #15030 added recovery by parsing these as normal identifiers and added a proper error reporting about the backticked identifiers to the lexer. The ~ case was overlooked in the lexer, so there was no additional error after the parser one was gone. The same error is reported now.

@auduchinok auduchinok requested a review from a team as a code owner October 27, 2023 18:19
@vzarytovskii vzarytovskii enabled auto-merge (squash) October 27, 2023 19:11
@vzarytovskii vzarytovskii merged commit 579cf1e into dotnet:main Oct 28, 2023
@auduchinok auduchinok deleted the lexer-reserved branch October 28, 2023 11:01
@vzarytovskii
Copy link
Member

@auduchinok this is a behaviour change, right? Before it was yielding the reserved diagnostics, and now parser error?

@auduchinok
Copy link
Member Author

auduchinok commented Oct 28, 2023

@auduchinok this is a behaviour change, right? Before it was yielding the reserved diagnostics, and now parser error?

@vzarytovskii No, I'd say there's no meaningful change in the behaviour for a user. It's the same as with the most improvements we have to error recovery in parsing and type checking, i.e. there might be a change in the error number and text, but usually for a much better one.

Prior to #15030 the error would be the generic parsing error:

error FS0010: Unexpected reserved keyword in implementation file

It was especially problematic with unfinished identifiers, as it wouldn't help you and would probably confuse much more instead:

` // typing a new (unfinished) name
```` // typing a new (unfinished) name

It has always made me wondering: why these identifiers are considered keywords? Why are they reserved? The error always felt wrong, and it seemed just like an overlook because of the generic and not helping message.

It also didn't help that the parser would consider each backtick as a separate token, so after four of them (i.e. an empty backticked name) it'd break on 4 levels of recovery, likely failing to parse lots of things around.

Now the parser is happy and the error actually helps you by saying that it's indeed considered an identifier, but not (yet) a valid one, so you're fine and can just finish the identifier. It's less so with ~ but it was historically treated the same way as the backticks, so I've just restored that, which is good enough, compared to what we've had before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Single tilde ~ in expressions compiles, but shouldn't
4 participants