From 318d27fbdc8dd117f3aec63bf7ef24b885eda7ab Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 5 Apr 2021 17:08:32 -0700 Subject: [PATCH 01/16] Learn from bool constants and conditional accesses inside ==/!= --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 174 +++++-- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 490 ++++++++++++++++++ 2 files changed, 628 insertions(+), 36 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index c5707faad9afd..c76d95e21add7 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -121,6 +121,11 @@ internal abstract partial class AbstractFlowPass private readonly bool _nonMonotonicTransfer; + protected void SetConditionalState((TLocalState whenTrue, TLocalState whenFalse) state) + { + SetConditionalState(state.whenTrue, state.whenFalse); + } + protected void SetConditionalState(TLocalState whenTrue, TLocalState whenFalse) { IsConditionalState = true; @@ -2235,20 +2240,103 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperator node) protected virtual void VisitBinaryOperatorChildren(ArrayBuilder stack) { var binary = stack.Pop(); - VisitRvalue(binary.Left); - while (true) + // Only the leftmost operator of a left-associative binary operator chain can learn from a conditional access on the left + // For simplicity, we just special case it here. + if (VisitPossibleConditionalAccess(binary.Left, out var stateWhenNotNull) + && canLearnFromOperator() + && (isNullableValueTypeConversion(binary.Right) || binary.Right.ConstantValue is object)) { VisitRvalue(binary.Right); + Meet(ref stateWhenNotNull, ref State); + var isNullConstant = binary.Right.ConstantValue?.IsNull == true; + SetConditionalState(isNullConstant == isEquals(binary) + ? (State, stateWhenNotNull) + : (stateWhenNotNull, State)); + + if (stack.Count == 0) + { + return; + } + + binary = stack.Pop(); + } + + while (true) + { + if (!canLearnFromOperator() + || !learnFromOperator()) + { + Unsplit(); + Visit(binary.Right); + } if (stack.Count == 0) { break; } - Unsplit(); // VisitRvalue does this binary = stack.Pop(); } + + bool canLearnFromOperator() + { + var kind = binary.OperatorKind; + var op = kind.Operator(); + return op is BinaryOperatorKind.Equal or BinaryOperatorKind.NotEqual && !kind.IsUserDefined(); + } + + static bool isNullableValueTypeConversion(BoundExpression expr) + { + var isNullableConversion = expr is BoundConversion + { + ConversionKind: ConversionKind.ExplicitNullable + or ConversionKind.ImplicitNullable + }; + Debug.Assert(!isNullableConversion || ((BoundConversion)expr).Operand.Type.IsNonNullableValueType()); + return isNullableConversion; + } + + static bool isEquals(BoundBinaryOperator binary) + => binary.OperatorKind.Operator() == BinaryOperatorKind.Equal; + + bool learnFromOperator() + { + if (TryVisitConditionalAccess(binary.Right, out var stateWhenNotNull) + && (isNullableValueTypeConversion(binary.Left) || binary.Left.ConstantValue is object)) + { + var isNullConstant = binary.Left.ConstantValue?.IsNull == true; + SetConditionalState(isNullConstant == isEquals(binary) + ? (State, stateWhenNotNull) + : (stateWhenNotNull, State)); + + return true; + } + else if (IsConditionalState && binary.Right.ConstantValue is { IsBoolean: true } rightConstant) + { + var (stateWhenTrue, stateWhenFalse) = (StateWhenTrue.Clone(), StateWhenFalse.Clone()); + Unsplit(); + Visit(binary.Right); + SetConditionalState(isEquals(binary) == rightConstant.BooleanValue + ? (stateWhenTrue, stateWhenFalse) + : (stateWhenFalse, stateWhenTrue)); + + return true; + } + else if (binary.Left.ConstantValue is { IsBoolean: true } leftConstant) + { + Unsplit(); + Visit(binary.Right); + if (IsConditionalState && isEquals(binary) != leftConstant.BooleanValue) + { + SetConditionalState(StateWhenFalse, StateWhenTrue); + } + + return true; + } + + return false; + } } public override BoundNode VisitUnaryOperator(BoundUnaryOperator node) @@ -2445,9 +2533,17 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node) } else { - TLocalState savedState = VisitPossibleConditionalAccess(node.LeftOperand, out var stateWhenNotNull) - ? stateWhenNotNull - : State.Clone(); + TLocalState savedState; + if (VisitPossibleConditionalAccess(node.LeftOperand, out var stateWhenNotNull)) + { + Debug.Assert(!IsConditionalState); + savedState = stateWhenNotNull; + } + else + { + Unsplit(); + savedState = State.Clone(); + } if (node.LeftOperand.ConstantValue != null) { @@ -2467,41 +2563,28 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node) return null; } - /// - /// Unconditionally visits an expression. - /// If the expression has "state when not null" after visiting, - /// the method returns 'true' and writes the state to . - /// - protected bool VisitPossibleConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) + protected bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) { - EnterRegionIfNeeded(node); - var hasStateWhenNotNull = visit(out stateWhenNotNull); - Debug.Assert(!IsConditionalState); - LeaveRegionIfNeeded(node); - return hasStateWhenNotNull; - - bool visit([NotNullWhen(true)] out TLocalState? stateWhenNotNull) + var access = node switch { - var access = node switch - { - BoundConditionalAccess ca => ca, - BoundConversion { Conversion: Conversion innerConversion, Operand: BoundConditionalAccess ca } when isAcceptableConversion(ca, innerConversion) => ca, - _ => null - }; + BoundConditionalAccess ca => ca, + BoundConversion { Conversion: Conversion innerConversion, Operand: BoundConditionalAccess ca } when isAcceptableConversion(ca, innerConversion) => ca, + _ => null + }; - if (access is not null) - { - return VisitConditionalAccess(access, out stateWhenNotNull); - } - else - { - stateWhenNotNull = default; - VisitWithStackGuard(node); - Unsplit(); - return false; - } + if (access is not null) + { + EnterRegionIfNeeded(access); + Unsplit(); + var hasStateWhenNotNull = VisitConditionalAccess(access, out stateWhenNotNull); + Debug.Assert(!IsConditionalState); + LeaveRegionIfNeeded(access); + return hasStateWhenNotNull; } + stateWhenNotNull = default; + return false; + // "State when not null" can only propagate out of a conditional access if // it is not subject to a user-defined conversion whose parameter is not of a non-nullable value type. static bool isAcceptableConversion(BoundConditionalAccess operand, Conversion conversion) @@ -2524,6 +2607,25 @@ static bool isAcceptableConversion(BoundConditionalAccess operand, Conversion co } } + /// + /// Unconditionally visits an expression. + /// If the expression has "state when not null" after visiting, + /// the method returns 'true' and writes the state to . + /// + protected bool VisitPossibleConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) + { + stateWhenNotNull = default; + if (TryVisitConditionalAccess(node, out stateWhenNotNull)) + { + return true; + } + else + { + Visit(node); + return false; + } + } + private bool VisitConditionalAccess(BoundConditionalAccess node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) { VisitRvalue(node.Receiver); diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index 67cfe75cabcd7..5fea9a6faced3 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -2875,6 +2875,496 @@ void M4(bool b) Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(35, 15)); } + [Theory] + [InlineData("true", "false")] + [InlineData("!false", "!true")] + [InlineData("(true || false)", "(true && false)")] + public void EqualsBoolConstant_01(string @true, string @false) + { + var source = @" +#nullable enable + +class C +{ + public bool M0(out int x, out int y) { x = 42; y = 42; return true; } + + public void M1(bool b) + { + int x, y; + _ = ((b && M0(out x, out y)) == " + @true + @") + ? x.ToString() + : y.ToString(); // 1 + } + + public void M2(bool b) + { + int x, y; + _ = ((b && M0(out x, out y)) != " + @true + @") + ? x.ToString() // 2 + : y.ToString(); + } + + public void M3(bool b) + { + int x, y; + _ = ((b && M0(out x, out y)) == " + @false + @") + ? x.ToString() // 3 + : y.ToString(); + } + + public void M4(bool b) + { + int x, y; + _ = ((b && M0(out x, out y)) != " + @false + @") + ? x.ToString() + : y.ToString(); // 4 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(13, 15), + // (20,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(20, 15), + // (28,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(28, 15), + // (37,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(37, 15) + ); + } + + [Theory] + [InlineData("true", "false")] + [InlineData("!false", "!true")] + [InlineData("(true || false)", "(true && false)")] + public void EqualsBoolConstant_02(string @true, string @false) + { + var source = @" +#nullable enable + +class C +{ + public bool M0(out int x, out int y) { x = 42; y = 42; return true; } + + public void M1(bool b) + { + int x, y; + _ = (" + @true + @" == (b && M0(out x, out y))) + ? x.ToString() + : y.ToString(); // 1 + } + + public void M2(bool b) + { + int x, y; + _ = (" + @true + @" != (b && M0(out x, out y))) + ? x.ToString() // 2 + : y.ToString(); + } + + public void M3(bool b) + { + int x, y; + _ = (" + @false + @" == (b && M0(out x, out y))) + ? x.ToString() // 3 + : y.ToString(); + } + + public void M4(bool b) + { + int x, y; + _ = (" + @false + @" != (b && M0(out x, out y))) + ? x.ToString() + : y.ToString(); // 4 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(13, 15), + // (20,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(20, 15), + // (28,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(28, 15), + // (37,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(37, 15) + ); + } + + [Fact] + public void EqualsBoolConstant_03() + { + var source = @" +#nullable enable + +class C +{ + public bool M0(out int x, out int y) { x = 42; y = 42; return true; } + + public void M1(bool b) + { + int x, y; + _ = ((b && M0(out x, out y)) == true != false) + ? x.ToString() + : y.ToString(); // 1 + } + + public void M2(bool b) + { + int x, y; + _ = (false == (b && M0(out x, out y)) != true) + ? x.ToString() + : y.ToString(); // 2 + } + + public void M3(bool b) + { + int x, y; + _ = (true == false != (b && M0(out x, out y))) + ? x.ToString() + : y.ToString(); // 3 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(13, 15), + // (21,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(21, 15), + // (29,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(29, 15) + ); + } + + [Theory] + [InlineData("b")] + [InlineData("true")] + [InlineData("false")] + public void EqualsCondAccess_01(string operand) + { + var source = @" +#nullable enable + +class C +{ + public bool M0(out int x, out int y) { x = 42; y = 42; return true; } + + public void M1(C? c, bool b) + { + int x, y; + _ = c?.M0(out x, out y) == " + operand + @" + ? x.ToString() + : y.ToString(); // 1 + } + + public void M2(C? c, bool b) + { + int x, y; + _ = c?.M0(out x, out y) != " + operand + @" + ? x.ToString() // 2 + : y.ToString(); + } + + public void M3(C? c, bool b) + { + int x, y; + _ = " + operand + @" == c?.M0(out x, out y) + ? x.ToString() + : y.ToString(); // 3 + } + + public void M4(C? c, bool b) + { + int x, y; + _ = " + operand + @" != c?.M0(out x, out y) + ? x.ToString() // 4 + : y.ToString(); + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(13, 15), + // (20,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(20, 15), + // (29,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(29, 15), + // (36,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(36, 15) + ); + } + + [Theory] + [InlineData("i")] + [InlineData("42")] + public void EqualsCondAccess_02(string operand) + { + var source = @" +#nullable enable + +class C +{ + public int M0(out int x, out int y) { x = 1; y = 2; return 0; } + + public void M1(C? c, int i) + { + int x, y; + _ = c?.M0(out x, out y) == " + operand + @" + ? x.ToString() + : y.ToString(); // 1 + } + + public void M2(C? c, int i) + { + int x, y; + _ = c?.M0(out x, out y) != " + operand + @" + ? x.ToString() // 2 + : y.ToString(); + } + + public void M3(C? c, int i) + { + int x, y; + _ = " + operand + @" == c?.M0(out x, out y) + ? x.ToString() + : y.ToString(); // 3 + } + + public void M4(C? c, int i) + { + int x, y; + _ = " + operand + @" != c?.M0(out x, out y) + ? x.ToString() // 4 + : y.ToString(); + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(13, 15), + // (20,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(20, 15), + // (29,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(29, 15), + // (36,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(36, 15) + ); + } + + [Theory] + [InlineData("object?")] + [InlineData("int?")] + [InlineData("bool?")] + public void EqualsCondAccess_03(string returnType) + { + var source = @" +#nullable enable + +class C +{ + public " + returnType + @" M0(out int x, out int y) { x = 42; y = 42; return null; } + + public void M1(C? c) + { + int x, y; + _ = c?.M0(out x, out y) != null + ? x.ToString() + : y.ToString(); // 1 + } + + public void M2(C? c) + { + int x, y; + _ = c?.M0(out x, out y) == null + ? x.ToString() // 2 + : y.ToString(); + } + + public void M3(C? c) + { + int x, y; + _ = null != c?.M0(out x, out y) + ? x.ToString() + : y.ToString(); // 3 + } + + public void M4(C? c) + { + int x, y; + _ = null == c?.M0(out x, out y) + ? x.ToString() // 4 + : y.ToString(); + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(13, 15), + // (20,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(20, 15), + // (29,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(29, 15), + // (36,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(36, 15) + ); + } + + [Fact] + public void EqualsCondAccess_04() + { + var source = @" +#nullable enable + +class C +{ + public bool M0(out int x, out int y) { x = 42; y = 42; return false; } + + public void M1(C? c, bool b) + { + int x1, y1, x2, y2; + _ = c?.M0(out x1, out y1) == c!.M0(out x2, out y2) + ? x1.ToString() + x2.ToString() + : y1.ToString() + y2.ToString(); // 1 + } + + public void M2(C? c, bool b) + { + int x1, y1, x2, y2; + _ = c?.M0(out x1, out y1) != c!.M0(out x2, out y2) + ? x1.ToString() + x2.ToString() // 2 + : y1.ToString() + y2.ToString(); + } + + public void M3(C? c, bool b) + { + int x1, y1, x2, y2; + _ = c!.M0(out x2, out y2) == c?.M0(out x1, out y1) + ? x1.ToString() + x2.ToString() + : y1.ToString() + y2.ToString(); // 3 + } + + public void M4(C? c, bool b) + { + int x1, y1, x2, y2; + _ = c!.M0(out x2, out y2) != c?.M0(out x1, out y1) + ? x1.ToString() + x2.ToString() // 4 + : y1.ToString() + y2.ToString(); + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,15): error CS0165: Use of unassigned local variable 'y1' + // : y1.ToString() + y2.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y1").WithArguments("y1").WithLocation(13, 15), + // (20,15): error CS0165: Use of unassigned local variable 'x1' + // ? x1.ToString() + x2.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x1").WithArguments("x1").WithLocation(20, 15), + // (29,15): error CS0165: Use of unassigned local variable 'y1' + // : y1.ToString() + y2.ToString(); // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y1").WithArguments("y1").WithLocation(29, 15), + // (36,15): error CS0165: Use of unassigned local variable 'x1' + // ? x1.ToString() + x2.ToString() // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x1").WithArguments("x1").WithLocation(36, 15) + ); + } + + [Fact] + public void EqualsCondAccess_05() + { + var source = @" +#nullable enable + +class C +{ + public static bool operator ==(C? left, C? right) => false; + public static bool operator !=(C? left, C? right) => false; + public override bool Equals(object obj) => false; + public override int GetHashCode() => 0; + + public C? M0(out int x, out int y) { x = 42; y = 42; return this; } + + public void M1(C? c) + { + int x, y; + _ = c?.M0(out x, out y) != null + ? x.ToString() // 1 + : y.ToString(); // 2 + } + + public void M2(C? c) + { + int x, y; + _ = c?.M0(out x, out y) == null + ? x.ToString() // 3 + : y.ToString(); // 4 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (17,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(17, 15), + // (18,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(18, 15), + // (25,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(25, 15), + // (26,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(26, 15) + ); + } + + [Fact] + public void EqualsCondAccess_06() + { + var source = @" +#nullable enable + +class C +{ + public C? M0(out int x, out int y) { x = 42; y = 42; return this; } + + public void M1(C c) + { + int x1, y1, x2, y2; + _ = c.M0(out x1, out y1)?.M0(out x2, out y2) != null + ? x1.ToString() + x2.ToString() + : y1.ToString() + y2.ToString(); // 1 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (13,31): error CS0165: Use of unassigned local variable 'y2' + // : y1.ToString() + y2.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y2").WithArguments("y2").WithLocation(13, 31) + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From a0247c95c4c81caef3434646b4928e256b149b94 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 6 Apr 2021 18:31:42 -0700 Subject: [PATCH 02/16] Handle lifted user defined operators --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 3 +- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 254 ++++++++++++++++++ 2 files changed, 256 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index c76d95e21add7..6d151ff02d17b 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2283,7 +2283,8 @@ bool canLearnFromOperator() { var kind = binary.OperatorKind; var op = kind.Operator(); - return op is BinaryOperatorKind.Equal or BinaryOperatorKind.NotEqual && !kind.IsUserDefined(); + return op is BinaryOperatorKind.Equal or BinaryOperatorKind.NotEqual + && (!kind.IsUserDefined() || kind.IsLifted()); } static bool isNullableValueTypeConversion(BoundExpression expr) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index 5fea9a6faced3..8846f0178078b 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3365,6 +3365,260 @@ public void M1(C c) ); } + [Fact] + public void EqualsCondAccess_07() + { + var source = @" +#nullable enable + +struct S +{ + public static bool operator ==(S left, S right) => false; + public static bool operator !=(S left, S right) => false; + public override bool Equals(object obj) => false; + public override int GetHashCode() => 0; + + public S M0(out int x, out int y) { x = 42; y = 42; return this; } + + public void M1(S? s) + { + int x, y; + _ = s?.M0(out x, out y) != null + ? x.ToString() + : y.ToString(); // 1 + } + + public void M2(S? s) + { + int x, y; + _ = s?.M0(out x, out y) == null + ? x.ToString() // 2 + : y.ToString(); + } + + public void M3(S? s) + { + int x, y; + _ = null != s?.M0(out x, out y) + ? x.ToString() + : y.ToString(); // 3 + } + + public void M4(S? s) + { + int x, y; + _ = null == s?.M0(out x, out y) + ? x.ToString() // 4 + : y.ToString(); + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (18,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(18, 15), + // (25,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(25, 15), + // (34,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(34, 15), + // (41,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(41, 15) + ); + } + + [Fact] + public void EqualsCondAccess_08() + { + var source = @" +#nullable enable + +struct S +{ + public static bool operator ==(S left, S right) => false; + public static bool operator !=(S left, S right) => false; + public override bool Equals(object obj) => false; + public override int GetHashCode() => 0; + + public S M0(out int x, out int y) { x = 42; y = 42; return this; } + + public void M1(S? s) + { + int x, y; + _ = s?.M0(out x, out y) != new S() + ? x.ToString() // 1 + : y.ToString(); + } + + public void M2(S? s) + { + int x, y; + _ = s?.M0(out x, out y) == new S() + ? x.ToString() + : y.ToString(); // 2 + } + + public void M3(S? s) + { + int x, y; + _ = new S() != s?.M0(out x, out y) + ? x.ToString() // 3 + : y.ToString(); + } + + public void M4(S? s) + { + int x, y; + _ = new S() == s?.M0(out x, out y) + ? x.ToString() + : y.ToString(); // 4 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (17,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(17, 15), + // (26,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(26, 15), + // (33,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(33, 15), + // (42,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(42, 15) + ); + } + + [Fact] + public void EqualsCondAccess_09() + { + var source = @" +#nullable enable + +struct S +{ + public static bool operator ==(S left, S right) => false; + public static bool operator !=(S left, S right) => false; + public override bool Equals(object obj) => false; + public override int GetHashCode() => 0; + + public S M0(out int x, out int y) { x = 42; y = 42; return this; } + + public void M1(S? s) + { + int x, y; + _ = s?.M0(out x, out y) != s + ? x.ToString() // 1 + : y.ToString(); // 2 + } + + public void M2(S? s) + { + int x, y; + _ = s?.M0(out x, out y) == s + ? x.ToString() // 3 + : y.ToString(); // 4 + } + + public void M3(S? s) + { + int x, y; + _ = s != s?.M0(out x, out y) + ? x.ToString() // 5 + : y.ToString(); // 6 + } + + public void M4(S? s) + { + int x, y; + _ = s == s?.M0(out x, out y) + ? x.ToString() // 7 + : y.ToString(); // 8 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (17,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(17, 15), + // (18,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(18, 15), + // (25,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(25, 15), + // (26,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(26, 15), + // (33,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 5 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(33, 15), + // (34,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 6 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(34, 15), + // (41,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 7 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(41, 15), + // (42,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 8 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(42, 15) + ); + } + + [Theory] + [InlineData("S? left, S? right")] + [InlineData("S? left, S right")] + public void EqualsCondAccess_10(string operatorParameters) + { + var source = @" +#nullable enable + +struct S +{ + public static bool operator ==(" + operatorParameters + @") => false; + public static bool operator !=(" + operatorParameters + @") => false; + public override bool Equals(object obj) => false; + public override int GetHashCode() => 0; + + public S M0(out int x, out int y) { x = 42; y = 42; return this; } + + public void M1(S? s) + { + int x, y; + _ = s?.M0(out x, out y) != new S() + ? x.ToString() // 1 + : y.ToString(); // 2 + } + + public void M2(S? s) + { + int x, y; + _ = s?.M0(out x, out y) == new S() + ? x.ToString() // 3 + : y.ToString(); // 4 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (17,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(17, 15), + // (18,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(18, 15), + // (25,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(25, 15), + // (26,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(26, 15) + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From 3ab8e0c471f53b0bcb340a6fef6c958b9587aa9a Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 6 Apr 2021 19:02:11 -0700 Subject: [PATCH 03/16] Test user-defined conversion and equality operator --- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index 8846f0178078b..beef994a044c5 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3619,6 +3619,74 @@ public void M2(S? s) ); } + [Fact] + public void EqualsCondAccess_11() + { + var source = @" +#nullable enable + +struct T +{ + public static implicit operator S(T t) => new S(); + public T M0(out int x, out int y) { x = 42; y = 42; return this; } +} + +struct S +{ + public static bool operator ==(S left, S right) => false; + public static bool operator !=(S left, S right) => false; + public override bool Equals(object obj) => false; + public override int GetHashCode() => 0; + + public void M1(T? t) + { + int x, y; + _ = t?.M0(out x, out y) != new S() + ? x.ToString() // 1 + : y.ToString(); + } + + public void M2(T? t) + { + int x, y; + _ = t?.M0(out x, out y) == new S() + ? x.ToString() + : y.ToString(); // 2 + } + + public void M3(T? t) + { + int x, y; + _ = new S() != t?.M0(out x, out y) + ? x.ToString() // 3 + : y.ToString(); + } + + public void M4(T? t) + { + int x, y; + _ = new S() == t?.M0(out x, out y) + ? x.ToString() + : y.ToString(); // 4 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (21,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(21, 15), + // (30,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(30, 15), + // (37,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(37, 15), + // (46,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(46, 15) + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From 37fd3083a81a54aebe0778c81cd491f8f7c31b9d Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 6 Apr 2021 19:11:30 -0700 Subject: [PATCH 04/16] Avoid visiting multiple times --- .../CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index 6d151ff02d17b..f94ca3448ad8c 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2303,8 +2303,8 @@ static bool isEquals(BoundBinaryOperator binary) bool learnFromOperator() { - if (TryVisitConditionalAccess(binary.Right, out var stateWhenNotNull) - && (isNullableValueTypeConversion(binary.Left) || binary.Left.ConstantValue is object)) + if ((isNullableValueTypeConversion(binary.Left) || binary.Left.ConstantValue is object) + && TryVisitConditionalAccess(binary.Right, out var stateWhenNotNull)) { var isNullConstant = binary.Left.ConstantValue?.IsNull == true; SetConditionalState(isNullConstant == isEquals(binary) @@ -2564,7 +2564,7 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node) return null; } - protected bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) + private bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) { var access = node switch { @@ -2613,7 +2613,7 @@ static bool isAcceptableConversion(BoundConditionalAccess operand, Conversion co /// If the expression has "state when not null" after visiting, /// the method returns 'true' and writes the state to . /// - protected bool VisitPossibleConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) + private bool VisitPossibleConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) { stateWhenNotNull = default; if (TryVisitConditionalAccess(node, out stateWhenNotNull)) From ba24a120ea22ff8c71d1530e929fe1965b47bb3b Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 6 Apr 2021 20:05:11 -0700 Subject: [PATCH 05/16] Remove assertion for now --- .../CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index f94ca3448ad8c..ed0e2dddf1bc0 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2294,8 +2294,9 @@ static bool isNullableValueTypeConversion(BoundExpression expr) ConversionKind: ConversionKind.ExplicitNullable or ConversionKind.ImplicitNullable }; - Debug.Assert(!isNullableConversion || ((BoundConversion)expr).Operand.Type.IsNonNullableValueType()); - return isNullableConversion; + // PROTOTYPE: determine why an operand of a nullable conversion is able to be a nullable value type + // Debug.Assert(!isNullableConversion || ((BoundConversion)expr).Operand.Type.IsNonNullableValueType()); + return isNullableConversion && ((BoundConversion)expr).Operand.Type.IsNonNullableValueType(); } static bool isEquals(BoundBinaryOperator binary) From cf68034625cf1e82342e1bd77027322c9f16d0db Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 7 Apr 2021 11:16:54 -0700 Subject: [PATCH 06/16] Cleanup and add a few nullable conversion tests --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 10 +-- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 81 +++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index ed0e2dddf1bc0..59d07874227dc 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2289,14 +2289,10 @@ bool canLearnFromOperator() static bool isNullableValueTypeConversion(BoundExpression expr) { - var isNullableConversion = expr is BoundConversion + return expr is BoundConversion { - ConversionKind: ConversionKind.ExplicitNullable - or ConversionKind.ImplicitNullable - }; - // PROTOTYPE: determine why an operand of a nullable conversion is able to be a nullable value type - // Debug.Assert(!isNullableConversion || ((BoundConversion)expr).Operand.Type.IsNonNullableValueType()); - return isNullableConversion && ((BoundConversion)expr).Operand.Type.IsNonNullableValueType(); + ConversionKind: ConversionKind.ExplicitNullable or ConversionKind.ImplicitNullable, + } conv && conv.Operand.Type.IsNonNullableValueType(); } static bool isEquals(BoundBinaryOperator binary) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index beef994a044c5..1a6d50e2c42c8 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3687,6 +3687,87 @@ public void M4(T? t) ); } + [Fact] + public void EqualsCondAccess_12() + { + var source = @" +#nullable enable + +class C +{ + int? M0(object obj) => null; + void M(C? c, object? obj) + { + int x, y; + _ = (c?.M0(x = y = 0) == (int?)obj) + ? x.ToString() // 1 + : y.ToString(); // 2 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (11,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(11, 15), + // (12,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(12, 15) + ); + } + + [Fact] + public void EqualsCondAccess_13() + { + var source = @" +#nullable enable + +class C +{ + long? M0(object obj) => null; + void M(C? c, int i) + { + int x, y; + _ = (c?.M0(x = y = 0) == i) + ? x.ToString() + : y.ToString(); // 2 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (12,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(12, 15) + ); + } + + [Fact] + public void EqualsCondAccess_14() + { + var source = @" +#nullable enable + +class C +{ + long? M0(object obj) => null; + void M(C? c, int? i) + { + int x, y; + _ = (c?.M0(x = y = 0) == i) + ? x.ToString() // 1 + : y.ToString(); // 2 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (11,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(11, 15), + // (12,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(12, 15) + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From 2751d65bf50e1c5b2e8b5751cc511472af9c641b Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 7 Apr 2021 18:42:44 -0700 Subject: [PATCH 07/16] Update src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs --- src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index 59d07874227dc..f474d06c17089 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2612,7 +2612,6 @@ static bool isAcceptableConversion(BoundConditionalAccess operand, Conversion co /// private bool VisitPossibleConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) { - stateWhenNotNull = default; if (TryVisitConditionalAccess(node, out stateWhenNotNull)) { return true; @@ -3410,4 +3409,3 @@ private void VisitMethodBodies(BoundBlock blockBody, BoundBlock expressionBody) /// internal enum RegionPlace { Before, Inside, After }; } - From 9a94f872af04b9bae932f45ef149bacab22f1d21 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 8 Apr 2021 14:43:17 -0700 Subject: [PATCH 08/16] Address feedback --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 40 +++++---- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 81 +++++++++++++++---- 2 files changed, 92 insertions(+), 29 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index f474d06c17089..0db5f718ef4e7 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2243,9 +2243,11 @@ protected virtual void VisitBinaryOperatorChildren(ArrayBuilder binary.OperatorKind.Operator() == BinaryOperatorKind.Equal; - bool learnFromOperator() + // Returns true if `binary.Right` was visited by the call. + bool learnFromOperator(BoundBinaryOperator binary) { - if ((isNullableValueTypeConversion(binary.Left) || binary.Left.ConstantValue is object) - && TryVisitConditionalAccess(binary.Right, out var stateWhenNotNull)) + // `true == a?.b(out x)` + if (isKnownNullOrNotNull(binary.Left) && TryVisitConditionalAccess(binary.Right, out var stateWhenNotNull)) { var isNullConstant = binary.Left.ConstantValue?.IsNull == true; SetConditionalState(isNullConstant == isEquals(binary) @@ -2310,6 +2313,7 @@ bool learnFromOperator() return true; } + // `a && b(out x) == true` else if (IsConditionalState && binary.Right.ConstantValue is { IsBoolean: true } rightConstant) { var (stateWhenTrue, stateWhenFalse) = (StateWhenTrue.Clone(), StateWhenFalse.Clone()); @@ -2321,6 +2325,7 @@ bool learnFromOperator() return true; } + // `true == a && b(out x)` else if (binary.Left.ConstantValue is { IsBoolean: true } leftConstant) { Unsplit(); @@ -2561,6 +2566,11 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node) return null; } + /// + /// Visits a node only if it is a conditional access. + /// If the expression has "state when not null" after visiting, + /// the method returns 'true' and writes the state to . + /// private bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) { var access = node switch diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index 1a6d50e2c42c8..9afb13df80514 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -293,7 +293,7 @@ public void T100() { { int a; if (fFalse && G(out a)) F(a); } // Error { int a; if (fFalse && G(out a)) F(a); else No(); } // Error { int a; if (fFalse && G(out a)) No(); else F(a); } // Error - { int a; if (fFalse && G(out a)) No(); F(a); } // Error + { int a; if (fFalse && G(out a)) No(); F(a); } // Error { int a; if (fFalse && G(out a)) No(); else No(); F(a); } // Error // Unassigned. Unreachable expr considered assigned. @@ -327,10 +327,10 @@ public void T100() { // Unassigned. G(out a) is unreachable expr. { int a; if (fTrue || G(out a)) F(a); } // Error - { int a; if (fTrue || G(out a)) F(a); else No(); } // Error + { int a; if (fTrue || G(out a)) F(a); else No(); } // Error { int a; if (fTrue || G(out a)) No(); else F(a); } // Error { int a; if (fTrue || G(out a)) No(); F(a); } // Error - { int a; if (fTrue || G(out a)) No(); else No(); F(a); } // Error + { int a; if (fTrue || G(out a)) No(); else No(); F(a); } // Error { int a; if (fTrue || G(out a)) G(out a); else No(); F(a); } // Error // Assigned. @@ -430,7 +430,7 @@ public void T100() { // { int a; if (fFalse && G(out a)) No(); else F(a); } // Error Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(129, 55), // (130,50): error CS0165: Use of unassigned local variable 'a' - // { int a; if (fFalse && G(out a)) No(); F(a); } // Error + // { int a; if (fFalse && G(out a)) No(); F(a); } // Error Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(130, 50), // (131,61): error CS0165: Use of unassigned local variable 'a' // { int a; if (fFalse && G(out a)) No(); else No(); F(a); } // Error @@ -503,7 +503,7 @@ public void T100() { // { int a; if (fTrue || G(out a)) F(a); } // Error Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(163, 43), // (164,43): error CS0165: Use of unassigned local variable 'a' - // { int a; if (fTrue || G(out a)) F(a); else No(); } // Error + // { int a; if (fTrue || G(out a)) F(a); else No(); } // Error Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(164, 43), // Note: Dev10 spuriously reports (165,56,165,57): error CS0165: Use of unassigned local variable 'a' @@ -512,7 +512,7 @@ public void T100() { // { int a; if (fTrue || G(out a)) No(); F(a); } // Error Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(166, 49), // (167,60): error CS0165: Use of unassigned local variable 'a' - // { int a; if (fTrue || G(out a)) No(); else No(); F(a); } // Error + // { int a; if (fTrue || G(out a)) No(); else No(); F(a); } // Error Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(167, 60) // Note: Dev10 spuriously reports (168,66,168,67): error CS0165: Use of unassigned local variable 'a' @@ -634,7 +634,7 @@ public void T120() { if (f) { int a; while (fTrue || G(out a)) F(a); } // Error, unreachable expression if (f) { int a; while (fTrue || G(out a)) No(); F(a); } // Error, unreachable expression, not statement - // Assigned + // Assigned if (f) { int a; while (fFalse || G(out a)) F(a); } if (f) { int a; while (fFalse || G(out a)) No(); F(a); } } @@ -768,7 +768,7 @@ public void T130() { // Unassigned. if (f) { int a; do No(); while (fFalse && G(out a)); F(a); } // Error - // + // if (f) { int a; do No(); while (f || G(out a)); F(a); } // Assigned after false if (f) { int a; do No(); while (fTrue || G(out a)); F(a); } // unreachable expr, unassigned if (f) { int a; do No(); while (fFalse || G(out a)); F(a); } // Assigned @@ -816,7 +816,7 @@ public void T131() { // if (f) { int a; do { break; F(a); } while (f); } // Unreachable Diagnostic(ErrorCode.WRN_UnreachableCode, "F"), - // NOTE: By design, we will not match dev10's report of + // NOTE: By design, we will not match dev10's report of // (77,44,77,48): warning CS0162: Unreachable code detected // See DevDiv #13696. @@ -855,7 +855,7 @@ bool F() } "; - // NOTE: By design, we will not match dev10's report of + // NOTE: By design, we will not match dev10's report of // warning CS0162: Unreachable code detected // See DevDiv #13696. CreateCompilation(source).VerifyDiagnostics(); @@ -1359,7 +1359,7 @@ public void T370() { { int a; F(f ? 1 : 2); F(a); } // Error { int a; F(fFalse ? a : 2); } // no DA error; expr is not reachable - { int a; F(fFalse ? 1 : a); } // + { int a; F(fFalse ? 1 : a); } // { int a; F(fTrue ? a : 2); } // Error - should it also be unreachable? { int a; F(fTrue ? 1 : a); } // BUG: Spec says error. Should this be unreachable? @@ -1535,7 +1535,7 @@ public void T370() { // { int a; F(f ? 1 : 2); F(a); } // Error Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a"), // (164,33): error CS0165: Use of unassigned local variable 'a' - // { int a; F(fFalse ? 1 : a); } // + // { int a; F(fFalse ? 1 : a); } // Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a"), // (166,28): error CS0165: Use of unassigned local variable 'a' // { int a; F(fTrue ? a : 2); } // Error - should it also be unreachable? @@ -3047,6 +3047,59 @@ public void M3(bool b) ); } + [Fact] + public void EqualsBoolConstant_04() + { + var source = @" +#nullable enable + +class C +{ + void M1(object obj) + { + _ = (obj is string x == true) + ? x.ToString() + : x.ToString(); // 1 + } + + void M2(object obj) + { + _ = (obj is string x == false) + ? x.ToString() // 2 + : x.ToString(); + } + + void M3(object obj) + { + _ = (obj is string x != true) + ? x.ToString() // 3 + : x.ToString(); + } + + void M4(object obj) + { + _ = (obj is string x != false) + ? x.ToString() + : x.ToString(); // 4 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (10,15): error CS0165: Use of unassigned local variable 'x' + // : x.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(10, 15), + // (16,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(16, 15), + // (23,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(23, 15), + // (31,15): error CS0165: Use of unassigned local variable 'x' + // : x.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(31, 15) + ); + } + [Theory] [InlineData("b")] [InlineData("true")] @@ -4182,7 +4235,7 @@ static T F(System.ValueType o) public void AssignedInFinallyUsedInTry() { var source = -@" +@" public class Program { static void Main(string[] args) @@ -4192,7 +4245,7 @@ static void Main(string[] args) public static void Test() { - object obj; + object obj; try { From 29d587bbda97fb38a9b26d5eec79bc8e204a3c04 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 8 Apr 2021 15:27:07 -0700 Subject: [PATCH 09/16] Test == with cond access inside cast --- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index 9afb13df80514..4c1c9e2d28095 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3821,6 +3821,31 @@ void M(C? c, int? i) ); } + [Fact] + public void EqualsCondAccess_15() + { + var source = @" +#nullable enable + +class C +{ + C M0(object obj) => this; + void M(C? c) + { + int x, y; + _ = ((object?)c?.M0(x = y = 0) != null) + ? x.ToString() + : y.ToString(); // 1 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (12,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(12, 15) + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From 1514f6428562334a406558fac32423d2aaefcc58 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 8 Apr 2021 18:22:00 -0700 Subject: [PATCH 10/16] Prevent double visit. Test unconditional right. --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 13 +++++------ .../Test/Semantic/FlowAnalysis/FlowTests.cs | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index 0db5f718ef4e7..b4ae580af6daa 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2568,8 +2568,7 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node) /// /// Visits a node only if it is a conditional access. - /// If the expression has "state when not null" after visiting, - /// the method returns 'true' and writes the state to . + /// Returns 'true' if and only if the node was visited. /// private bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) { @@ -2584,10 +2583,10 @@ private bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)] { EnterRegionIfNeeded(access); Unsplit(); - var hasStateWhenNotNull = VisitConditionalAccess(access, out stateWhenNotNull); + VisitConditionalAccess(access, out stateWhenNotNull); Debug.Assert(!IsConditionalState); LeaveRegionIfNeeded(access); - return hasStateWhenNotNull; + return true; } stateWhenNotNull = default; @@ -2633,15 +2632,14 @@ private bool VisitPossibleConditionalAccess(BoundExpression node, [NotNullWhen(t } } - private bool VisitConditionalAccess(BoundConditionalAccess node, [NotNullWhen(true)] out TLocalState? stateWhenNotNull) + private void VisitConditionalAccess(BoundConditionalAccess node, out TLocalState stateWhenNotNull) { VisitRvalue(node.Receiver); if (node.Receiver.ConstantValue != null && !IsConstantNull(node.Receiver)) { VisitRvalue(node.AccessExpression); - stateWhenNotNull = default; - return false; + stateWhenNotNull = this.State.Clone(); } else { @@ -2675,7 +2673,6 @@ private bool VisitConditionalAccess(BoundConditionalAccess node, [NotNullWhen(tr stateWhenNotNull = State; State = savedState; Join(ref State, ref stateWhenNotNull); - return true; } } diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index 4c1c9e2d28095..ec2458f5019ac 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3846,6 +3846,28 @@ void M(C? c) ); } + [Fact] + public void EqualsCondAccess_16() + { + var source = @" +#nullable enable + +class C +{ + C M0(object obj) => this; + void M(C? c) + { + int x, y; + _ = ""a""?.Equals(x = y = 0) == true + ? x.ToString() + : y.ToString(); + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From 426ffeea2597b4e7f3b2b123b7f0a0a167324110 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 8 Apr 2021 18:23:58 -0700 Subject: [PATCH 11/16] simplify --- src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index ec2458f5019ac..d389957060a44 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3854,8 +3854,7 @@ public void EqualsCondAccess_16() class C { - C M0(object obj) => this; - void M(C? c) + void M() { int x, y; _ = ""a""?.Equals(x = y = 0) == true From f2007a626254d4273a7304dfe6151d5f55abcd95 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 9 Apr 2021 14:47:40 -0700 Subject: [PATCH 12/16] Test tuple equality --- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index d389957060a44..d71c965d961c3 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3867,6 +3867,43 @@ void M() ); } + [Fact] + public void EqualsCondAccess_17() + { + var source = @" +#nullable enable + +class C +{ + void M(C? c) + { + int x, y; + _ = (c?.Equals(x = 0), c?.Equals(y = 0)) == (true, true) + ? x.ToString() // 1 + : y.ToString(); // 2 + } +} +"; + // This could be made to work (i.e. removing diagnostic 1) but isn't a high priority scenario. + // The corresponding scenario in nullable also doesn't work: + // void M(string? x, string? y) + // { + // if ((x, y) == ("a", "b")) + // { + // x.ToString(); // warning + // y.ToString(); // warning + // } + // } + CreateCompilation(source).VerifyDiagnostics( + // (10,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(10, 15), + // (11,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(11, 15) + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From b2fd64fcbd90028ebd72c5579aa0379eca1c8d48 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 9 Apr 2021 14:57:21 -0700 Subject: [PATCH 13/16] Add issue reference --- src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index d71c965d961c3..409599bfcde09 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3894,6 +3894,7 @@ void M(C? c) // y.ToString(); // warning // } // } + // https://github.com/dotnet/roslyn/issues/50980 CreateCompilation(source).VerifyDiagnostics( // (10,15): error CS0165: Use of unassigned local variable 'x' // ? x.ToString() // 1 From 5a9c6410322e1e1898a6d8d61086e7980707efeb Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 9 Apr 2021 16:12:00 -0700 Subject: [PATCH 14/16] Address feedback --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 8 ++++-- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 28 +++++++++---------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index b4ae580af6daa..4f2f7ed8ce376 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2237,6 +2237,7 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperator node) stack.Free(); } +#nullable enable protected virtual void VisitBinaryOperatorChildren(ArrayBuilder stack) { var binary = stack.Pop(); @@ -2292,9 +2293,9 @@ static bool isKnownNullOrNotNull(BoundExpression expr) { return expr.ConstantValue is object || (expr is BoundConversion - { - ConversionKind: ConversionKind.ExplicitNullable or ConversionKind.ImplicitNullable, - } conv && conv.Operand.Type.IsNonNullableValueType()); + { + ConversionKind: ConversionKind.ExplicitNullable or ConversionKind.ImplicitNullable, + } conv && conv.Operand.Type!.IsNonNullableValueType()); } static bool isEquals(BoundBinaryOperator binary) @@ -2341,6 +2342,7 @@ bool learnFromOperator(BoundBinaryOperator binary) return false; } } +#nullable disable public override BoundNode VisitUnaryOperator(BoundUnaryOperator node) { diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index 409599bfcde09..ca76be168d083 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3057,46 +3057,46 @@ class C { void M1(object obj) { - _ = (obj is string x == true) + _ = (obj is string x and string y == true) ? x.ToString() - : x.ToString(); // 1 + : y.ToString(); // 1 } void M2(object obj) { - _ = (obj is string x == false) + _ = (obj is string x and string y == false) ? x.ToString() // 2 - : x.ToString(); + : y.ToString(); } void M3(object obj) { - _ = (obj is string x != true) + _ = (obj is string x and string y != true) ? x.ToString() // 3 - : x.ToString(); + : y.ToString(); } void M4(object obj) { - _ = (obj is string x != false) + _ = (obj is string x and string y != false) ? x.ToString() - : x.ToString(); // 4 + : y.ToString(); // 4 } } "; CreateCompilation(source).VerifyDiagnostics( - // (10,15): error CS0165: Use of unassigned local variable 'x' - // : x.ToString(); // 1 - Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(10, 15), + // (10,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(10, 15), // (16,15): error CS0165: Use of unassigned local variable 'x' // ? x.ToString() // 2 Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(16, 15), // (23,15): error CS0165: Use of unassigned local variable 'x' // ? x.ToString() // 3 Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(23, 15), - // (31,15): error CS0165: Use of unassigned local variable 'x' - // : x.ToString(); // 4 - Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(31, 15) + // (31,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(31, 15) ); } From 11d304f5c2a3887da1f24930200559cbd7d73dac Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 9 Apr 2021 17:06:45 -0700 Subject: [PATCH 15/16] Add a few tests --- .../Test/Semantic/FlowAnalysis/FlowTests.cs | 109 +++++++++++++++++- 1 file changed, 104 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index ca76be168d083..0fca9adc9fb06 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -3749,22 +3749,45 @@ public void EqualsCondAccess_12() class C { int? M0(object obj) => null; - void M(C? c, object? obj) + + void M1(C? c, object? obj) { int x, y; _ = (c?.M0(x = y = 0) == (int?)obj) ? x.ToString() // 1 : y.ToString(); // 2 } + + void M2(C? c, object? obj) + { + int x, y; + _ = (c?.M0(x = y = 0) == (int?)null) + ? x.ToString() // 3 + : y.ToString(); + } + + void M3(C? c, object? obj) + { + int x, y; + _ = (c?.M0(x = y = 0) == null) + ? x.ToString() // 4 + : y.ToString(); + } } "; CreateCompilation(source).VerifyDiagnostics( - // (11,15): error CS0165: Use of unassigned local variable 'x' + // (12,15): error CS0165: Use of unassigned local variable 'x' // ? x.ToString() // 1 - Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(11, 15), - // (12,15): error CS0165: Use of unassigned local variable 'y' + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(12, 15), + // (13,15): error CS0165: Use of unassigned local variable 'y' // : y.ToString(); // 2 - Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(12, 15) + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(13, 15), + // (20,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(20, 15), + // (28,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(28, 15) ); } @@ -3905,6 +3928,82 @@ void M(C? c) ); } + [Fact] + public void EqualsCondAccess_18() + { + var source = @" +#nullable enable + +class C +{ + public static bool operator ==(C? left, C? right) => false; + public static bool operator !=(C? left, C? right) => false; + public override bool Equals(object obj) => false; + public override int GetHashCode() => 0; + + public C M0(out int x, out int y) { x = 42; y = 42; return this; } + + public void M1(C? c) + { + int x, y; + _ = c?.M0(out x, out y) != null + ? x.ToString() // 1 + : y.ToString(); // 2 + } + + public void M2(C? c) + { + int x, y; + _ = c?.M0(out x, out y) == null + ? x.ToString() // 3 + : y.ToString(); // 4 + } + + public void M3(C? c) + { + int x, y; + _ = null != c?.M0(out x, out y) + ? x.ToString() // 5 + : y.ToString(); // 6 + } + + public void M4(C? c) + { + int x, y; + _ = null == c?.M0(out x, out y) + ? x.ToString() // 7 + : y.ToString(); // 8 + } +} +"; + CreateCompilation(source).VerifyDiagnostics( + // (17,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 1 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(17, 15), + // (18,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 2 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(18, 15), + // (25,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 3 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(25, 15), + // (26,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 4 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(26, 15), + // (33,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 5 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(33, 15), + // (34,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 6 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(34, 15), + // (41,15): error CS0165: Use of unassigned local variable 'x' + // ? x.ToString() // 7 + Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(41, 15), + // (42,15): error CS0165: Use of unassigned local variable 'y' + // : y.ToString(); // 8 + Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(42, 15) + ); + } + [WorkItem(545352, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545352")] [Fact] public void UseDefViolationInDelegateInSwitchWithGoto() From 4ec1a72b04d55c1c9b51e99a4f0beb0c5a8880cd Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 9 Apr 2021 18:26:21 -0700 Subject: [PATCH 16/16] Update src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs --- .../CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index 4f2f7ed8ce376..2acef1c397f5c 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2292,10 +2292,8 @@ static bool canLearnFromOperator(BoundBinaryOperator binary) static bool isKnownNullOrNotNull(BoundExpression expr) { return expr.ConstantValue is object - || (expr is BoundConversion - { - ConversionKind: ConversionKind.ExplicitNullable or ConversionKind.ImplicitNullable, - } conv && conv.Operand.Type!.IsNonNullableValueType()); + || (expr is BoundConversion { ConversionKind: ConversionKind.ExplicitNullable or ConversionKind.ImplicitNullable } conv + && conv.Operand.Type!.IsNonNullableValueType()); } static bool isEquals(BoundBinaryOperator binary)