Skip to content

Commit

Permalink
Collection expressions: analyze nullability of element assignments
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed Sep 29, 2023
1 parent 904f294 commit b8d0c85
Show file tree
Hide file tree
Showing 6 changed files with 785 additions and 33 deletions.
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4025,8 +4025,8 @@ private bool HasLocalScope(BoundCollectionExpression expr)
switch (collectionTypeKind)
{
case CollectionExpressionTypeKind.ReadOnlySpan:
Debug.Assert(elementType is { });
return !LocalRewriter.ShouldUseRuntimeHelpersCreateSpan(expr, elementType);
Debug.Assert(elementType.Type is { });
return !LocalRewriter.ShouldUseRuntimeHelpersCreateSpan(expr, elementType.Type);
case CollectionExpressionTypeKind.Span:
return true;
case CollectionExpressionTypeKind.CollectionBuilder:
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -768,20 +768,20 @@ private void GenerateImplicitConversionErrorForCollectionExpression(
TypeSymbol targetType,
BindingDiagnosticBag diagnostics)
{
var collectionTypeKind = ConversionsBase.GetCollectionExpressionTypeKind(Compilation, targetType, out var elementType);
var collectionTypeKind = ConversionsBase.GetCollectionExpressionTypeKind(Compilation, targetType, out TypeWithAnnotations elementTypeWithAnnotations);
if (collectionTypeKind == CollectionExpressionTypeKind.CollectionBuilder)
{
Debug.Assert(elementType is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases.
if (!TryGetCollectionIterationType((ExpressionSyntax)node.Syntax, targetType, out TypeWithAnnotations elementTypeWithAnnotations))
Debug.Assert(elementTypeWithAnnotations.Type is null); // GetCollectionExpressionTypeKind() does not set elementType for CollectionBuilder cases.
if (!TryGetCollectionIterationType((ExpressionSyntax)node.Syntax, targetType, out elementTypeWithAnnotations))
{
Error(diagnostics, ErrorCode.ERR_CollectionBuilderNoElementType, node.Syntax, targetType);
return;
}
Debug.Assert(elementTypeWithAnnotations.HasType);
elementType = elementTypeWithAnnotations.Type;
}

bool reportedErrors = false;
var elementType = elementTypeWithAnnotations.Type;

if (collectionTypeKind == CollectionExpressionTypeKind.CollectionInitializer)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ protected override Conversion GetCollectionExpressionConversion(
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
var syntax = node.Syntax;
var collectionTypeKind = GetCollectionExpressionTypeKind(Compilation, targetType, out var elementType);
var collectionTypeKind = GetCollectionExpressionTypeKind(Compilation, targetType, out TypeWithAnnotations elementTypeWithAnnotations);

switch (collectionTypeKind)
{
Expand All @@ -163,16 +163,16 @@ protected override Conversion GetCollectionExpressionConversion(

case CollectionExpressionTypeKind.CollectionBuilder:
{
_binder.TryGetCollectionIterationType((Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax)syntax, targetType, out TypeWithAnnotations elementTypeWithAnnotations);
elementType = elementTypeWithAnnotations.Type;
if (elementType is null)
_binder.TryGetCollectionIterationType((Syntax.ExpressionSyntax)syntax, targetType, out elementTypeWithAnnotations);
if (elementTypeWithAnnotations.Type is null)
{
return Conversion.NoConversion;
}
}
break;
}

var elementType = elementTypeWithAnnotations.Type;
Debug.Assert(collectionTypeKind == CollectionExpressionTypeKind.CollectionInitializer || elementType is { });

if (collectionTypeKind == CollectionExpressionTypeKind.CollectionInitializer)
Expand Down
20 changes: 10 additions & 10 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1624,15 +1624,15 @@ private static bool HasAnonymousFunctionConversion(BoundExpression source, TypeS
return IsAnonymousFunctionCompatibleWithType((UnboundLambda)source, destination, compilation) == LambdaConversionResult.Success;
}

internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSharpCompilation compilation, TypeSymbol destination, out TypeSymbol? elementType)
internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSharpCompilation compilation, TypeSymbol destination, out TypeWithAnnotations elementType)
{
Debug.Assert(compilation is { });

if (destination is ArrayTypeSymbol arrayType)
{
if (arrayType.IsSZArray)
{
elementType = arrayType.ElementType;
elementType = arrayType.ElementTypeWithAnnotations;
return CollectionExpressionTypeKind.Array;
}
}
Expand All @@ -1650,26 +1650,26 @@ internal static CollectionExpressionTypeKind GetCollectionExpressionTypeKind(CSh
}
else if (implementsIEnumerable(compilation, destination))
{
elementType = null;
elementType = default;
return CollectionExpressionTypeKind.CollectionInitializer;
}
else if (isListInterface(compilation, destination, out elementType))
{
return CollectionExpressionTypeKind.ListInterface;
}

elementType = null;
elementType = default;
return CollectionExpressionTypeKind.None;

static bool isSpanType(CSharpCompilation compilation, TypeSymbol targetType, WellKnownType spanType, [NotNullWhen(true)] out TypeSymbol? elementType)
static bool isSpanType(CSharpCompilation compilation, TypeSymbol targetType, WellKnownType spanType, [NotNullWhen(true)] out TypeWithAnnotations elementType)
{
if (targetType is NamedTypeSymbol { Arity: 1 } namedType
&& areEqual(namedType.OriginalDefinition, compilation.GetWellKnownType(spanType)))
{
elementType = namedType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0].Type;
elementType = namedType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0];
return true;
}
elementType = null;
elementType = default;
return false;
}

Expand Down Expand Up @@ -1699,7 +1699,7 @@ static bool implementsIEnumerable(CSharpCompilation compilation, TypeSymbol targ
return allInterfaces.Any(static (a, b) => areEqual(a, b), ienumerableType);
}

static bool isListInterface(CSharpCompilation compilation, TypeSymbol targetType, [NotNullWhen(true)] out TypeSymbol? elementType)
static bool isListInterface(CSharpCompilation compilation, TypeSymbol targetType, [NotNullWhen(true)] out TypeWithAnnotations elementType)
{
if (targetType is NamedTypeSymbol
{
Expand All @@ -1712,10 +1712,10 @@ SpecialType.System_Collections_Generic_ICollection_T or
TypeArgumentsWithAnnotationsNoUseSiteDiagnostics: [var typeArg]
})
{
elementType = typeArg.Type;
elementType = typeArg;
return true;
}
elementType = null;
elementType = default;
return false;
}

Expand Down
40 changes: 29 additions & 11 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,9 @@ private PooledDictionary<BoundExpression, Func<TypeWithAnnotations, TypeWithStat
private static readonly ImmutableArray<BoundKind> s_skippedExpressions = ImmutableArray.Create(BoundKind.ArrayInitialization,
BoundKind.ObjectInitializerExpression,
BoundKind.CollectionInitializerExpression,
BoundKind.DynamicCollectionElementInitializer);
BoundKind.DynamicCollectionElementInitializer,
BoundKind.CollectionExpressionSpreadElement); // https://github.com/dotnet/roslyn/issues/68786: We should handle spreads

#endif

/// <summary>
Expand Down Expand Up @@ -3476,29 +3478,45 @@ protected override void VisitStatement(BoundStatement statement)
// https://github.com/dotnet/roslyn/issues/68786: Use inferInitialObjectState() to set the initial
// state of the instance: see the call to InheritNullableStateOfTrackableStruct() in particular.
int containerSlot = GetOrCreatePlaceholderSlot(node);
bool delayCompletionForType = false; // https://github.com/dotnet/roslyn/issues/68786: Should be true if the collection expression is target typed.
NullableFlowState resultState = NullableFlowState.NotNull;

var collectionKind = ConversionsBase.GetCollectionExpressionTypeKind(this.compilation, node.Type, out var targetElementType);
if (collectionKind is CollectionExpressionTypeKind.CollectionBuilder)
{
var createMethod = node.CollectionBuilderMethod;
if (createMethod is not null)
{
var readOnlySpan = (NamedTypeSymbol)createMethod.Parameters[0].Type;
targetElementType = readOnlySpan.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0];
var annotations = createMethod.GetFlowAnalysisAnnotations();
resultState = ApplyUnconditionalAnnotations(createMethod.ReturnTypeWithAnnotations, annotations).ToTypeWithState().State;
}
}

