Always check get method parameters in override indexer #74337
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
| @@ -16929,6 +16929,35 @@ class B2 : A<string> | |||
| Diagnostic(ErrorCode.ERR_ScopedMismatchInParameterOfOverrideOrImplementation, "F6").WithArguments("s").WithLocation(17, 32)); | |||
| } | |||
|
|
|||
| [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73384")] | |||
| public void Indexer_EscapeInGetter() | |||
There was a problem hiding this comment.
Do we have similar tests for interfaces?
There was a problem hiding this comment.
No and we had a dupe of almost the same check which causes the error to get dropped. Fixing.
| // (28,19): warning CS9191: The 'ref' modifier for argument 1 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead. | ||
| // _ = c[ref x]; | ||
| Diagnostic(ErrorCode.WRN_BadArgRef, "x").WithArguments("1").WithLocation(28, 19)); | ||
| // (17,9): warning CS9196: Reference kind modifier of parameter 'ref readonly int x' doesn't match the corresponding parameter 'in int x' in overridden or implemented member. |
There was a problem hiding this comment.
Do we have the inverse of this scenario in a test: override ref readonly with in?
There was a problem hiding this comment.
Yes, Implementation_RefReadonly_In_Indexer for example tests this scenario in an interface and implementation.
| @@ -1028,9 +1028,7 @@ void checkSingleOverriddenMember(Symbol overridingMember, Symbol overriddenMembe | |||
| overridingMemberLocation, | |||
| overriddenMethod, | |||
| overridingMethod, | |||
There was a problem hiding this comment.
Was the checkReturnType value superfluous before?
There was a problem hiding this comment.
It was only used to suppress nullable variance diagnostics related to the return type. The only time false was passed for it was for a property setter, which is going to return void anyway, so we don't really expect any nullable diagnostics related to the return type to occur.
There was a problem hiding this comment.
So, I hate to be the voice of insanity, but IL doesn't require that set be a void-returning method. What do we do if we're overriding a non-void setter?
There was a problem hiding this comment.
This change may cause redundant or strange nullable diagnostics to appear in that case. I am OK with that.
|
@dotnet/roslyn-compiler for a second review |
Closes #73384