-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Language flag to only change diagnostics in syntax tree #11041
Conversation
@dotnet/roslyn-compiler for review. Thanks |
@VSadov @dotnet/roslyn-compiler for review, please. |
</data> | ||
<data name="String1" xml:space="preserve"> | ||
<value /> | ||
</data> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this addition intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I'll remove.
LGTM. There are a lot of test changes, I only glanced over them. |
// (7,18): error CS1002: ; expected | ||
// (a, b) => | ||
Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(7, 18) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is the only one that troubled me, as it used to pass parsing (it would have failed later).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is evidence that we need to really bang on the parser in situations that involve mixing lambdas, casts, tuple types, tuple expressions, etc.
In reply to: 62090097 [](ancestors = 62090097)
👍 |
@dotnet-bot test prtest/win/dbg/unit32 please |
LGTM |
Parsing now goes down the same paths with or without feature flags, but it adds diagnostics when a feature is not enabled.
Note this affected some scenarios with illegal C# 6 code, which is now still illegal but different errors.
There is even one scenario where the parsing used to be legal, but is now a parsing error (
ParserErrorMessageTests.cs
below, which I discussed with Vlad).Fixes #10794 too (CompilerTrait).