Consider nullability in conversion to constraint type#46405
Consider nullability in conversion to constraint type#46405cston merged 6 commits intodotnet:masterfrom
Conversation
| var comp = CreateCompilation(source); | ||
| var comp = CreateCompilation(source, parseOptions: TestOptions.RegularPreview); | ||
| comp.VerifyEmitDiagnostics(); | ||
| } |
There was a problem hiding this comment.
The only changes in LambdaNullReturn_01 to _06 is that each pair of tests have been combined. Two new tests 04 and _05 were added at the end. (There were already too many similar tests and adding 4 new tests rather than combining pairs would have made the situation worse.)
|
@dotnet/roslyn-compiler please review. |
|
Looking |
| @@ -6474,9 +6483,25 @@ static NullableFlowState getReferenceConversionResultState(TypeWithAnnotations t | |||
| // Converting to a less-derived type (object, interface, type parameter). | |||
| // If the operand is MaybeNull or MaybeDefault, the result should be | |||
There was a problem hiding this comment.
Is the comment still accurate? I'm having trouble mapping it to the logic below. #Resolved
| } | ||
|
|
||
| // If type parameter 1 depends on type parameter 2, | ||
| // returns the corresponding annotation. |
There was a problem hiding this comment.
what does "corresponding annotation" mean? #Resolved
| else if (targetType.NullableAnnotation.IsNotAnnotated() && | ||
| type is TypeParameterSymbol typeParameter1 && | ||
| targetType.Type is TypeParameterSymbol typeParameter2 && | ||
| dependsOnTypeParameter(typeParameter1, typeParameter2, NullableAnnotation.NotAnnotated, out var annotation)) |
There was a problem hiding this comment.
Can dependsOnTypeParameter ever return false here (and above in getReferenceConversionResultState)? #Resolved
There was a problem hiding this comment.
The dependsOnTypeParameter() check can fail in getReferenceConversionResultState(), with the following for instance (see NullabilityOfTypeParameters_171()):
#nullable enable
class C
{
static U F<T, U>(T t)
where T : C?
where U : T
{
return (U)t;
}
}I suspect the check here in getBoxingConversionResultState() cannot fail, because both the target type and the operand type must be unconstrained type parameters, and we only create boxing conversions for those cases when there is an implicit conversion from operand to target type which requires checking the constraint types.
In reply to: 466677704 [](ancestors = 466677704)
| { | ||
| return NullableFlowState.MaybeDefault; | ||
| var type = operandType.Type; | ||
| if (type?.IsTypeParameterDisallowingAnnotationInCSharp8() != true) |
There was a problem hiding this comment.
nit: consider aligning with next local function if (type is null || !type.IsTypeParameterDisallowingAnnotationInCSharp8()) #Resolved
| static object F3<T>(T? t) where T : class => t; // 1 | ||
| static object? F4<T>(T? t) where T : class => t; | ||
| }"; | ||
| comp = CreateCompilation(source2, parseOptions: TestOptions.RegularPreview); |
There was a problem hiding this comment.
is the language version relevant? If not, consider using C# 8.
Also applicable to other scenarios in this test. #Resolved
jcouv
left a comment
There was a problem hiding this comment.
Done with review pass (iteration 4)
…features/function-pointers * upstream/master: (465 commits) Stop specifying Rich Nav's version of dotnet Fix nullable reference warning Fix up some calls to SymbolKey.Resolve now that it's annotated Remove try/catch of ArgumentException Null annotate SymbolKey Annotate Location.IsInSource as ensuring SourceTree is non-null Fix nullable annotation of Compilation.CreateErrorTypeSymbol Update null state for add/remove on field-like events (#46477) Throw IndexOutOfRangeException from BitVector.this[int] if index < 0 (#46627) Delete some dead support for changing legacy options Delete our use of Microsoft.VisualStudio.CodingConventions Fix crash in the preview of a code action that modified an .editorconfig Move null check above where the variable is used (#46558) Add integration test to configure diagnostic severity via editorconfig Disable a few Mac tests Consider nullability in conversion to constraint type (#46405) Added interpolated strings and tests Introduce request context (#46557) Simplify NativeIntegerAttribute encoding (#46522) Update extension getenumerator status. (#46607) ...
Fixes #46150
Fixes #46410