// https://github.com/dotnet/roslyn/issues/68786: Set nullability of elements from the inferred target type nullability.
foreach (var element in node.Elements)
{
switch (element)
{
case BoundCollectionElementInitializer initializer:
var collectionType = initializer.AddMethod.ContainingType;
var completion = VisitCollectionElementInitializer(initializer, collectionType, delayCompletionForType);
if (completion is { })
{
// https://github.com/dotnet/roslyn/issues/68786: Complete the analysis later.
completion(containerSlot, collectionType);
}

var completion = VisitCollectionElementInitializer(initializer, collectionType,
delayCompletionForType: false /* All collection expressions are target-typed */);

Debug.Assert(completion is null);
break;
case BoundCollectionExpressionSpreadElement spread:
// https://github.com/dotnet/roslyn/issues/68786: We should check the spread
Visit(spread);
break;
default:
VisitRvalue(element);
_ = VisitOptionalImplicitConversion(element, targetElementType, useLegacyWarnings: false, trackMembers: false, AssignmentKind.Assignment);

break;
}
}

SetResultType(node, TypeWithState.Create(node.Type, NullableFlowState.NotNull));
SetResultType(node, TypeWithState.Create(node.Type, resultState));
return null;
}

Expand Down
Loading

0 comments on commit b8d0c85

Please sign in to comment.