Allow null literals to contribute nullability to type inference#56022
Allow null literals to contribute nullability to type inference#56022RikkiGibson merged 7 commits intodotnet:mainfrom
Conversation
ghost
added
the
|
From our offline chat, I'm not sure that's the right approach. I'd suggest reading through https://github.com/dotnet/csharplang/blob/main/proposals/csharp-8.0/nullable-reference-types-specification.md#generic-type-inference In reply to: 909400174 |
jcouv
left a comment
jcouv
left a comment
There was a problem hiding this comment.
Done with review pass (iteration 3)
…ion/MethodTypeInference.cs
|
@dotnet/roslyn-compiler Please review |
|
@jaredpar FYI I tried building dotnet/runtime with this change and I wasn't able to observe any new nullable warnings. I wasn't able to build everything because of weird variant of dotnet/runtime#42397 but I was able to build ~427 C# projects successfully. |
| { | ||
| var ordinal = ((TypeParameterSymbol)target.Type).Ordinal; | ||
| var typeWithAnnotations = _extensions.GetTypeWithAnnotations(argument); | ||
| _nullableAnnotationLowerBounds[ordinal] = _nullableAnnotationLowerBounds[ordinal].Join(typeWithAnnotations.NullableAnnotation); |
There was a problem hiding this comment.
nit: Given that we're now only using Join and the starting values are NotAnnotated. I'm wonder if we should just store a boolean. I'll leave that for consideration for you and second reviewer. I don't feel strongly (seems equivalent). #Closed
There was a problem hiding this comment.
I also don't feel strongly about it. This did inspire some additional tests for 'null!' and 'null' in a disabled context, so thanks :)
|
@dotnet/roslyn-compiler for a second review. |
|
@dotnet/roslyn-compiler Please review |
1 similar comment
|
@dotnet/roslyn-compiler Please review |
RikkiGibson
changed the title
4 participants
Closes #43536
We skip removing conversions from call arguments when the conversion is predefined and applied to a null literal. Note that other kinds of constant-null expressions we do expect to have a natural type, so we do remove their conversion.Changed approach to instead add special "nullable bounds" to method type inference per #56022 (comment). Will update the PR title prior to merge to avoid messing up email threading.