Reuse nullable override checks for delegate conversions #46953
Conversation
RikkiGibson
added
Area-Compilers
Feature - Nullable Reference Types
RikkiGibson
force-pushed
the
delegate-flow-annotations
branch
from
bf34d25 to
13c310c
Compare
|
|
||
| [Fact] | ||
| [WorkItem(44129, "https://github.com/dotnet/roslyn/issues/44129")] | ||
| public void DelegateCreation_FromMethodGroup_NullabilityAttributes() |
There was a problem hiding this comment.
Consider adding some variations of this test taking into account variance. Delegate types with out parameters and these attributes, for example. For the same type of scenario, also consider testing usage of MaybeNullWhen.
There was a problem hiding this comment.
I did not test with 'out' parameters because it turns out they are not covariant in the language, but I added a test for by-value parameters and returns.
I do feel like the variance checks themselves are fairly well exercised by the overload resolution tests--delegates are just an analogous reuse of those checks, so it feels like it might be OK to not cover every last scenario
| string? M1(string s) | ||
| { | ||
| var d = (D)M; | ||
| var d = (D)M1; // 1 |
There was a problem hiding this comment.
To clarify why these tests changed: the nullable override analysis will refrain from warning about parameters if we have already warned about the return type. Thus to exercise all the same code paths in this test we need to have a method with an incompatible return type and another method with a compatible one.
| } | ||
|
|
||
| CL0<string?> M1(CL0<string> x) { throw new System.Exception(); } | ||
| CL0<string> M2(CL0<string> x) { throw new System.Exception(); } |
There was a problem hiding this comment.
This test changed for the same reason as #46953 (comment).
|
Please review @dotnet/roslyn-compiler |
|
@dotnet/roslyn-compiler Please review |
|
Wouldn't have thought to re-use the override check logic, but it makes sense. Neat! |
ghost
added this to the
…-only-errors * upstream/master: (236 commits) Fix bug when "End statement" is used in single-line if (dotnet#47062) Solution asset cache refactoring (dotnet#46948) add specific tests to validate behavior between keys and snapshots Extract into separate files rename parameters rename parameters rename parameters rename parameters Add CancellationToken parameters to SyntaxTreeOptionsProvider Reuse nullable override checks for delegate conversions (dotnet#46953) Introduce warning for multiple entry points (sync + async) (dotnet#46832) Switch from throwing NotImplementedException and return E_NOTIMPL Delete Building for Core CLR.md (dotnet#47146) Adjust PrintMembers to avoid boxing and avoid extra space (dotnet#47095) Track asynchronous operation in InProcLanguageServer Use Task.FromCanceled where appropriate Apply suggestions from code review Address feedback Expose ParseOptions on generator context (dotnet#46919) Remove redundant statement in added tests ...
4 participants
Closes #44129