Skip to content

Commit

Permalink
Nullable analysis of conditional operator should use best common type…
Browse files Browse the repository at this point in the history 
… unless target typed (#73047)
  • Loading branch information
@cston
cston committed Apr 18, 2024
1 parent e2a1f8f commit 3dd59d6
Show file tree
Hide file tree
Showing 4 changed files with 230 additions and 32 deletions.
7 changes: 1 addition & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4339,23 +4339,18 @@ private BoundExpression BindValueConditionalOperator(ConditionalExpressionSyntax
return new BoundUnconvertedConditionalOperator(node, condition, trueExpr, falseExpr, constantValue, noCommonTypeError, hasErrors: constantValue?.IsBad == true);
}

TypeSymbol type;
bool hasErrors;
if (bestType.IsErrorType())
{
trueExpr = BindToNaturalType(trueExpr, diagnostics, reportNoTargetType: false);
falseExpr = BindToNaturalType(falseExpr, diagnostics, reportNoTargetType: false);
type = bestType;
hasErrors = true;
}
else
{
trueExpr = GenerateConversionForAssignment(bestType, trueExpr, diagnostics);
falseExpr = GenerateConversionForAssignment(bestType, falseExpr, diagnostics);
hasErrors = trueExpr.HasAnyErrors || falseExpr.HasAnyErrors;
// If one of the conversions went wrong (e.g. return type of method group being converted
// didn't match), then we don't want to use bestType because it's not accurate.
type = hasErrors ? CreateErrorType() : bestType;
}

if (!hasErrors)
Expand All @@ -4364,7 +4359,7 @@ private BoundExpression BindValueConditionalOperator(ConditionalExpressionSyntax
hasErrors = constantValue != null && constantValue.IsBad;
}

return new BoundConditionalOperator(node, isRef: false, condition, trueExpr, falseExpr, constantValue, naturalTypeOpt: type, wasTargetTyped: false, type, hasErrors);
return new BoundConditionalOperator(node, isRef: false, condition, trueExpr, falseExpr, constantValue, naturalTypeOpt: bestType, wasTargetTyped: false, bestType, hasErrors);
}
#nullable disable

Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5867,7 +5867,7 @@ void makeAndAdjustReceiverSlot(BoundExpression receiver)

TypeSymbol? resultType;
bool wasTargetTyped = node is BoundConditionalOperator { WasTargetTyped: true };
if (node.HasErrors || wasTargetTyped)
if (wasTargetTyped)
{
resultType = null;
}
Expand Down Expand Up @@ -5895,6 +5895,7 @@ void makeAndAdjustReceiverSlot(BoundExpression receiver)

if (resultType is null)
{
Debug.Assert(!wasTargetTyped);
if (!wasTargetTyped)
{
// This can happen when we're inferring the return type of a lambda or visiting a node without diagnostics like
Expand Down
203 changes: 203 additions & 0 deletions src/Compilers/CSharp/Test/Emit2/Semantics/CollectionExpressionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37997,6 +37997,209 @@ static void Main()
Diagnostic(ErrorCode.ERR_NoTypeDef, "[x, ..y]").WithArguments("MyCollectionA<>", $"{assemblyA}, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null").WithLocation(7, 35));
}

[WorkItem("https://github.com/dotnet/roslyn/issues/72898")]
[Fact]
public void Nullable_ConditionalOperator_Error()
{
string sourceA = """
using System;
using System.Collections;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
[CollectionBuilder(typeof(MyCollectionBuilder), "Create")]
struct MyCollection<T> : IEnumerable<T>
{
IEnumerator<T> IEnumerable<T>.GetEnumerator() => null;
IEnumerator IEnumerable.GetEnumerator() => null;
}
class MyCollectionBuilder
{
public static MyCollection<T> Create<T>(ReadOnlySpan<T> items) => default;
}
""";
string sourceB = """
#nullable enable
using System.Collections.Generic;
class Program
{
static IEnumerable<object> F(bool b, MyCollection<object> x, object y)
{
return b ? x : [y);
}
}
""";

var comp = CreateCompilation([sourceB, sourceA], targetFramework: TargetFramework.Net80);
comp.VerifyEmitDiagnostics(
// (7,26): error CS1003: Syntax error, ',' expected
// return b ? x : [y);
Diagnostic(ErrorCode.ERR_SyntaxError, ")").WithArguments(",").WithLocation(7, 26),
// (7,27): error CS1003: Syntax error, ']' expected
// return b ? x : [y);
Diagnostic(ErrorCode.ERR_SyntaxError, ";").WithArguments("]").WithLocation(7, 27));

var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
var nodes = tree.GetRoot().DescendantNodes();
var expr = nodes.OfType<IdentifierNameSyntax>().Last();
Assert.Equal("y", expr.ToString());
_ = model.GetSymbolInfo(expr);

var conditional = nodes.OfType<ConditionalExpressionSyntax>().Single();
var info = model.GetTypeInfo(conditional);
Assert.Equal("MyCollection<System.Object>", info.Type.ToTestDisplayString());
}

