Skip to content

Commit 9fc06bb

Browse files
RikkiGibsonRikkiGibson
committed
Do not pass target typed expressions to BestTypeInferrer
1 parent 10f8cc8 commit 9fc06bb

File tree

4 files changed

+195
-10
lines changed

4 files changed

+195
-10
lines changed

‎src/Compilers/CSharp/Portable/Binder/Semantics/BestTypeInferrer.cs‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,20 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
4444
return result;
4545
}
4646

47+
private static bool IsTargetTypedExpression(BoundExpression node)
48+
{
49+
if (node is BoundExpressionWithNullability withNullability)
50+
{
51+
node = withNullability.Expression;
52+
}
53+
54+
return node is BoundConditionalOperator { WasTargetTyped: true } or
55+
BoundConvertedSwitchExpression { WasTargetTyped: true } or
56+
BoundObjectCreationExpressionBase { WasTargetTyped: true } or
57+
BoundDelegateCreationExpression { WasTargetTyped: true } or
58+
BoundCollectionExpression { WasTargetTyped: true };
59+
}
60+
4761
/// <remarks>
4862
/// This method finds the best common type of a set of expressions as per section 7.5.2.14 of the specification.
4963
/// NOTE: If some or all of the expressions have error types, we return error type as the inference result.
@@ -69,6 +83,7 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
6983
HashSet<TypeSymbol> candidateTypes = new HashSet<TypeSymbol>(comparer);
7084
foreach (BoundExpression expr in exprs)
7185
{
86+
Debug.Assert(!IsTargetTypedExpression(expr));
7287
TypeSymbol? type = expr.GetTypeOrFunctionType();
7388

7489
if (type is { })
@@ -132,9 +147,9 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
132147
try
133148
{
134149
var conversionsWithoutNullability = conversions.WithNullability(false);
135-
TypeSymbol? type1 = expr1.Type;
136150

137-
if (type1 is { })
151+
Debug.Assert(!IsTargetTypedExpression(expr1));
152+
if (expr1.Type is { } type1)
138153
{
139154
if (type1.IsErrorType())
140155
{
@@ -148,9 +163,8 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
148163
}
149164
}
150165

151-
TypeSymbol? type2 = expr2.Type;
152-
153-
if (type2 is { })
166+
Debug.Assert(!IsTargetTypedExpression(expr1));
167+
if (expr2.Type is { } type2)
154168
{
155169
if (type2.IsErrorType())
156170
{

‎src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3702,7 +3702,7 @@ TypeWithState convertCollection(BoundCollectionExpression node, TypeWithAnnotati
37023702

37033703
// Record the final state
37043704
NullableFlowState resultState = getResultState(node, collectionKind);
3705-
var resultTypeWithState = TypeWithState.Create(node.Type, resultState);
3705+
var resultTypeWithState = TypeWithState.Create(strippedTargetCollectionType, resultState);
37063706
SetAnalyzedNullability(node, resultTypeWithState);
37073707
return resultTypeWithState;
37083708
}
@@ -4613,7 +4613,11 @@ internal static TypeWithAnnotations BestTypeForLambdaReturns(
46134613
{
46144614
var (returnExpr, resultType, _) = returns[i];
46154615
resultTypes.Add(resultType);
4616-
placeholdersBuilder.Add(CreatePlaceholderIfNecessary(returnExpr, resultType));
4616+
4617+
if (!IsTargetTypedExpression(returnExpr))
4618+
{
4619+
placeholdersBuilder.Add(CreatePlaceholderIfNecessary(returnExpr, resultType));
4620+
}
46174621
}
46184622

46194623
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
@@ -5817,6 +5821,14 @@ void makeAndAdjustReceiverSlot(BoundExpression receiver)
58175821
{
58185822
resultType = null;
58195823
}
5824+
else if (IsTargetTypedExpression(consequence))
5825+
{
5826+
resultType = alternativeRValue.Type;
5827+
}
5828+
else if (IsTargetTypedExpression(alternative))
5829+
{
5830+
resultType = consequenceRValue.Type;
5831+
}
58205832
else
58215833
{
58225834
// Determine nested nullability using BestTypeInferrer.
@@ -9305,7 +9317,11 @@ private void TrackAnalyzedNullabilityThroughConversionGroup(TypeWithState result
93059317
while (conversionOpt != null && conversionOpt != convertedNode)
93069318
{
93079319
Debug.Assert(conversionOpt.ConversionGroupOpt == conversionGroup);
9308-
visitResult = withType(visitResult, conversionOpt.Type);
9320+
// TODO2: what does it mean for these types to differ? Why is it correct to copy over the top-level nullability when the types have differences beyond nullability?
9321+
if (!visitResult.LValueType.Type.Equals(conversionOpt.Type, TypeCompareKind.AllNullableIgnoreOptions))
9322+
{
9323+
visitResult = withType(visitResult, conversionOpt.Type);
9324+
}
93099325
SetAnalyzedNullability(conversionOpt, visitResult);
93109326
conversionOpt = conversionOpt.Operand as BoundConversion;
93119327
}

‎src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -898,8 +898,11 @@ private void VisitSwitchExpressionCore(BoundSwitchExpression node, bool inferTyp
898898
resultTypes.Add(armType);
899899
Join(ref endState, ref this.State);
900900

901-
// Build placeholders for inference in order to preserve annotations.
902-
placeholderBuilder.Add(CreatePlaceholderIfNecessary(expression, armType.ToTypeWithAnnotations(compilation)));
901+
if (!IsTargetTypedExpression(expression))
902+
{
903+
// Build placeholders for inference in order to preserve annotations.
904+
placeholderBuilder.Add(CreatePlaceholderIfNecessary(expression, armType.ToTypeWithAnnotations(compilation)));
905+
}
903906
}
904907

905908
SetState(endState);

‎src/Compilers/CSharp/Test/Symbol/Symbols/Source/NullablePublicAPITests.cs‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Microsoft.CodeAnalysis.Diagnostics;
1515
using Microsoft.CodeAnalysis.Test.Utilities;
1616
using Roslyn.Test.Utilities;
17+
using Roslyn.Utilities;
1718
using Xunit;
1819
using PublicNullableAnnotation = Microsoft.CodeAnalysis.NullableAnnotation;
1920
using PublicNullableFlowState = Microsoft.CodeAnalysis.NullableFlowState;
@@ -5235,5 +5236,156 @@ void test(SemanticModelOptions options)
52355236
Assert.Equal(PublicNullableAnnotation.None, typeInfo.Type.NullableAnnotation);
52365237
}
52375238
}
5239+
5240+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71522")]
5241+
public void CollectionExpression_NestedNullability_01()
5242+
{
5243+
var source = """
5244+
#nullable enable
5245+
5246+
var b = false;
5247+
var arr = b ? ["1"] : new[] { "2" };
5248+
""";
5249+
5250+
var comp = CreateCompilation(source);
5251+
var tree = comp.SyntaxTrees[0];
5252+
var model = comp.GetSemanticModel(tree);
5253+
5254+
var root = tree.GetRoot();
5255+
var collectionExpr = root.DescendantNodes().OfType<CollectionExpressionSyntax>().Single();
5256+
var typeInfo = model.GetTypeInfo(collectionExpr);
5257+
var type = (IArrayTypeSymbol)typeInfo.ConvertedType;
5258+
Assert.Equal("System.String[]", type.ToTestDisplayString());
5259+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.NullableAnnotation);
5260+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.ElementNullableAnnotation);
5261+
}
5262+
5263+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71522")]
5264+
public void CollectionExpression_NestedNullability_02()
5265+
{
5266+
var source = """
5267+
#nullable enable
5268+
5269+
using System;
5270+
using System.Collections.Generic;
5271+
using System.Linq.Expressions;
5272+
5273+
class C
5274+
{
5275+
void M()
5276+
{
5277+
string[] x = ["1"];
5278+
}
5279+
}
5280+
""";
5281+
5282+
var comp = CreateCompilation(source);
5283+
var tree = comp.SyntaxTrees[0];
5284+
var model = comp.GetSemanticModel(tree);
5285+
5286+
var root = tree.GetRoot();
5287+
var collectionExpr = root.DescendantNodes().OfType<CollectionExpressionSyntax>().Single();
5288+
var typeInfo = model.GetTypeInfo(collectionExpr);
5289+
var type = (IArrayTypeSymbol)typeInfo.ConvertedType;
5290+
Assert.Equal("System.String[]", type.ToTestDisplayString());
5291+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.NullableAnnotation);
5292+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.ElementNullableAnnotation);
5293+
}
5294+
5295+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71522")]
5296+
public void CollectionExpression_NestedNullability_03()
5297+
{
5298+
var source = """
5299+
#nullable enable
5300+
5301+
var b = false;
5302+
var arr = b switch { true => ["1"], false => new[] { "2" } };
5303+
""";
5304+
5305+
var comp = CreateCompilation(source);
5306+
var tree = comp.SyntaxTrees[0];
5307+
var model = comp.GetSemanticModel(tree);
5308+
5309+
var root = tree.GetRoot();
5310+
var collectionExpr = root.DescendantNodes().OfType<CollectionExpressionSyntax>().Single();
5311+
var typeInfo = model.GetTypeInfo(collectionExpr);
5312+
var type = (IArrayTypeSymbol)typeInfo.ConvertedType;
5313+
Assert.Equal("System.String[]", type.ToTestDisplayString());
5314+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.NullableAnnotation);
5315+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.ElementNullableAnnotation);
5316+
}
5317+
5318+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71522")]
5319+
public void CollectionExpression_NestedNullability_04()
5320+
{
5321+
var source = """
5322+
#nullable enable
5323+
5324+
var arr = new[] { ["1"], new[] { "2" } };
5325+
""";
5326+
5327+
var comp = CreateCompilation(source);
5328+
var tree = comp.SyntaxTrees[0];
5329+
var model = comp.GetSemanticModel(tree);
5330+
5331+
var root = tree.GetRoot();
5332+
var collectionExpr = root.DescendantNodes().OfType<CollectionExpressionSyntax>().Single();
5333+
var typeInfo = model.GetTypeInfo(collectionExpr);
5334+
var type = (IArrayTypeSymbol)typeInfo.ConvertedType;
5335+
Assert.Equal("System.String[]", type.ToTestDisplayString());
5336+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.NullableAnnotation);
5337+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.ElementNullableAnnotation);
5338+
}
5339+
5340+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71522")]
5341+
public void CollectionExpression_NestedNullability_05()
5342+
{
5343+
var source = """
5344+
#nullable enable
5345+
public class C
5346+
{
5347+
public string[] M1(bool b)
5348+
{
5349+
var arr = b ? ["1"] : new[] { "2" };
5350+
arr[0] = null; // 1
5351+
return arr;
5352+
}
5353+
5354+
public string[] M2(bool b)
5355+
{
5356+
var arr = new[] { "2" };
5357+
arr = b ? ["1"] : arr;
5358+
arr[0] = null; // 2
5359+
return arr;
5360+
}
5361+
}
5362+
""";
5363+
5364+
var comp = CreateCompilation(source);
5365+
comp.VerifyEmitDiagnostics(
5366+
// (7,18): warning CS8625: Cannot convert null literal to non-nullable reference type.
5367+
// arr[0] = null; // 1
5368+
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(7, 18),
5369+
// (15,18): warning CS8625: Cannot convert null literal to non-nullable reference type.
5370+
// arr[0] = null; // 2
5371+
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(15, 18));
5372+
5373+
var tree = comp.SyntaxTrees[0];
5374+
var model = comp.GetSemanticModel(tree);
5375+
5376+
var root = tree.GetRoot();
5377+
var collectionExprs = root.DescendantNodes().OfType<CollectionExpressionSyntax>().ToArray();
5378+
Assert.Equal(2, collectionExprs.Length);
5379+
foreach (var collectionExpr in collectionExprs)
5380+
{
5381+
var typeInfo = model.GetTypeInfo(collectionExpr);
5382+
var type = (IArrayTypeSymbol)typeInfo.ConvertedType;
5383+
Assert.Equal("System.String[]", type.ToTestDisplayString());
5384+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.NullableAnnotation);
5385+
Assert.Equal(PublicNullableAnnotation.NotAnnotated, type.ElementNullableAnnotation);
5386+
}
5387+
}
5388+
5389+
// TODO2: test collection-exprs in lambda returns to exercise all BestTypeInferrer usages in NullableWalker
52385390
}
52395391
}

0 commit comments

Comments
 (0)