Rectify async checks logic in presence of AsyncMethodBuilder attribute #65352
Conversation
| @@ -1102,7 +1102,7 @@ protected void AsyncMethodChecks(bool verifyReturnType, Location errorLocation, | |||
|
|
|||
| if (this.HasAsyncMethodBuilderAttribute(out _)) | |||
| { | |||
| hasErrors |= MessageID.IDS_AsyncMethodBuilderOverride.CheckFeatureAvailability(diagnostics, this.DeclaringCompilation, errorLocation); | |||
| hasErrors |= !MessageID.IDS_AsyncMethodBuilderOverride.CheckFeatureAvailability(diagnostics, this.DeclaringCompilation, errorLocation); | |||
There was a problem hiding this comment.
!MessageID.IDS_AsyncMethodBuilderOverride.CheckFeatureAvailability(diagnostics, this.DeclaringCompilation, errorLocation);
In my opinion registering language version check in hasErrors is not the right thing to do. Wrong language version shouldn't change the analysis, and here, it clearly does. #Closed
There was a problem hiding this comment.
In that case, should
roslyn/src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
Lines 226 to 230 in 9c04fee
There was a problem hiding this comment.
@Youssef1313 That seems a good idea. There's at least one check there that is conditional on hasErrors.
|
Done with review pass (commit 1) |
| } | ||
| """; | ||
|
|
||
| var compilation = CreateCompilation(source, parseOptions: useCSharp9 ? TestOptions.Regular9 : TestOptions.Regular10, targetFramework: TargetFramework.Net70); |
There was a problem hiding this comment.
I think Regular10 is better. That's what we usually do for explicit LangVer tests that verify the boundary when the feature was introduced.
|
@dotnet/roslyn-compiler for second review (one-line fix). Thanks |
Fixes #60332