Cannot assign maybe-null value to TNotNull variable #41445
Conversation
jcouv
added
Area-Compilers
New Language Feature - Nullable Reference Types
jcouv
force-pushed
the
warn-notnull
branch
2 times, most recently
from
329dd94
to
bc9506b
Compare
| @@ -121906,6 +121908,80 @@ void M() | |||
| ); | |||
| } | |||
|
|
|||
| [Fact, WorkItem(41437, "https://github.com/dotnet/roslyn/issues/41437")] | |||
There was a problem hiding this comment.
@dotnet/roslyn-compiler This PR fixing a bug we noticed in LDM last week with a TNotNull.
Note that fixing this makes it harder to use TryGet APIs (with [MaybeNullWhen(...)] with a TNotNull) and this problem hit the roslyn codebase.
@cston I wonder if the introduction of T?? later in C# 9 might improve that situation (ie. by inferring TNotNull?? for the out var instead of TNotNull). Something to consider...
There was a problem hiding this comment.
@jaredpar From discussion with Chuck, it's probably better to hold this PR back and ship it along with T?? work. I'll stop by to talk.
There was a problem hiding this comment.
From offline discussion with Jared, we'll hold this PR until we do the T?? work.
jcouv
added
PR For Personal Review Only
jcouv
force-pushed
the
warn-notnull
branch
2 times, most recently
from
6aadf94
to
1c6aeda
Compare
| @@ -34740,16 +34740,14 @@ class C<T> | |||
|
|
|||
| [Theory] | |||
| [InlineData("T")] | |||
There was a problem hiding this comment.
The test no longer needs parameters. #Closed
jcouv
removed
Blocked
PR For Personal Review Only
| // return t; // 1 | ||
| Diagnostic(ErrorCode.WRN_NullReferenceReturn, "t").WithLocation(8, 16) | ||
| ); | ||
| } |
There was a problem hiding this comment.
} [](start = 8, length = 1)
Please test:
[return: MaybeNull] TNotNull M<TNotNull>(TNotNull t) where TNotNull : notnull
{
...
}
``` #Closed
| } | ||
|
|
||
| [Fact] | ||
| public void TNotNullFieldMustBeAssigned() |
There was a problem hiding this comment.
TNotNullFieldMustBeAssigned [](start = 20, length = 27)
Was the behavior of this method affected? This case and others are covered in UninitializedNonNullableFieldTests.GenericType_NotNullConstraint. #Closed
|
Taking a look |
ghost
modified the milestones:
| var source = @" | ||
| class C | ||
| { | ||
| TNotNull M<TNotNull>(TNotNull t) where TNotNull : notnull |
There was a problem hiding this comment.
Sorry, I should have asked earlier, but do corresponding tests already exist for 'class' constraint? (I would be surprised if they didn't..)
There was a problem hiding this comment.
Yes, it's covered by NullCastToUnannotatableReferenceTypedTypeParameter I think
* upstream/master: (304 commits) Tweak diagnostics to account for records (dotnet#46341) Diagnose precedence inversion in a warning wave (dotnet#46239) Remove PROTOTYPE tag (dotnet#45965) Only run a single pass of NullableWalker per-member (dotnet#46402) Fix crash, and offer "declare as nullable" for tuple fields (dotnet#46437) Simplify contract for RunWithShutdownBlockAsync Fix optprof plugin input check if EditorAdaptersFactoryService gives us a null buffer Cannot assign maybe-null value to TNotNull variable (dotnet#41445) Fix overload resolution to handle binary compat in the face of covariant returns (dotnet#46367) Same failure on Linux Skip some tests on Mac Added search option for inline parameter name hints Spelling tweak docs Improve comment Improve the "not exhaustive" diagnostic in the presence of a when clause. (dotnet#46143) PR feedback Use record keyword to display records (dotnet#46338) remove test that aserts .NET Standard should be prefered over .NET Framework ...
Fixes #41437
Note: some references to #41437 were added to the code since this PR was opened. Don't forget to resolve them before merging.