Don't report modifier errors while parsing. #16339
Conversation
CyrusNajmabadi
force-pushed
the
parserDiagnostics29
branch
3 times, most recently
from
c468b3d
to
e82cbb4
Compare
|
Note: this PR depends on #16338 which shoudl be reviewed first. |
|
I also recommend reviewing #16341 first. |
| @@ -40,17 +40,19 @@ internal sealed class LocalFunctionSymbol : MethodSymbol | |||
| _syntax = syntax; | |||
| _containingSymbol = containingSymbol; | |||
|
|
|||
| var diagnostics = DiagnosticBag.GetInstance(); | |||
There was a problem hiding this comment.
I think this new variable is unused? #Resolved
There was a problem hiding this comment.
You are correct. Removing. #Resolved
|
Customer scenario None. This is general infrastructure work for the compiler based on design changes the team agreed on. Bugs this fixes: N/A Workarounds, if any N/A Risk Low. Performance impact Low. Is this a regression from a previous update? N/A Root cause analysis: N/A. How was the bug found? N/A |
1 similar comment
|
Customer scenario None. This is general infrastructure work for the compiler based on design changes the team agreed on. Bugs this fixes: N/A Workarounds, if any N/A Risk Low. Performance impact Low. Is this a regression from a previous update? N/A Root cause analysis: N/A. How was the bug found? N/A |
|
Tagging @MattGertz Improvements in parser that benefit IDE and compiler. |
|
Approved pending tests. |
CyrusNajmabadi
force-pushed
the
parserDiagnostics29
branch
from
9fc3bd1
to
3bc9c84
Compare
|
retest this please |
CyrusNajmabadi
force-pushed
the
parserDiagnostics29
branch
from
be988d1
to
d54126e
Compare
|
Rebased to 15.6 |
|
@CyrusNajmabadi This branch is fairly stale, should it still be active? |
|
@jasonmalinowski Yes it should. I'm still waiting on a second compiler review. @jcouv could you look when you have a chance? |
| @@ -529,7 +529,7 @@ internal enum ErrorCode | |||
| ERR_PartialMethodInvalidModifier = 750, | |||
| ERR_PartialMethodOnlyInPartialClass = 751, | |||
| ERR_PartialMethodCannotHaveOutParameters = 752, | |||
| ERR_PartialMethodOnlyMethods = 753, | |||
| // ERR_PartialMethodOnlyMethods = 753, Removed as it is subsumed by ERR_PartialMisplaced | |||
There was a problem hiding this comment.
ERR_PartialMethodOnlyMethods [](start = 11, length = 28)
Did you remove the corresponding resource string? #Resolved
There was a problem hiding this comment.
#fixed #Resolved
| { | ||
| var result = DeclarationModifiers.None; | ||
|
|
||
| bool seenNoDuplicates = true; |
There was a problem hiding this comment.
Consider naming those positively (seenDuplicates, seenAccessibilityDuplicates). #Resolved
There was a problem hiding this comment.
I'm preserving the logic as it was in ParseModifiers. I think that makes the review a bit easier. #Resolved
| @@ -44,7 +45,8 @@ internal static class ModifierUtils | |||
| switch (oneError) | |||
| { | |||
| case DeclarationModifiers.Partial: | |||
| diagnostics.Add(ErrorCode.ERR_PartialMethodOnlyMethods, errorLocation); | |||
| // Errors about 'partial' are reported in ToDeclarationModifiers. So no need to report | |||
There was a problem hiding this comment.
The comment is not quite right. There is one path that calls CheckModifiers directly. See SourceMemberContainerSymbol.cs MakeAndCheckTypeModifiers.
There was a problem hiding this comment.
True. Let me investigate. We may have a test hole here.
| Diagnostic(ErrorCode.ERR_SemicolonExpected, "partial").WithLocation(6, 13), | ||
| // (6,13): error CS0102: The type 'PartialPartial' already contains a definition for '' | ||
| // partial partial void PM() | ||
| Diagnostic(ErrorCode.ERR_DuplicateNameInClass, "").WithArguments("PartialPartial", "").WithLocation(6, 13), |
There was a problem hiding this comment.
The empty string seems strange. Is that intended?
Please file issue if this is the one you meant by "not great".
…c-member-reference * dotnet/features/ioperation: (134 commits) Fix build break in IOperation feature branch Fix unit tests that broke with previous commit Address PR feedback from Chuck Fix unit test Add IOperation support for variations of object creation expressions (type parameter, nopia and dynamic) Remove dead code from GenerateConstructor. Disable NETCore version check Fixed review notes. Add comment Disable NETCore version check Show modifier/method/type block declaration keywords after attribute lists Improve type argument list parsing in error conditions. (dotnet#19467) Don't report modifier errors while parsing. (dotnet#16339) Fix assert and merge Remove extra space Restore failure behavior. Simplify implementation of parsing constraints. (dotnet#16429) Fix csproj Remove CapturedVariablesByLambda (dotnet#20878) Remove testing wait ...
No description provided.