diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 693349c0a56ff..dc11aff874323 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -1770,62 +1770,59 @@ private TypeWithState InferResultNullability(BinaryOperatorKind operatorKind, Me protected override void AfterLeftChildHasBeenVisited(BoundBinaryOperator binary) { Debug.Assert(!IsConditionalState); - //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798 + TypeWithState leftType = ResultType; + bool warnOnNullReferenceArgument = (binary.OperatorKind.IsUserDefined() && (object)binary.MethodOpt != null && binary.MethodOpt.ParameterCount == 2); + + if (warnOnNullReferenceArgument) { - TypeWithState leftType = ResultType; - bool warnOnNullReferenceArgument = (binary.OperatorKind.IsUserDefined() && (object)binary.MethodOpt != null && binary.MethodOpt.ParameterCount == 2); + ReportArgumentWarnings(binary.Left, leftType, binary.MethodOpt.Parameters[0]); + } - if (warnOnNullReferenceArgument) - { - ReportArgumentWarnings(binary.Left, leftType, binary.MethodOpt.Parameters[0]); - } + var rightType = VisitRvalueWithState(binary.Right); + Debug.Assert(!IsConditionalState); + // At this point, State.Reachable may be false for + // invalid code such as `s + throw new Exception()`. - var rightType = VisitRvalueWithState(binary.Right); - Debug.Assert(!IsConditionalState); - // At this point, State.Reachable may be false for - // invalid code such as `s + throw new Exception()`. + if (warnOnNullReferenceArgument) + { + ReportArgumentWarnings(binary.Right, rightType, binary.MethodOpt.Parameters[1]); + } - if (warnOnNullReferenceArgument) - { - ReportArgumentWarnings(binary.Right, rightType, binary.MethodOpt.Parameters[1]); - } + Debug.Assert(!IsConditionalState); + ResultType = InferResultNullability(binary, leftType, rightType); - Debug.Assert(!IsConditionalState); - ResultType = InferResultNullability(binary, leftType, rightType); + BinaryOperatorKind op = binary.OperatorKind.Operator(); + if (op == BinaryOperatorKind.Equal || op == BinaryOperatorKind.NotEqual) + { + BoundExpression operandComparedToNull = null; + TypeWithState operandComparedToNullType = default; - BinaryOperatorKind op = binary.OperatorKind.Operator(); - if (op == BinaryOperatorKind.Equal || op == BinaryOperatorKind.NotEqual) + if (binary.Right.ConstantValue?.IsNull == true) { - BoundExpression operandComparedToNull = null; - TypeWithState operandComparedToNullType = default; + operandComparedToNull = binary.Left; + operandComparedToNullType = leftType; + } + else if (binary.Left.ConstantValue?.IsNull == true) + { + operandComparedToNull = binary.Right; + operandComparedToNullType = rightType; + } - if (binary.Right.ConstantValue?.IsNull == true) - { - operandComparedToNull = binary.Left; - operandComparedToNullType = leftType; - } - else if (binary.Left.ConstantValue?.IsNull == true) - { - operandComparedToNull = binary.Right; - operandComparedToNullType = rightType; - } + if (operandComparedToNull != null) + { + // Skip reference conversions + operandComparedToNull = SkipReferenceConversions(operandComparedToNull); - if (operandComparedToNull != null) + // Set all nested conditional slots. For example in a?.b?.c we'll set a, b, and c. + var slotBuilder = ArrayBuilder.GetInstance(); + GetSlotsToMarkAsNotNullable(operandComparedToNull, slotBuilder); + if (slotBuilder.Count != 0) { - // Skip reference conversions - operandComparedToNull = SkipReferenceConversions(operandComparedToNull); - - // Set all nested conditional slots. For example in a?.b?.c we'll set a, b, and c. - var slotBuilder = ArrayBuilder.GetInstance(); - GetSlotsToMarkAsNotNullable(operandComparedToNull, slotBuilder); - if (slotBuilder.Count != 0) - { - Split(); - ref LocalState stateToUpdate = ref (op == BinaryOperatorKind.Equal) ? ref this.StateWhenFalse : ref this.StateWhenTrue; - MarkSlotsAsNotNull(slotBuilder, ref stateToUpdate); - } - slotBuilder.Free(); + Split(); + ref LocalState stateToUpdate = ref (op == BinaryOperatorKind.Equal) ? ref this.StateWhenFalse : ref this.StateWhenTrue; + MarkSlotsAsNotNull(slotBuilder, ref stateToUpdate); } + slotBuilder.Free(); } } } @@ -1858,7 +1855,7 @@ private void GetSlotsToMarkAsNotNullable(BoundExpression operand, ArrayBuilder state) internal void EnsureCapacity(int capacity) { + if (!Reachable) + { + return; + } + if (_state == null) { _state = new ArrayBuilder(capacity); @@ -5687,7 +5670,7 @@ internal NullableFlowState this[int slot] { get { - if (slot < Capacity) + if (slot < Capacity && this.Reachable) { return _state[slot]; } @@ -5696,10 +5679,13 @@ internal NullableFlowState this[int slot] } set { - // https://github.com/dotnet/roslyn/issues/32047 : all variables should be considered not null in unreachable code. - // Moreover, no states should be modified in unreachable code, as there is only one unreachable state. - EnsureCapacity(slot + 1); - _state[slot] = value; + if (this.Reachable) + { + // All variables are be considered not null in unreachable code. + // Moreover, no states should be modified in unreachable code, as there is only one unreachable state. + EnsureCapacity(slot + 1); + _state[slot] = value; + } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index b5f2a0d84832f..09c896fb03c6e 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -6847,7 +6847,7 @@ class B : A where T : A.I comp.VerifyDiagnostics(); } - [Fact] + [Fact, WorkItem(27686, "https://github.com/dotnet/roslyn/issues/27686")] public void AssignObliviousIntoLocals() { var obliviousLib = @" @@ -6871,7 +6871,6 @@ void M() } } "; - // Should a declared type affect an oblivious state? https://github.com/dotnet/roslyn/issues/27686 var compilation = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue(), parseOptions: TestOptions.Regular8, references: new[] { obliviousComp.EmitToImageReference() }); @@ -38870,7 +38869,7 @@ static void G(S? s) ); } - [Fact] + [Fact, WorkItem(28798, "https://github.com/dotnet/roslyn/issues/28798")] public void IsPattern_AffectsNullConditionalOperator_VarPattern() { var source = @@ -38888,8 +38887,6 @@ static void G(string? s) } } }"; - // Should also warn on unreachable code - // https://github.com/dotnet/roslyn/issues/28798 var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue()); comp.VerifyDiagnostics( // (7,13): warning CS8602: Possible dereference of a null reference.