-
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
Minor using decl fixes #39373
Minor using decl fixes #39373
Conversation
{ | ||
Debug.Assert(this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword); | ||
SyntaxToken awaitToken = this.EatContextualToken(SyntaxKind.AwaitKeyword); | ||
return CheckFeatureAvailability(awaitToken, MessageID.IDS_FeatureAsyncStreams); | ||
return (feature.HasValue) ? CheckFeatureAvailability(awaitToken, feature.Value) : awaitToken; |
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.
(feature.HasValue) [](start = 23, length = 18)
Parentheses are not needed. #Closed
{ | ||
Debug.Assert(this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword); | ||
SyntaxToken awaitToken = this.EatContextualToken(SyntaxKind.AwaitKeyword); | ||
return CheckFeatureAvailability(awaitToken, MessageID.IDS_FeatureAsyncStreams); | ||
return (feature.HasValue) ? CheckFeatureAvailability(awaitToken, feature.Value) : awaitToken; |
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.
Value [](start = 89, length = 5)
Can use GetValueOrDefault()
since we've checked HasValue
. #Closed
"; | ||
var comp = CreateCompilationWithTasksExtensions(new[] { source }); | ||
var comp = CreateCompilationWithTasksExtensions(new[] { source, CSharpTestBase.IAsyncDisposableDefinition }); |
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.
CSharpTestBase [](start = 76, length = 14)
Is type qualifier needed? Same question for other tests.
@@ -39,11 +29,11 @@ public System.Threading.Tasks.ValueTask DisposeAsync() | |||
} | |||
} | |||
"; | |||
var comp = CreateCompilationWithTasksExtensions(source + s_interfaces, parseOptions: TestOptions.Regular7_3); | |||
var comp = CreateCompilationWithTasksExtensions(source + CSharpTestBase.IAsyncDisposableDefinition, parseOptions: TestOptions.Regular7_3); |
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.
source + CSharpTestBase.IAsyncDisposableDefinition [](start = 60, length = 50)
Since you're changing all these, consider using new[] { source, IAsyncDisposableDefinition }
instead if there's no reason to concatenate the two source files.
} | ||
else | ||
{ | ||
return this.ParseLocalDeclarationStatement(parseAwaitKeywordForAsyncStreams()); | ||
return this.ParseLocalDeclarationStatement(parseAwaitKeyword()); |
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 missing a possible LangVersion diagnostic for using var
? Or is it reported elsewhere?
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, previously we were reporting for both the await
and the using
. This change removes the await
warning in this path so just the using gets the lang ver warning.
Some minor cleanup for using declarations
Fixes #28892
Fixes #32318