[Fact]
public void Nullable_SwitchExpression_Error()
{
string sourceA = """
using System;
using System.Collections;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
[CollectionBuilder(typeof(MyCollectionBuilder), "Create")]
struct MyCollection<T> : IEnumerable<T>
{
IEnumerator<T> IEnumerable<T>.GetEnumerator() => null;
IEnumerator IEnumerable.GetEnumerator() => null;
}
class MyCollectionBuilder
{
public static MyCollection<T> Create<T>(ReadOnlySpan<T> items) => default;
}
""";
string sourceB = """
#nullable enable
using System.Collections.Generic;
class Program
{
static void F(bool b, MyCollection<object> x, object y)
{
_ = b switch
{
true => x,
false => [y),
};
}
}
""";

var comp = CreateCompilation([sourceB, sourceA], targetFramework: TargetFramework.Net80);
comp.VerifyEmitDiagnostics(
// (10,28): error CS1003: Syntax error, ',' expected
// false => [y),
Diagnostic(ErrorCode.ERR_SyntaxError, ")").WithArguments(",").WithLocation(10, 28),
// (10,30): error CS1003: Syntax error, ']' expected
// false => [y),
Diagnostic(ErrorCode.ERR_SyntaxError, "").WithArguments("]").WithLocation(10, 30));

var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
var expr = tree.GetRoot().DescendantNodes().OfType<IdentifierNameSyntax>().Last();
Assert.Equal("y", expr.ToString());
_ = model.GetSymbolInfo(expr);
}

[Fact]
public void Nullable_ArrayInitializer_Error()
{
string sourceA = """
using System;
using System.Collections;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
[CollectionBuilder(typeof(MyCollectionBuilder), "Create")]
struct MyCollection<T> : IEnumerable<T>
{
IEnumerator<T> IEnumerable<T>.GetEnumerator() => null;
IEnumerator IEnumerable.GetEnumerator() => null;
}
class MyCollectionBuilder
{
public static MyCollection<T> Create<T>(ReadOnlySpan<T> items) => default;
}
""";
string sourceB = """
#nullable enable
using System.Collections.Generic;
class Program
{
static void F(MyCollection<object> x, object y)
{
_ = new[] { x, [y) };
}
}
""";

var comp = CreateCompilation([sourceB, sourceA], targetFramework: TargetFramework.Net80);
comp.VerifyEmitDiagnostics(
// (7,26): error CS1003: Syntax error, ',' expected
// _ = new[] { x, [y) };
Diagnostic(ErrorCode.ERR_SyntaxError, ")").WithArguments(",").WithLocation(7, 26),
// (7,28): error CS1003: Syntax error, ']' expected
// _ = new[] { x, [y) };
Diagnostic(ErrorCode.ERR_SyntaxError, "}").WithArguments("]").WithLocation(7, 28));

var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
var expr = tree.GetRoot().DescendantNodes().OfType<IdentifierNameSyntax>().Last();
Assert.Equal("y", expr.ToString());
_ = model.GetSymbolInfo(expr);
}

[Fact]
public void Nullable_LambdaExpression_Error()
{
string sourceA = """
using System;
using System.Collections;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
[CollectionBuilder(typeof(MyCollectionBuilder), "Create")]
struct MyCollection<T> : IEnumerable<T>
{
IEnumerator<T> IEnumerable<T>.GetEnumerator() => null;
IEnumerator IEnumerable.GetEnumerator() => null;
}
class MyCollectionBuilder
{
public static MyCollection<T> Create<T>(ReadOnlySpan<T> items) => default;
}
""";
string sourceB = """
#nullable enable
using System;
using System.Collections.Generic;
class Program
{
static void F(MyCollection<object> x, object y)
{
var f = (bool b) =>
{
if (b) return x;
return [y);
};
}
}
""";

var comp = CreateCompilation([sourceB, sourceA], targetFramework: TargetFramework.Net80);
comp.VerifyEmitDiagnostics(
// (11,26): error CS1003: Syntax error, ',' expected
// return [y);
Diagnostic(ErrorCode.ERR_SyntaxError, ")").WithArguments(",").WithLocation(11, 26),
// (11,27): error CS1003: Syntax error, ']' expected
// return [y);
Diagnostic(ErrorCode.ERR_SyntaxError, ";").WithArguments("]").WithLocation(11, 27));

var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
var expr = tree.GetRoot().DescendantNodes().OfType<IdentifierNameSyntax>().Last();
Assert.Equal("y", expr.ToString());
_ = model.GetSymbolInfo(expr);
}

