Local constant declarations can specify const multiple times #6929
Conversation
|
@dotnet-bot retest prtest/win/dbg/unit64 please |
davkean
added
the
Community
|
tag @dotnet/roslyn-compiler |
|
okay to test |
|
LGTM |
| @@ -7996,6 +7996,12 @@ private void ParseDeclarationModifiers(SyntaxListBuilder list) | |||
| mod = this.AddError(mod, ErrorCode.ERR_BadMemberFlag, mod.Text); | |||
| } | |||
|
|
|||
| // check for duplicates, can only be const | |||
There was a problem hiding this comment.
Comments shouldn't be used to break apart if / else blocks. Please move the comment into the else block.
There was a problem hiding this comment.
@jaredpar: Thanks, I wasn't sure about this :) Was just an if initially. Edit: Holy Batman, I just noticed how big that file is!
|
👍 modulo the style comment. |
| else if (list.Any(mod.Kind)) | ||
| { | ||
| // check for duplicates, can only be const | ||
| mod = this.AddError(mod, ErrorCode.ERR_TypeExpected, mod.Text); |
There was a problem hiding this comment.
I think it would be better to report more specific error, i.e. duplicate modifier.
There was a problem hiding this comment.
@AlekseyTs: Agreed, but I for now I just recreated the behavior of fields. The previous compiler reports a different error if I recall correctly, but for consistency I chose Roslyn's behavior. Edit: I cant seem to replicate the behavior I saw earlier on old compiler. Error was CS1041: Identifier expected, 'const' is a keyword'. (Or perhaps that was another case?)
There was a problem hiding this comment.
Given that this function is only used to parse a local declaration, I agree that ERR_TypeExpected is in agreement with the language grammar.
|
LGTM |
|
test eta please |
|
Merging this in, thanks again for another contribution! |
Local constant declarations can specify const multiple times
Fixes #6761.
Matches behavior for const field declarations.
Side note: There should be a
ParseAndValidatethat ignores additional errors. Sometimes the extra errors make a test needlessly complex.