[Fact]
public void SynthesizedReadOnlyList_SingleElement()
{
Expand Down
49 changes: 24 additions & 25 deletions src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IFixedStatement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
using System.Linq;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
Expand Down Expand Up @@ -599,7 +600,6 @@ unsafe void M(bool b)
Statements (0)
Next (Regular) Block[B1]
Entering: {R1} {R2}
.locals {R1}
{
Locals: [System.Int32* p]
Expand All @@ -611,89 +611,88 @@ unsafe void M(bool b)
Statements (0)
Jump if False (Regular) to Block[B3]
IParameterReferenceOperation: b (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'b')
Next (Regular) Block[B2]
Block[B2] - Block
Predecessors: [B1]
Statements (1)
IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '&i1')
Value:
Value:
IAddressOfOperation (OperationKind.AddressOf, Type: System.Int32*, IsInvalid) (Syntax: '&i1')
Reference:
Reference:
IFieldReferenceOperation: System.Int32 MyClass.i1 (OperationKind.FieldReference, Type: System.Int32, IsInvalid) (Syntax: 'i1')
Instance Receiver:
Instance Receiver:
IInstanceReferenceOperation (ReferenceKind: ContainingTypeInstance) (OperationKind.InstanceReference, Type: MyClass, IsInvalid, IsImplicit) (Syntax: 'i1')
Next (Regular) Block[B4]
Block[B3] - Block
Predecessors: [B1]
Statements (1)
IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '&i2')
Value:
Value:
IAddressOfOperation (OperationKind.AddressOf, Type: System.Int32*, IsInvalid) (Syntax: '&i2')
Reference:
Reference:
IFieldReferenceOperation: System.Int32 MyClass.i2 (OperationKind.FieldReference, Type: System.Int32, IsInvalid) (Syntax: 'i2')
Instance Receiver:
Instance Receiver:
IInstanceReferenceOperation (ReferenceKind: ContainingTypeInstance) (OperationKind.InstanceReference, Type: MyClass, IsInvalid, IsImplicit) (Syntax: 'i2')
Next (Regular) Block[B4]
Block[B4] - Block
Predecessors: [B2] [B3]
Statements (1)
ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32*, IsInvalid, IsImplicit) (Syntax: 'p = b ? &i1 : &i2')
Left:
Left:
ILocalReferenceOperation: p (IsDeclaration: True) (OperationKind.LocalReference, Type: System.Int32*, IsInvalid, IsImplicit) (Syntax: 'p = b ? &i1 : &i2')
Right:
IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: ?, IsInvalid, IsImplicit) (Syntax: 'b ? &i1 : &i2')
Right:
IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Int32*, IsInvalid, IsImplicit) (Syntax: 'b ? &i1 : &i2')
Next (Regular) Block[B5]
Leaving: {R2}
}
Block[B5] - Block
Predecessors: [B4]
Statements (0)
Jump if False (Regular) to Block[B7]
IParameterReferenceOperation: b (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'b')
Leaving: {R1}
Next (Regular) Block[B6]
Block[B6] - Block
Predecessors: [B5]
Statements (1)
IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null) (Syntax: 'System.Cons ... is {*p}"");')
Expression:
Expression:
IInvocationOperation (void System.Console.WriteLine(System.String value)) (OperationKind.Invocation, Type: System.Void) (Syntax: 'System.Cons ... P is {*p}"")')
Instance Receiver:
Instance Receiver:
null
Arguments(1):
IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: value) (OperationKind.Argument, Type: null) (Syntax: '$""P is {*p}""')
IInterpolatedStringOperation (OperationKind.InterpolatedString, Type: System.String) (Syntax: '$""P is {*p}""')
Parts(2):
IInterpolatedStringTextOperation (OperationKind.InterpolatedStringText, Type: null) (Syntax: 'P is ')
Text:
Text:
ILiteralOperation (OperationKind.Literal, Type: System.String, Constant: ""P is "", IsImplicit) (Syntax: 'P is ')
IInterpolationOperation (OperationKind.Interpolation, Type: null) (Syntax: '{*p}')
Expression:
Expression:
IOperation: (OperationKind.None, Type: System.Int32) (Syntax: '*p')
Children(1):
ILocalReferenceOperation: p (OperationKind.LocalReference, Type: System.Int32*) (Syntax: 'p')
Alignment:
Alignment:
null
FormatString:
FormatString:
null
InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
Next (Regular) Block[B7]
Leaving: {R1}
}
Block[B7] - Exit
Predecessors: [B5] [B6]
Statements (0)
";
VerifyFlowGraphAndDiagnosticsForTest<BlockSyntax>(source, expectedFlowGraph, expectedDiagnostics, compilationOptions: TestOptions.UnsafeDebugDll);
var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugDll);
VerifyFlowGraphAndDiagnosticsForTest<BlockSyntax>(comp, expectedFlowGraph, expectedDiagnostics);

var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
var expr = tree.GetRoot().DescendantNodes().OfType<ConditionalExpressionSyntax>().Single();
var info = model.GetTypeInfo(expr);
Assert.Equal("System.Int32*", info.Type.ToTestDisplayString());
}

[CompilerTrait(CompilerFeature.IOperation, CompilerFeature.Dataflow)]
Expand Down

0 comments on commit 3dd59d6

Please sign in to comment.