From f253208eaa99655c7598ce0f01ef97877397472a Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Mon, 14 Oct 2019 20:40:58 -0700 Subject: [PATCH 01/10] Partial fix for #39220 Fixes #39220 Filter out "bad" slots not a member of the container. Fixes #33428 --- .../Portable/FlowAnalysis/NullableWalker.cs | 117 ++++++++++++++++-- .../Semantics/NullableReferenceTypesTests.cs | 42 +++++++ 2 files changed, 146 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 27e26c8ebeb32..0e8fdf3ce38f9 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -681,6 +681,73 @@ private static void Analyze( } } + /// + /// Check that slots for member symbols indicate members of the enclosing slot's type. + /// + [Conditional("DEBUG")] + private void ValidateLastSlot() + { + int i = nextVariableSlot - 1; + if (i <= 1) + return; + + var variable = variableBySlot[i].Symbol; + var containingSlot = variableBySlot[i].ContainingSlot; + if (containingSlot < 1) + return; + + var containingSlotType = variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type; + Debug.Assert(IsMember(variable, containingSlotType)); + } + + static bool IsMember(Symbol member, TypeSymbol possibleContainer) + { + TypeSymbol memberContainer = member.ContainingType; + if (memberContainer.Equals(possibleContainer, TypeCompareKind.AllIgnoreOptions)) + return true; + + switch (memberContainer.TypeKind) + { + case TypeKind.Interface: + { + var allInterfaces = possibleContainer switch + { + // interface members are only inherited into interfaces and type parameters. + TypeParameterSymbol typeParameter => typeParameter.AllEffectiveInterfacesNoUseSiteDiagnostics, + { TypeKind: TypeKind.Interface } => possibleContainer.AllInterfacesNoUseSiteDiagnostics, + _ => ImmutableArray.Empty, + }; + + foreach (var possibleContainerBase in allInterfaces) + { + if (memberContainer.Equals(possibleContainerBase, TypeCompareKind.AllIgnoreOptions)) + return true; + } + + return false; + } + default: + { + var effectiveBase = possibleContainer switch + { + // interface members are only inherited into interfaces and type parameters. + TypeParameterSymbol typeParameter => typeParameter.EffectiveBaseClassNoUseSiteDiagnostics, + _ => possibleContainer.BaseTypeNoUseSiteDiagnostics, + }; + + for (var possibleContainerBase = effectiveBase; + !(possibleContainerBase is null); + possibleContainerBase = possibleContainerBase.BaseTypeNoUseSiteDiagnostics) + { + if (memberContainer.Equals(possibleContainerBase, TypeCompareKind.AllIgnoreOptions)) + return true; + } + + return false; + } + } + } + private SharedWalkerState SaveSharedState() => new SharedWalkerState( _variableSlot.ToImmutableDictionary(), @@ -1181,7 +1248,7 @@ private void TrackNullableStateForAssignment( var newState = valueType.State; SetStateAndTrackForFinally(ref this.State, targetSlot, newState); - InheritDefaultState(targetSlot); + InheritDefaultState(targetType.Type, targetSlot); // https://github.com/dotnet/roslyn/issues/33428: Can the areEquivalentTypes check be removed // if InheritNullableStateOfMember asserts the member is valid for target and value? @@ -1254,7 +1321,11 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont { Debug.Assert(targetContainerSlot > 0); Debug.Assert(skipSlot > 0); - // https://github.com/dotnet/roslyn/issues/33428: Ensure member is valid for target and value. + + // Ensure member is valid for target and value. + if (!IsMember(member, variableBySlot[targetContainerSlot].Symbol.GetTypeOrReturnType().Type)) + return; + // PROTOTYPE(ngafter): The above line is a good place to set a breakpoint to diagnose this set of changes. TypeWithAnnotations fieldOrPropertyType = member.GetTypeOrReturnType(); @@ -1263,6 +1334,7 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont fieldOrPropertyType.IsNullableType()) { int targetMemberSlot = GetOrCreateSlot(member, targetContainerSlot); + ValidateLastSlot(); if (targetMemberSlot > 0) { NullableFlowState value = isDefaultValue ? NullableFlowState.MaybeNull : fieldOrPropertyType.ToTypeWithState().State; @@ -1290,9 +1362,11 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont else if (EmptyStructTypeCache.IsTrackableStructType(fieldOrPropertyType.Type)) { int targetMemberSlot = GetOrCreateSlot(member, targetContainerSlot); + ValidateLastSlot(); if (targetMemberSlot > 0) { int valueMemberSlot = (valueContainerSlot > 0) ? GetOrCreateSlot(member, valueContainerSlot) : -1; + ValidateLastSlot(); if (valueMemberSlot == skipSlot) { return; @@ -1319,7 +1393,7 @@ private void SetStateAndTrackForFinally(ref LocalState state, int slot, Nullable } } - private void InheritDefaultState(int targetSlot) + private void InheritDefaultState(TypeSymbol targetType, int targetSlot) { Debug.Assert(targetSlot > 0); @@ -1328,12 +1402,11 @@ private void InheritDefaultState(int targetSlot) { var variable = variableBySlot[slot]; if (variable.ContainingSlot != targetSlot) - { continue; - } - SetStateAndTrackForFinally(ref this.State, slot, GetDefaultState(variable.Symbol)); - InheritDefaultState(slot); + var symbol = AsMemberOfType(targetType, variable.Symbol); + SetStateAndTrackForFinally(ref this.State, slot, GetDefaultState(symbol)); + InheritDefaultState(symbol.GetTypeOrReturnType().Type, slot); } } @@ -1607,7 +1680,7 @@ private void DeclareLocal(LocalSymbol local) if (slot > 0) { PopulateOneSlot(ref this.State, slot); - InheritDefaultState(slot); + InheritDefaultState(_variableTypes.TryGetValue(local, out var typeWithAnootations) ? typeWithAnootations.Type : local.Type, slot); } } } @@ -1953,7 +2026,12 @@ private void VisitObjectElementInitializer(int containingSlot, BoundAssignmentOp if ((object)symbol != null) { - int slot = (containingSlot < 0) ? -1 : GetOrCreateSlot(symbol, containingSlot); + if (containingSlot > 0 && !IsMember(symbol, base.variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type)) + { + // PROTOTYPE(ngafter): This block is here to help0 debug things. + } + int slot = (containingSlot < 0 || !IsMember(symbol, base.variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type)) ? -1 : GetOrCreateSlot(symbol, containingSlot); + ValidateLastSlot(); VisitObjectCreationInitializer(symbol, slot, node.Right, GetLValueAnnotations(node.Left)); // https://github.com/dotnet/roslyn/issues/35040: Should likely be setting _resultType in VisitObjectCreationInitializer // and using that value instead of reconstructing here @@ -2073,7 +2151,9 @@ public override BoundNode VisitAnonymousObjectCreationExpression(BoundAnonymousO var argument = arguments[i]; var argumentType = argumentTypes[i]; var property = AnonymousTypeManager.GetAnonymousTypeProperty(anonymousType, i); - TrackNullableStateForAssignment(argument, property.TypeWithAnnotations, GetOrCreateSlot(property, receiverSlot), argumentType, MakeSlot(argument)); + var slot = GetOrCreateSlot(property, receiverSlot); + ValidateLastSlot(); + TrackNullableStateForAssignment(argument, property.TypeWithAnnotations, slot, argumentType, MakeSlot(argument)); var currentDeclaration = getDeclaration(node, property, ref currentDeclarationIndex); if (currentDeclaration is object) @@ -4759,8 +4839,12 @@ private void TrackNullableStateOfTupleElements( } } - void trackState(BoundExpression value, FieldSymbol field, TypeWithState valueType) => - TrackNullableStateForAssignment(value, field.TypeWithAnnotations, GetOrCreateSlot(field, slot), valueType, MakeSlot(value)); + void trackState(BoundExpression value, FieldSymbol field, TypeWithState valueType) + { + int targetSlot = GetOrCreateSlot(field, slot); + ValidateLastSlot(); + TrackNullableStateForAssignment(value, field.TypeWithAnnotations, targetSlot, valueType, MakeSlot(value)); + } } private void TrackNullableStateOfNullableValue(int containingSlot, TypeSymbol containingType, BoundExpression value, TypeWithState valueType, int valueSlot) @@ -4851,10 +4935,12 @@ void trackConvertedValue(FieldSymbol targetField, Conversion conversion, FieldSy case ConversionKind.ExplicitTuple: { int targetFieldSlot = GetOrCreateSlot(targetField, slot); + ValidateLastSlot(); if (targetFieldSlot > 0) { this.State[targetFieldSlot] = NullableFlowState.NotNull; int valueFieldSlot = GetOrCreateSlot(valueField, valueSlot); + ValidateLastSlot(); if (valueFieldSlot > 0) { TrackNullableStateOfTupleConversion(conversionOpt, convertedNode, conversion, targetField.Type, valueField.Type, targetFieldSlot, valueFieldSlot, assignmentKind, parameterOpt, reportWarnings); @@ -4868,10 +4954,12 @@ void trackConvertedValue(FieldSymbol targetField, Conversion conversion, FieldSy if (AreNullableAndUnderlyingTypes(targetField.Type, valueField.Type, out _)) { int targetFieldSlot = GetOrCreateSlot(targetField, slot); + ValidateLastSlot(); if (targetFieldSlot > 0) { this.State[targetFieldSlot] = NullableFlowState.NotNull; int valueFieldSlot = GetOrCreateSlot(valueField, valueSlot); + ValidateLastSlot(); if (valueFieldSlot > 0) { TrackNullableStateOfNullableValue(targetFieldSlot, targetField.Type, null, valueField.TypeWithAnnotations.ToTypeWithState(), valueFieldSlot); @@ -4895,6 +4983,7 @@ void trackConvertedValue(FieldSymbol targetField, Conversion conversion, FieldSy reportRemainingWarnings: reportWarnings, diagnosticLocation: (conversionOpt ?? convertedNode).Syntax.GetLocation()); int targetFieldSlot = GetOrCreateSlot(targetField, slot); + ValidateLastSlot(); if (targetFieldSlot > 0) { this.State[targetFieldSlot] = convertedType.State; @@ -6750,7 +6839,9 @@ private int GetNullableOfTValueSlot(TypeSymbol containingType, int containingSlo var getValue = (MethodSymbol)compilation.GetSpecialTypeMember(SpecialMember.System_Nullable_T_get_Value); valueProperty = getValue?.AsMember((NamedTypeSymbol)containingType)?.AssociatedSymbol; - return (valueProperty is null) ? -1 : GetOrCreateSlot(valueProperty, containingSlot, forceSlotEvenIfEmpty: forceSlotEvenIfEmpty); + var result = (valueProperty is null) ? -1 : GetOrCreateSlot(valueProperty, containingSlot, forceSlotEvenIfEmpty: forceSlotEvenIfEmpty); + ValidateLastSlot(); + return result; } protected override void VisitForEachExpression(BoundForEachStatement node) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 14829742c1ae9..2d226a1d71897 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -120135,5 +120135,47 @@ public void M7(C? a, bool b) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "a").WithLocation(36, 9) ); } + + [Fact] + [WorkItem(39220, "https://github.com/dotnet/roslyn/issues/39220")] + public void GotoMayCauseAnotherAnalysisPass() + { + var source = +@"#nullable enable +class Program +{ + static void Test(string? s) + { + if (s == null) return; + + heck: + var c = GetC(s); + var prop = c.Property; + prop.ToString(); // BOOM (after goto) + s = null; + goto heck; + } + + static void Main() + { + Test(""""); + } + + public static C GetC(T t) => new C(t); +} + +class C +{ + public C(T t) => Property = t; + public T Property { get; } +} +"; + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (11,9): warning CS8602: Dereference of a possibly null reference. + // prop.ToString(); // BOOM (after goto) + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "prop").WithLocation(11, 9) + ); + } } } From 15a06b875b7ebe576d8a60012cd946df86adbf99 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Mon, 6 Jan 2020 15:20:59 -0800 Subject: [PATCH 02/10] Adjust tests. --- .../Portable/FlowAnalysis/NullableWalker.cs | 21 +- .../Semantics/NullableReferenceTypesTests.cs | 224 +++++++++--------- 2 files changed, 125 insertions(+), 120 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 0e8fdf3ce38f9..bd809d8805786 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2151,15 +2151,20 @@ public override BoundNode VisitAnonymousObjectCreationExpression(BoundAnonymousO var argument = arguments[i]; var argumentType = argumentTypes[i]; var property = AnonymousTypeManager.GetAnonymousTypeProperty(anonymousType, i); - var slot = GetOrCreateSlot(property, receiverSlot); - ValidateLastSlot(); - TrackNullableStateForAssignment(argument, property.TypeWithAnnotations, slot, argumentType, MakeSlot(argument)); - - var currentDeclaration = getDeclaration(node, property, ref currentDeclarationIndex); - if (currentDeclaration is object) + if (property.Type.SpecialType != SpecialType.System_Void) { - TakeIncrementalSnapshot(currentDeclaration); - SetAnalyzedNullability(currentDeclaration, new VisitResult(argumentType, property.TypeWithAnnotations)); + // A void element results in an error type in the anonymous type but not in the property's container! + // To avoid failing an assertion later, we skip them. + var slot = GetOrCreateSlot(property, receiverSlot); + ValidateLastSlot(); + TrackNullableStateForAssignment(argument, property.TypeWithAnnotations, slot, argumentType, MakeSlot(argument)); + + var currentDeclaration = getDeclaration(node, property, ref currentDeclarationIndex); + if (currentDeclaration is object) + { + TakeIncrementalSnapshot(currentDeclaration); + SetAnalyzedNullability(currentDeclaration, new VisitResult(argumentType, property.TypeWithAnnotations)); + } } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 2d226a1d71897..1cf608d0fbfa6 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -59834,17 +59834,17 @@ static void F(string x, string? y) t.Item2.ToString(); // 2 u.Item1.ToString(); u.Item2.ToString(); // 3 - v.Item1.ToString(); - v.Item2.ToString(); // 4 + v.Item1.ToString(); // 4 + v.Item2.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); comp.VerifyDiagnostics( // (5,30): warning CS8619: Nullability of reference types in value of type '(object x, string? y)' doesn't match target type '(object, string)'. - // (object, string) t = ((object, string))(x, y)/*T:(object! x, string? y)*/; // 1 + // (object, string) t = ((object, string))(x, y)/*T:(string! x, string? y)*/; // 1 Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "((object, string))(x, y)").WithArguments("(object x, string? y)", "(object, string)").WithLocation(5, 30), // (5,52): warning CS8600: Converting null literal or possible null value to non-nullable type. - // (object, string) t = ((object, string))(x, y)/*T:(object! x, string? y)*/; // 1 + // (object, string) t = ((object, string))(x, y)/*T:(string! x, string? y)*/; // 1 Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "y").WithLocation(5, 52), // (9,9): warning CS8602: Dereference of a possibly null reference. // t.Item2.ToString(); // 2 @@ -59852,9 +59852,9 @@ static void F(string x, string? y) // (11,9): warning CS8602: Dereference of a possibly null reference. // u.Item2.ToString(); // 3 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.Item2").WithLocation(11, 9), - // (13,9): warning CS8602: Dereference of a possibly null reference. - // v.Item2.ToString(); // 4 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "v.Item2").WithLocation(13, 9)); + // (12,9): warning CS8602: Dereference of a possibly null reference. + // v.Item1.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "v.Item1").WithLocation(12, 9)); comp.VerifyTypes(); } @@ -61768,15 +61768,18 @@ static void F(string x, string? y) (object? a, object? b) u = t; t.Item1.ToString(); t.Item2.ToString(); // 1 - u.a.ToString(); - u.b.ToString(); // 2 + u.a.ToString(); // 2 + u.b.ToString(); // 3 } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); comp.VerifyDiagnostics( // (8,9): warning CS8602: Dereference of a possibly null reference. - // t.Item2.ToString(); // 2 + // t.Item2.ToString(); // 1 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t.Item2").WithLocation(8, 9), + // (9,9): warning CS8602: Dereference of a possibly null reference. + // u.a.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.a").WithLocation(9, 9), // (10,9): warning CS8602: Dereference of a possibly null reference. // u.b.ToString(); // 3 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.b").WithLocation(10, 9)); @@ -68706,17 +68709,14 @@ static void F() a.FA.ToString(); a = new B() { FA = 2, FB = null }; // 1 ((B)a).FA.ToString(); - ((B)a).FB.ToString(); // 2 + ((B)a).FB.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); comp.VerifyDiagnostics( // (15,36): warning CS8625: Cannot convert null literal to non-nullable reference type. // a = new B() { FA = 2, FB = null }; // 1 - Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(15, 36), - // (17,9): warning CS8602: Dereference of a possibly null reference. - // ((B)a).FB.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((B)a).FB").WithLocation(17, 9)); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(15, 36)); } [Fact] @@ -68741,17 +68741,14 @@ static void F() a.FA.ToString(); a = new B() { FB = null }; // 1 b = (B)a; - b.FB.ToString(); // 2 + b.FB.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); comp.VerifyDiagnostics( // (16,28): warning CS8625: Cannot convert null literal to non-nullable reference type. // a = new B() { FB = null }; // 1 - Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(16, 28), - // (18,9): warning CS8602: Dereference of a possibly null reference. - // b.FB.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.FB").WithLocation(18, 9)); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(16, 28)); } [Fact] @@ -68808,10 +68805,10 @@ static void F(IB b) b.PB = null; // 1 object o = b; b = o; // 2 - b.PA.ToString(); + b.PA.ToString(); // 3 b = (IB)o; - b.PB.ToString(); // 3 - ((IB)o).PA.ToString(); + b.PB.ToString(); + ((IB)o).PA.ToString(); // 4 } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -68822,9 +68819,12 @@ static void F(IB b) // (16,13): error CS0266: Cannot implicitly convert type 'object' to 'IB'. An explicit conversion exists (are you missing a cast?) // b = o; // 2 Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "o").WithArguments("object", "IB").WithLocation(16, 13), - // (19,9): warning CS8602: Dereference of a possibly null reference. - // b.PB.ToString(); // 3 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.PB").WithLocation(19, 9)); + // (17,9): warning CS8602: Dereference of a possibly null reference. + // b.PA.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.PA").WithLocation(17, 9), + // (20,9): warning CS8602: Dereference of a possibly null reference. + // ((IB)o).PA.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((IB)o).PA").WithLocation(20, 9)); } [Fact] @@ -69242,17 +69242,14 @@ static void F() (B, object) t = (new B() { FA = 1 }, new B() { FB = null }); // 1 t.Item1.FA.ToString(); ((A)t.Item1).FA.ToString(); - ((B)t.Item2).FB.ToString(); // 2 + ((B)t.Item2).FB.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); comp.VerifyDiagnostics( // (13,61): warning CS8625: Cannot convert null literal to non-nullable reference type. // (B, object) t = (new B() { FA = 1 }, new B() { FB = null }); // 1 - Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(13, 61), - // (16,9): warning CS8602: Dereference of a possibly null reference. - // ((B)t.Item2).FB.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((B)t.Item2).FB").WithLocation(16, 9)); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(13, 61)); } [Fact] @@ -69273,8 +69270,8 @@ class Program static void F() { (B, object) t = (new B() { FA = 1 }, new B() { FB = null }); // 1 - (((A, A))t).Item1.FA.ToString(); - (((B, B))t).Item2.FB.ToString(); // 2 + (((A, A))t).Item1.FA.ToString(); // 2 + (((B, B))t).Item2.FB.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -69282,9 +69279,9 @@ static void F() // (13,61): warning CS8625: Cannot convert null literal to non-nullable reference type. // (B, object) t = (new B() { FA = 1 }, new B() { FB = null }); // 1 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(13, 61), - // (15,9): warning CS8602: Dereference of a possibly null reference. - // (((B, B))t).Item2.FB.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((B, B))t).Item2.FB").WithLocation(15, 9)); + // (14,9): warning CS8602: Dereference of a possibly null reference. + // (((A, A))t).Item1.FA.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((A, A))t).Item1.FA").WithLocation(14, 9)); } [Fact] @@ -69314,11 +69311,11 @@ static void F() t.Item2.Item1.FA.ToString(); (A, (B, B)) u; u = t; // 4 - u.Item1.FA.ToString(); - u.Item2.Item2.FB.ToString(); // 5 + u.Item1.FA.ToString(); // 5 + u.Item2.Item2.FB.ToString(); u = ((A, (B, B)))t; - u.Item1.FA.ToString(); - u.Item2.Item2.FB.ToString(); // 6 + u.Item1.FA.ToString(); // 6 + u.Item2.Item2.FB.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -69335,12 +69332,12 @@ static void F() // (22,13): error CS0266: Cannot implicitly convert type '(B, (A, A))' to '(A, (B, B))'. An explicit conversion exists (are you missing a cast?) // u = t; // 4 Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "t").WithArguments("(B, (A, A))", "(A, (B, B))").WithLocation(22, 13), - // (24,9): warning CS8602: Dereference of a possibly null reference. - // u.Item2.Item2.FB.ToString(); // 5 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.Item2.Item2.FB").WithLocation(24, 9), - // (27,9): warning CS8602: Dereference of a possibly null reference. - // u.Item2.Item2.FB.ToString(); // 6 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.Item2.Item2.FB").WithLocation(27, 9)); + // (23,9): warning CS8602: Dereference of a possibly null reference. + // u.Item1.FA.ToString(); // 5 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.Item1.FA").WithLocation(23, 9), + // (26,9): warning CS8602: Dereference of a possibly null reference. + // u.Item1.FA.ToString(); // 6 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.Item1.FA").WithLocation(26, 9)); } [Fact] @@ -69358,17 +69355,20 @@ class Program static void F() { (object, object)? t = (new C() { F = 1 }, new object()); - (((C, object))t).Item1.F.ToString(); + (((C, object))t).Item1.F.ToString(); // 1 (object, object)? u = (new C() { F = 2 }, new object()); - ((C)u.Value.Item1).F.ToString(); + ((C)u.Value.Item1).F.ToString(); // 2 } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); // https://github.com/dotnet/roslyn/issues/34086: Track state across Nullable conversions with nested conversions. comp.VerifyDiagnostics( - // (10,9): warning CS8602: Dereference of a possibly null reference. - // (((C, object))t).Item1.F.ToString(); - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((C, object))t).Item1.F").WithLocation(10, 9)); + // (10,9): warning CS8602: Dereference of a possibly null reference. + // (((C, object))t).Item1.F.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((C, object))t).Item1.F").WithLocation(10, 9), + // (12,9): warning CS8602: Dereference of a possibly null reference. + // ((C)u.Value.Item1).F.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((C)u.Value.Item1).F").WithLocation(12, 9)); } [Fact] @@ -69406,9 +69406,6 @@ static void F() // (11,10): warning CS8619: Nullability of reference types in value of type '(S?, object?)' doesn't match target type '(S, S)'. // (((S, S))t).Item1.F.ToString(); Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "((S, S))t").WithArguments("(S?, object?)", "(S, S)").WithLocation(11, 10), - // (12,9): warning CS8602: Possible dereference of a null reference. - // (((S, S))t).Item2.G.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((S, S))t).Item2.G").WithLocation(12, 9), // (12,10): warning CS8619: Nullability of reference types in value of type '(S?, object?)' doesn't match target type '(S, S)'. // (((S, S))t).Item2.G.ToString(); // 2 Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "((S, S))t").WithArguments("(S?, object?)", "(S, S)").WithLocation(12, 10), @@ -69427,14 +69424,14 @@ public void Conversions_TupleConversions_06() static void F1((object?, object) t1) { var u1 = ((string, string))t1; // 1 - u1.Item1.ToString(); // 2 + u1.Item1.ToString(); u1.Item1.ToString(); } static void F2((object?, object) t2) { var u2 = ((string?, string?))t2; - u2.Item1.ToString(); // 3 - u2.Item2.ToString(); + u2.Item1.ToString(); // 2 + u2.Item2.ToString(); // 3 } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -69442,12 +69439,12 @@ static void F2((object?, object) t2) // (5,18): warning CS8619: Nullability of reference types in value of type '(object?, object)' doesn't match target type '(string, string)'. // var u1 = ((string, string))t1; // 1 Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "((string, string))t1").WithArguments("(object?, object)", "(string, string)").WithLocation(5, 18), - // (6,9): warning CS8602: Dereference of a possibly null reference. - // u1.Item1.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u1.Item1").WithLocation(6, 9), // (12,9): warning CS8602: Dereference of a possibly null reference. - // u2.Item1.ToString(); // 3 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u2.Item1").WithLocation(12, 9)); + // u2.Item1.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u2.Item1").WithLocation(12, 9), + // (13,9): warning CS8602: Dereference of a possibly null reference. + // u2.Item2.ToString(); /// 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u2.Item2").WithLocation(13, 9)); } [Fact] @@ -69918,8 +69915,8 @@ class Program static void F() { object o = new S() { F = 1, G = null }; // 1 - ((S)o).F.ToString(); - ((S)o).G.ToString(); // 2 + ((S)o).F.ToString(); // 2 + ((S)o).G.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -69927,9 +69924,9 @@ static void F() // (10,41): warning CS8625: Cannot convert null literal to non-nullable reference type. // object o = new S() { F = 1, G = null }; // 1 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 41), - // (12,9): warning CS8602: Dereference of a possibly null reference. - // ((S)o).G.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((S)o).G").WithLocation(12, 9)); + // (11,9): warning CS8602: Dereference of a possibly null reference. + // ((S)o).F.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((S)o).F").WithLocation(11, 9)); } [Fact] @@ -69973,8 +69970,8 @@ class Program static void F() { object? o = (S?)new S() { F = 1, G = null }; // 1 - ((S?)o).Value.F.ToString(); - ((S?)o).Value.G.ToString(); // 2 + ((S?)o).Value.F.ToString(); // 2 + ((S?)o).Value.G.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -69982,9 +69979,9 @@ static void F() // (10,46): warning CS8625: Cannot convert null literal to non-nullable reference type. // object? o = (S?)new S() { F = 1, G = null }; // 1 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 46), - // (12,9): warning CS8602: Dereference of a possibly null reference. - // ((S?)o).Value.G.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((S?)o).Value.G").WithLocation(12, 9)); + // (11,9): warning CS8602: Dereference of a possibly null reference. + // ((S?)o).Value.F.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((S?)o).Value.F").WithLocation(11, 9)); } [Fact] @@ -70002,20 +69999,20 @@ class Program static void F1() where T : I, new() { object o1 = new T() { P = 1, Q = null }; // 1 - ((T)o1).P.ToString(); - ((T)o1).Q.ToString(); // 2 + ((T)o1).P.ToString(); // 2 + ((T)o1).Q.ToString(); } static void F2() where T : class, I, new() { object o2 = new T() { P = 2, Q = null }; // 3 - ((T)o2).P.ToString(); - ((T)o2).Q.ToString(); // 4 + ((T)o2).P.ToString(); // 4 + ((T)o2).Q.ToString(); } static void F3() where T : struct, I { object o3 = new T() { P = 3, Q = null }; // 5 - ((T)o3).P.ToString(); - ((T)o3).Q.ToString(); // 6 + ((T)o3).P.ToString(); // 6 + ((T)o3).Q.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -70023,21 +70020,21 @@ static void F3() where T : struct, I // (10,42): warning CS8625: Cannot convert null literal to non-nullable reference type. // object o1 = new T() { P = 1, Q = null }; // 1 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 42), - // (12,9): warning CS8602: Dereference of a possibly null reference. - // ((T)o1).Q.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)o1).Q").WithLocation(12, 9), + // (11,9): warning CS8602: Dereference of a possibly null reference. + // ((T)o1).P.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)o1).P").WithLocation(11, 9), // (16,42): warning CS8625: Cannot convert null literal to non-nullable reference type. // object o2 = new T() { P = 2, Q = null }; // 3 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(16, 42), - // (18,9): warning CS8602: Dereference of a possibly null reference. - // ((T)o2).Q.ToString(); // 4 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)o2).Q").WithLocation(18, 9), + // (17,9): warning CS8602: Dereference of a possibly null reference. + // ((T)o2).P.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)o2).P").WithLocation(17, 9), // (22,42): warning CS8625: Cannot convert null literal to non-nullable reference type. // object o3 = new T() { P = 3, Q = null }; // 5 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(22, 42), - // (24,9): warning CS8602: Dereference of a possibly null reference. - // ((T)o3).Q.ToString(); // 6 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)o3).Q").WithLocation(24, 9)); + // (23,9): warning CS8602: Dereference of a possibly null reference. + // ((T)o3).P.ToString(); // 6 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)o3).P").WithLocation(23, 9)); } [Fact] @@ -70105,20 +70102,20 @@ class Program static void F1() where T : I, new() { (object, object) t1 = (new T() { P = 1 }, new T() { Q = null }); // 1 - ((T)t1.Item1).P.ToString(); - (((T, T))t1).Item2.Q.ToString(); // 2 + ((T)t1.Item1).P.ToString(); // 2 + (((T, T))t1).Item2.Q.ToString(); } static void F2() where T : class, I, new() { (object, object) t2 = (new T() { P = 2 }, new T() { Q = null }); // 3 - ((T)t2.Item1).P.ToString(); - (((T, T))t2).Item2.Q.ToString(); // 4 + ((T)t2.Item1).P.ToString(); // 4 + (((T, T))t2).Item2.Q.ToString(); } static void F3() where T : struct, I { (object, object) t3 = (new T() { P = 3 }, new T() { Q = null }); // 5 - ((T)t3.Item1).P.ToString(); - (((T, T))t3).Item2.Q.ToString(); // 6 + ((T)t3.Item1).P.ToString(); // 6 + (((T, T))t3).Item2.Q.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -70126,21 +70123,21 @@ static void F3() where T : struct, I // (10,65): warning CS8625: Cannot convert null literal to non-nullable reference type. // (object, object) t1 = (new T() { P = 1 }, new T() { Q = null }); // 1 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 65), - // (12,9): warning CS8602: Dereference of a possibly null reference. - // (((T, T))t1).Item2.Q.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((T, T))t1).Item2.Q").WithLocation(12, 9), + // (11,9): warning CS8602: Dereference of a possibly null reference. + // ((T)t1.Item1).P.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)t1.Item1).P").WithLocation(11, 9), // (16,65): warning CS8625: Cannot convert null literal to non-nullable reference type. // (object, object) t2 = (new T() { P = 2 }, new T() { Q = null }); // 3 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(16, 65), - // (18,9): warning CS8602: Dereference of a possibly null reference. - // (((T, T))t2).Item2.Q.ToString(); // 4 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((T, T))t2).Item2.Q").WithLocation(18, 9), + // (17,9): warning CS8602: Dereference of a possibly null reference. + // ((T)t2.Item1).P.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)t2.Item1).P").WithLocation(17, 9), // (22,65): warning CS8625: Cannot convert null literal to non-nullable reference type. // (object, object) t3 = (new T() { P = 3 }, new T() { Q = null }); // 5 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(22, 65), - // (24,9): warning CS8602: Dereference of a possibly null reference. - // (((T, T))t3).Item2.Q.ToString(); // 6 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((T, T))t3).Item2.Q").WithLocation(24, 9)); + // (23,9): warning CS8602: Dereference of a possibly null reference. + // ((T)t3.Item1).P.ToString(); // 6 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T)t3.Item1).P").WithLocation(23, 9)); } [Fact] @@ -70158,8 +70155,8 @@ class Program static void F() where T : struct, I { object o = (T?)new T() { P = 1, Q = null }; // 1 - ((T?)o).Value.P.ToString(); - ((T?)o).Value.Q.ToString(); // 2 + ((T?)o).Value.P.ToString(); // 2 + ((T?)o).Value.Q.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -70167,9 +70164,9 @@ static void F() where T : struct, I // (10,45): warning CS8625: Cannot convert null literal to non-nullable reference type. // object o = (T?)new T() { P = 1, Q = null }; // 1 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 45), - // (12,9): warning CS8602: Dereference of a possibly null reference. - // ((T?)o).Value.Q.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T?)o).Value.Q").WithLocation(12, 9)); + // (11,9): warning CS8602: Dereference of a possibly null reference. + // ((T?)o).Value.P.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T?)o).Value.P").WithLocation(11, 9)); } [Fact] @@ -70188,8 +70185,8 @@ class Program static void F() where T : struct, I { (object, object) t = ((T?, T?))(new T() { P = 1 }, new T() { Q = null }); // 1 - ((T?)t.Item1).Value.P.ToString(); - (((T?, T?))t).Item2.Value.Q.ToString(); // 2 + ((T?)t.Item1).Value.P.ToString(); // 2 + (((T?, T?))t).Item2.Value.Q.ToString(); // 3 } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -70200,9 +70197,12 @@ static void F() where T : struct, I // (10,74): warning CS8625: Cannot convert null literal to non-nullable reference type. // (object, object) t = ((T?, T?))(new T() { P = 1 }, new T() { Q = null }); // 1 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 74), - // (12,9): warning CS8602: Possible dereference of a null reference. - // (((T?, T?))t).Item2.Value.Q.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "(((T?, T?))t).Item2.Value.Q").WithLocation(12, 9)); + // (11,9): warning CS8602: Dereference of a possibly null reference. + // ((T?)t.Item1).Value.P.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((T?)t.Item1).Value.P").WithLocation(11, 9), + // (12,9): warning CS8629: Nullable value type may be null. + // (((T?, T?))t).Item2.Value.Q.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "(((T?, T?))t).Item2").WithLocation(12, 9)); } [Fact] From 6f55117a36a083adca46c5085e98a90e1a5cca12 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Fri, 10 Jan 2020 14:44:02 -0800 Subject: [PATCH 03/10] Fix a few bugs with consistency of the use of slots in the nullable walker. --- .../Portable/Binder/Binder_Expressions.cs | 2 +- .../Portable/Binder/DecisionDagBuilder.cs | 35 ++- .../Portable/BoundTree/BoundDagEvaluation.cs | 21 +- .../Portable/Compilation/CSharpCompilation.cs | 3 + .../DefiniteAssignment.VariableIdentifier.cs | 2 +- .../FlowAnalysis/LocalDataFlowPass.cs | 6 +- .../Portable/FlowAnalysis/NullableWalker.cs | 293 ++++++++---------- .../FlowAnalysis/NullableWalker_Patterns.cs | 19 +- .../LocalRewriter/LocalRewriter_Patterns.cs | 1 - .../CSharp/Test/Emit/CodeGen/PatternTests.cs | 131 ++++---- .../Semantics/NullableReferenceTypesTests.cs | 97 +++--- .../Symbols/SymbolEqualityComparer.cs | 3 + 12 files changed, 300 insertions(+), 313 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index f840395a473f2..0cd80c738bd9b 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -8133,7 +8133,7 @@ private BoundConditionalAccess GenerateBadConditionalAccessNodeError(Conditional // conditional access is not really a binary operator. DiagnosticInfo diagnosticInfo = new CSDiagnosticInfo(ErrorCode.ERR_BadUnaryOp, SyntaxFacts.GetText(operatorToken.Kind()), access.Display); diagnostics.Add(new CSDiagnostic(diagnosticInfo, operatorToken.GetLocation())); - access = BadExpression(access.Syntax, access); + receiver = BadExpression(receiver.Syntax, receiver); return new BoundConditionalAccess(node, receiver, access, CreateErrorType(), hasErrors: true); } diff --git a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs index 68c83b0f86125..61c2ed95ac20a 100644 --- a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs +++ b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs @@ -313,20 +313,43 @@ private void MakeTestsAndBindings( tests.Add(valueAsITupleEvaluation); var valueAsITuple = new BoundDagTemp(syntax, iTupleType, valueAsITupleEvaluation); - var lengthEvaluation = new BoundDagPropertyEvaluation(syntax, getLengthProperty, valueAsITuple); + var lengthEvaluation = new BoundDagPropertyEvaluation(syntax, getLengthProperty, OriginalInput(valueAsITuple, getLengthProperty)); tests.Add(lengthEvaluation); var lengthTemp = new BoundDagTemp(syntax, this._compilation.GetSpecialType(SpecialType.System_Int32), lengthEvaluation); tests.Add(new BoundDagValueTest(syntax, ConstantValue.Create(patternLength), lengthTemp)); + var getItemPropertyInput = OriginalInput(valueAsITuple, getItemProperty); for (int i = 0; i < patternLength; i++) { - var indexEvaluation = new BoundDagIndexEvaluation(syntax, getItemProperty, i, valueAsITuple); + var indexEvaluation = new BoundDagIndexEvaluation(syntax, getItemProperty, i, getItemPropertyInput); tests.Add(indexEvaluation); var indexTemp = new BoundDagTemp(syntax, objectType, indexEvaluation); MakeTestsAndBindings(indexTemp, pattern.Subpatterns[i].Pattern, tests, bindings); } } + /// + /// Get the earliest input of which the symbol is a member. + /// A BoundDagTypeEvaluation doesn't change the underlying object being pointed to. + /// So two evaluations act on the same input so long as they have the same original input. + /// We use this method to compute the original input for an evaluation. + /// + private BoundDagTemp OriginalInput(BoundDagTemp input, Symbol symbol) + { + while (input.Source is BoundDagTypeEvaluation source && IsDerivedType(source.Input.Type, symbol.ContainingType)) + { + input = source.Input; + } + + return input; + } + + bool IsDerivedType(TypeSymbol possibleDerived, TypeSymbol possibleBase) + { + HashSet useSiteDiagnostics = null; + return this._conversions.HasIdentityOrImplicitReferenceConversion(possibleDerived, possibleBase, ref useSiteDiagnostics); + } + private void MakeTestsAndBindings( BoundDagTemp input, BoundDeclarationPattern declaration, @@ -447,7 +470,7 @@ private void MakeTestsAndBindings( if (recursive.DeconstructMethod != null) { MethodSymbol method = recursive.DeconstructMethod; - var evaluation = new BoundDagDeconstructEvaluation(recursive.Syntax, method, input); + var evaluation = new BoundDagDeconstructEvaluation(recursive.Syntax, method, OriginalInput(input, method)); tests.Add(evaluation); int extensionExtra = method.IsStatic ? 1 : 0; int count = Math.Min(method.ParameterCount - extensionExtra, recursive.Deconstruction.Length); @@ -477,7 +500,7 @@ private void MakeTestsAndBindings( BoundPattern pattern = recursive.Deconstruction[i].Pattern; SyntaxNode syntax = pattern.Syntax; FieldSymbol field = elements[i]; - var evaluation = new BoundDagFieldEvaluation(syntax, field, input); // fetch the ItemN field + var evaluation = new BoundDagFieldEvaluation(syntax, field, OriginalInput(input, field)); // fetch the ItemN field tests.Add(evaluation); var output = new BoundDagTemp(syntax, field.Type, evaluation); MakeTestsAndBindings(output, pattern, tests, bindings); @@ -504,10 +527,10 @@ private void MakeTestsAndBindings( switch (symbol) { case PropertySymbol property: - evaluation = new BoundDagPropertyEvaluation(pattern.Syntax, property, input); + evaluation = new BoundDagPropertyEvaluation(pattern.Syntax, property, OriginalInput(input, property)); break; case FieldSymbol field: - evaluation = new BoundDagFieldEvaluation(pattern.Syntax, field, input); + evaluation = new BoundDagFieldEvaluation(pattern.Syntax, field, OriginalInput(input, field)); break; default: Debug.Assert(recursive.HasAnyErrors); diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs index aff66fe451b40..c0e2c5b6220a6 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.CodeAnalysis.CSharp.Symbols; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp @@ -9,7 +10,7 @@ partial class BoundDagEvaluation public override bool Equals(object obj) => obj is BoundDagEvaluation other && this.Equals(other); public virtual bool Equals(BoundDagEvaluation other) { - return other != (object)null && this.Kind == other.Kind && this.GetOriginalInput().Equals(other.GetOriginalInput()) && this.Symbol == other.Symbol; + return other != (object)null && this.Kind == other.Kind && this.Input.Equals(other.Input) && this.Symbol == other.Symbol; } private Symbol Symbol { @@ -28,23 +29,7 @@ private Symbol Symbol } public override int GetHashCode() { - return Hash.Combine(GetOriginalInput().GetHashCode(), this.Symbol?.GetHashCode() ?? 0); - } - - /// - /// Returns the original input for this evaluation, stripped of all Type Evaluations. - /// - /// A BoundDagTypeEvaluation doesn't change the underlying object being pointed to - /// So two evaluations act on the same input so long as they have the same original input. - /// - private BoundDagTemp GetOriginalInput() - { - var input = this.Input; - while (input.Source is BoundDagTypeEvaluation source) - { - input = source.Input; - } - return input; + return Hash.Combine(Input.GetHashCode(), this.Symbol?.GetHashCode() ?? 0); } public static bool operator ==(BoundDagEvaluation left, BoundDagEvaluation right) diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 6f810f920ef4d..397332d0518d5 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -62,6 +62,9 @@ public sealed partial class CSharpCompilation : Compilation private ImmutableArray _lazyClsComplianceDiagnostics; private Conversions _conversions; + /// + /// A conversions object that ignores nullability. + /// internal Conversions Conversions { get diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs index 62ab1668ec8eb..06ee5ff7b195b 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs @@ -80,7 +80,7 @@ public bool Equals(VariableIdentifier other) return true; } - return Symbol.OriginalDefinition.Equals(other.Symbol.OriginalDefinition, SymbolEqualityComparer.ConsiderEverything.CompareKind); + return Symbol.Equals(other.Symbol, SymbolEqualityComparer.IgnoreEverything.CompareKind); } public override bool Equals(object obj) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs index f49923c684804..a290839df55bb 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs @@ -105,7 +105,7 @@ protected virtual bool IsEmptyStructType(TypeSymbol type) /// /// Force a variable to have a slot. Returns -1 if the variable has an empty struct type. /// - protected int GetOrCreateSlot(Symbol symbol, int containingSlot = 0, bool forceSlotEvenIfEmpty = false) + protected virtual int GetOrCreateSlot(Symbol symbol, int containingSlot = 0, bool forceSlotEvenIfEmpty = false) { Debug.Assert(containingSlot >= 0); @@ -226,7 +226,8 @@ protected Symbol GetNonMemberSymbol(int slot) VariableIdentifier variableId = variableBySlot[slot]; while (variableId.ContainingSlot > 0) { - Debug.Assert(variableId.Symbol.Kind == SymbolKind.Field || variableId.Symbol.Kind == SymbolKind.Property || variableId.Symbol.Kind == SymbolKind.Event); + Debug.Assert(variableId.Symbol.Kind == SymbolKind.Field || variableId.Symbol.Kind == SymbolKind.Property || variableId.Symbol.Kind == SymbolKind.Event, + "inconsistent property symbol owner"); variableId = variableBySlot[variableId.ContainingSlot]; } return variableId.Symbol; @@ -284,6 +285,7 @@ protected int MakeMemberSlot(BoundExpression receiverOpt, Symbol member) { containingSlot = 0; } + return GetOrCreateSlot(member, containingSlot); } } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index bd809d8805786..55562c20217ae 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -681,73 +681,6 @@ private static void Analyze( } } - /// - /// Check that slots for member symbols indicate members of the enclosing slot's type. - /// - [Conditional("DEBUG")] - private void ValidateLastSlot() - { - int i = nextVariableSlot - 1; - if (i <= 1) - return; - - var variable = variableBySlot[i].Symbol; - var containingSlot = variableBySlot[i].ContainingSlot; - if (containingSlot < 1) - return; - - var containingSlotType = variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type; - Debug.Assert(IsMember(variable, containingSlotType)); - } - - static bool IsMember(Symbol member, TypeSymbol possibleContainer) - { - TypeSymbol memberContainer = member.ContainingType; - if (memberContainer.Equals(possibleContainer, TypeCompareKind.AllIgnoreOptions)) - return true; - - switch (memberContainer.TypeKind) - { - case TypeKind.Interface: - { - var allInterfaces = possibleContainer switch - { - // interface members are only inherited into interfaces and type parameters. - TypeParameterSymbol typeParameter => typeParameter.AllEffectiveInterfacesNoUseSiteDiagnostics, - { TypeKind: TypeKind.Interface } => possibleContainer.AllInterfacesNoUseSiteDiagnostics, - _ => ImmutableArray.Empty, - }; - - foreach (var possibleContainerBase in allInterfaces) - { - if (memberContainer.Equals(possibleContainerBase, TypeCompareKind.AllIgnoreOptions)) - return true; - } - - return false; - } - default: - { - var effectiveBase = possibleContainer switch - { - // interface members are only inherited into interfaces and type parameters. - TypeParameterSymbol typeParameter => typeParameter.EffectiveBaseClassNoUseSiteDiagnostics, - _ => possibleContainer.BaseTypeNoUseSiteDiagnostics, - }; - - for (var possibleContainerBase = effectiveBase; - !(possibleContainerBase is null); - possibleContainerBase = possibleContainerBase.BaseTypeNoUseSiteDiagnostics) - { - if (memberContainer.Equals(possibleContainerBase, TypeCompareKind.AllIgnoreOptions)) - return true; - } - - return false; - } - } - } - private SharedWalkerState SaveSharedState() => new SharedWalkerState( _variableSlot.ToImmutableDictionary(), @@ -922,78 +855,94 @@ protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundE protected override int MakeSlot(BoundExpression node) { - switch (node.Kind) + int result = makeSlot(node); +#if DEBUG + if (result != -1) { - case BoundKind.ThisReference: - case BoundKind.BaseReference: - { - var method = getTopLevelMethod(_symbol as MethodSymbol); - var thisParameter = method?.ThisParameter; - return (object)thisParameter != null ? GetOrCreateSlot(thisParameter) : -1; - } - case BoundKind.Conversion: - { - int slot = getPlaceholderSlot(node); - if (slot > 0) + // Check that the slot represents a value of an equivalent type to the node + TypeSymbol slotType = variableBySlot[result].Symbol.GetTypeOrReturnType().Type; + TypeSymbol nodeType = node.Type; + HashSet discardedUseSiteDiagnostics = null; + var conversionsWithoutNullability = this.compilation.Conversions; + Debug.Assert(nodeType.IsErrorType() || + conversionsWithoutNullability.HasIdentityOrImplicitReferenceConversion(slotType, nodeType, ref discardedUseSiteDiagnostics) || + conversionsWithoutNullability.HasBoxingConversion(slotType, nodeType, ref discardedUseSiteDiagnostics)); + } +#endif + return result; + + int makeSlot(BoundExpression node) + { + switch (node.Kind) + { + case BoundKind.ThisReference: + case BoundKind.BaseReference: { - return slot; + var method = getTopLevelMethod(_symbol as MethodSymbol); + var thisParameter = method?.ThisParameter; + return (object)thisParameter != null ? GetOrCreateSlot(thisParameter) : -1; } - var conv = (BoundConversion)node; - switch (conv.Conversion.Kind) + case BoundKind.Conversion: { - case ConversionKind.ExplicitNullable: - { - var operand = conv.Operand; - var operandType = operand.Type; - var convertedType = conv.Type; - if (AreNullableAndUnderlyingTypes(operandType, convertedType, out _)) + int slot = getPlaceholderSlot(node); + if (slot > 0) + { + return slot; + } + var conv = (BoundConversion)node; + switch (conv.Conversion.Kind) + { + case ConversionKind.ExplicitNullable: { - // Explicit conversion of Nullable to T is equivalent to Nullable.Value. - // For instance, in the following, when evaluating `((A)a).B` we need to recognize - // the nullability of `(A)a` (not nullable) and the slot (the slot for `a.Value`). - // struct A { B? B; } - // struct B { } - // if (a?.B != null) _ = ((A)a).B.Value; // no warning - int containingSlot = MakeSlot(operand); - return containingSlot < 0 ? -1 : GetNullableOfTValueSlot(operandType, containingSlot, out _); + var operand = conv.Operand; + var operandType = operand.Type; + var convertedType = conv.Type; + if (AreNullableAndUnderlyingTypes(operandType, convertedType, out _)) + { + // Explicit conversion of Nullable to T is equivalent to Nullable.Value. + // For instance, in the following, when evaluating `((A)a).B` we need to recognize + // the nullability of `(A)a` (not nullable) and the slot (the slot for `a.Value`). + // struct A { B? B; } + // struct B { } + // if (a?.B != null) _ = ((A)a).B.Value; // no warning + int containingSlot = MakeSlot(operand); + return containingSlot < 0 ? -1 : GetNullableOfTValueSlot(operandType, containingSlot, out _); + } } - } - break; - case ConversionKind.Identity: - case ConversionKind.DefaultLiteral: - case ConversionKind.ImplicitReference: - case ConversionKind.ExplicitReference: - case ConversionKind.ImplicitTupleLiteral: - case ConversionKind.ExplicitTupleLiteral: - case ConversionKind.Boxing: - case ConversionKind.Unboxing: - // No need to create a slot for the boxed value (in the Boxing case) since assignment already - // clones slots and there is not another scenario where creating a slot is observable. - return MakeSlot(conv.Operand); + break; + case ConversionKind.Identity: + case ConversionKind.DefaultLiteral: + case ConversionKind.ImplicitReference: + case ConversionKind.ImplicitTupleLiteral: + case ConversionKind.Boxing: + // No need to create a slot for the boxed value (in the Boxing case) since assignment already + // clones slots and there is not another scenario where creating a slot is observable. + return MakeSlot(conv.Operand); + } } - } - break; - case BoundKind.DefaultLiteral: - case BoundKind.DefaultExpression: - case BoundKind.ObjectCreationExpression: - case BoundKind.DynamicObjectCreationExpression: - case BoundKind.AnonymousObjectCreationExpression: - case BoundKind.NewT: - case BoundKind.TupleLiteral: - case BoundKind.ConvertedTupleLiteral: - return getPlaceholderSlot(node); - case BoundKind.ConditionalReceiver: - { + break; + case BoundKind.DefaultLiteral: + case BoundKind.DefaultExpression: + case BoundKind.ObjectCreationExpression: + case BoundKind.DynamicObjectCreationExpression: + case BoundKind.AnonymousObjectCreationExpression: + case BoundKind.NewT: + case BoundKind.TupleLiteral: + case BoundKind.ConvertedTupleLiteral: + return getPlaceholderSlot(node); + case BoundKind.ConditionalAccess: + return getPlaceholderSlot(node); + case BoundKind.ConditionalReceiver: return _lastConditionalAccessSlot; - } - default: - { - int slot = getPlaceholderSlot(node); - return (slot > 0) ? slot : base.MakeSlot(node); - } - } + default: + { + int slot = getPlaceholderSlot(node); + return (slot > 0) ? slot : base.MakeSlot(node); + } + } - return -1; + return -1; + } int getPlaceholderSlot(BoundExpression expr) { @@ -1019,6 +968,15 @@ static MethodSymbol getTopLevelMethod(MethodSymbol method) } } + protected override int GetOrCreateSlot(Symbol symbol, int containingSlot = 0, bool forceSlotEvenIfEmpty = false) + { + + if (containingSlot > 0 && !IsSlotMember(containingSlot, symbol)) + return -1; + + return base.GetOrCreateSlot(symbol, containingSlot, forceSlotEvenIfEmpty); + } + private void VisitAndUnsplitAll(ImmutableArray nodes) where T : BoundNode { if (nodes.IsDefault) @@ -1316,6 +1274,17 @@ private void InheritNullableStateOfTrackableStruct(TypeSymbol targetType, int ta } } + private bool IsSlotMember(int slot, Symbol possibleMember) + { + TypeSymbol possibleBase = possibleMember.ContainingType; + TypeSymbol possibleDerived = NominalSlotType(slot); + HashSet discardedUseSiteDiagnostics = null; + var conversionsWithoutNullability = this.compilation.Conversions; + return + conversionsWithoutNullability.HasIdentityOrImplicitReferenceConversion(possibleDerived, possibleBase, ref discardedUseSiteDiagnostics) || + conversionsWithoutNullability.HasBoxingConversion(possibleDerived, possibleBase, ref discardedUseSiteDiagnostics); + } + // 'skipSlot' is the original target slot that should be skipped in case of cycles. private void InheritNullableStateOfMember(int targetContainerSlot, int valueContainerSlot, Symbol member, bool isDefaultValue, int skipSlot) { @@ -1323,9 +1292,8 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont Debug.Assert(skipSlot > 0); // Ensure member is valid for target and value. - if (!IsMember(member, variableBySlot[targetContainerSlot].Symbol.GetTypeOrReturnType().Type)) + if (!IsSlotMember(targetContainerSlot, member)) return; - // PROTOTYPE(ngafter): The above line is a good place to set a breakpoint to diagnose this set of changes. TypeWithAnnotations fieldOrPropertyType = member.GetTypeOrReturnType(); @@ -1334,7 +1302,6 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont fieldOrPropertyType.IsNullableType()) { int targetMemberSlot = GetOrCreateSlot(member, targetContainerSlot); - ValidateLastSlot(); if (targetMemberSlot > 0) { NullableFlowState value = isDefaultValue ? NullableFlowState.MaybeNull : fieldOrPropertyType.ToTypeWithState().State; @@ -1362,11 +1329,9 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont else if (EmptyStructTypeCache.IsTrackableStructType(fieldOrPropertyType.Type)) { int targetMemberSlot = GetOrCreateSlot(member, targetContainerSlot); - ValidateLastSlot(); if (targetMemberSlot > 0) { int valueMemberSlot = (valueContainerSlot > 0) ? GetOrCreateSlot(member, valueContainerSlot) : -1; - ValidateLastSlot(); if (valueMemberSlot == skipSlot) { return; @@ -1376,6 +1341,11 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont } } + private TypeSymbol NominalSlotType(int slot) + { + return variableBySlot[slot].Symbol.GetTypeOrReturnType().Type; + } + /// /// Whenever assigning a variable, and that variable is not declared at the point the state is being set, /// and the new state is not , this method should be called to perform the @@ -2026,12 +1996,7 @@ private void VisitObjectElementInitializer(int containingSlot, BoundAssignmentOp if ((object)symbol != null) { - if (containingSlot > 0 && !IsMember(symbol, base.variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type)) - { - // PROTOTYPE(ngafter): This block is here to help0 debug things. - } - int slot = (containingSlot < 0 || !IsMember(symbol, base.variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type)) ? -1 : GetOrCreateSlot(symbol, containingSlot); - ValidateLastSlot(); + int slot = (containingSlot < 0 || !IsSlotMember(containingSlot, symbol)) ? -1 : GetOrCreateSlot(symbol, containingSlot); VisitObjectCreationInitializer(symbol, slot, node.Right, GetLValueAnnotations(node.Left)); // https://github.com/dotnet/roslyn/issues/35040: Should likely be setting _resultType in VisitObjectCreationInitializer // and using that value instead of reconstructing here @@ -2156,7 +2121,6 @@ public override BoundNode VisitAnonymousObjectCreationExpression(BoundAnonymousO // A void element results in an error type in the anonymous type but not in the property's container! // To avoid failing an assertion later, we skip them. var slot = GetOrCreateSlot(property, receiverSlot); - ValidateLastSlot(); TrackNullableStateForAssignment(argument, property.TypeWithAnnotations, slot, argumentType, MakeSlot(argument)); var currentDeclaration = getDeclaration(node, property, ref currentDeclarationIndex); @@ -2688,27 +2652,21 @@ private void GetSlotsToMarkAsNotNullable(BoundExpression operand, ArrayBuilder 0) { // We need to continue the walk regardless of whether the receiver should be updated. var receiverType = conditional.Receiver.Type; - if (PossiblyNullableType(receiverType)) - { - slotBuilder.Add(slot); - } - if (receiverType.IsNullableType()) - { slot = GetNullableOfTValueSlot(receiverType, slot, out _); - } } if (slot > 0) { // When MakeSlot is called on the nested AccessExpression, it will recurse through receivers - // until it gets to the BoundConditionalReceiver associated with this node. In our override, - // we substitute this slot when we encounter a BoundConditionalReceiver, and reset the + // until it gets to the BoundConditionalReceiver associated with this node. In our override + // of MakeSlot, we substitute this slot when we encounter a BoundConditionalReceiver, and reset the // _lastConditionalAccess field. _lastConditionalAccessSlot = slot; operand = conditional.AccessExpression; @@ -2718,6 +2676,7 @@ private void GetSlotsToMarkAsNotNullable(BoundExpression operand, ArrayBuilder 0 && PossiblyNullableType(expressionType)) - { state[slot] = NullableFlowState.MaybeNull; - } - - return slot; } private static BoundExpression SkipReferenceConversions(BoundExpression possiblyConversion) @@ -2957,8 +2910,13 @@ public override BoundNode VisitConditionalAccess(BoundConditionalAccess node) { // In the when-null branch, the receiver is known to be maybe-null. // In the other branch, the receiver is known to be non-null. - _lastConditionalAccessSlot = LearnFromNullTest(receiver, ref receiverState); + LearnFromNullTest(receiver, ref receiverState); LearnFromNonNullTest(receiver, ref this.State); + var nextConditionalAccessSlot = MakeSlot(receiver); + if (receiver.Type.IsNullableType()) + nextConditionalAccessSlot = GetNullableOfTValueSlot(receiver.Type, nextConditionalAccessSlot, out _); + + _lastConditionalAccessSlot = nextConditionalAccessSlot; } var accessTypeWithAnnotations = VisitLvalueWithAnnotations(node.AccessExpression); @@ -2973,7 +2931,7 @@ public override BoundNode VisitConditionalAccess(BoundConditionalAccess node) // Per LDM 2019-02-13 decision, the result of a conditional access "may be null" even if // both the receiver and right-hand-side are believed not to be null. - SetResultType(node, TypeWithState.Create(resultType, NullableFlowState.MaybeDefault)); + SetResultType(node, TypeWithState.Create(resultType, NullableFlowState.MaybeNull)); _currentConditionalReceiverVisitResult = default; _lastConditionalAccessSlot = previousConditionalAccessSlot; return null; @@ -4847,7 +4805,6 @@ private void TrackNullableStateOfTupleElements( void trackState(BoundExpression value, FieldSymbol field, TypeWithState valueType) { int targetSlot = GetOrCreateSlot(field, slot); - ValidateLastSlot(); TrackNullableStateForAssignment(value, field.TypeWithAnnotations, targetSlot, valueType, MakeSlot(value)); } } @@ -4940,12 +4897,10 @@ void trackConvertedValue(FieldSymbol targetField, Conversion conversion, FieldSy case ConversionKind.ExplicitTuple: { int targetFieldSlot = GetOrCreateSlot(targetField, slot); - ValidateLastSlot(); if (targetFieldSlot > 0) { this.State[targetFieldSlot] = NullableFlowState.NotNull; int valueFieldSlot = GetOrCreateSlot(valueField, valueSlot); - ValidateLastSlot(); if (valueFieldSlot > 0) { TrackNullableStateOfTupleConversion(conversionOpt, convertedNode, conversion, targetField.Type, valueField.Type, targetFieldSlot, valueFieldSlot, assignmentKind, parameterOpt, reportWarnings); @@ -4959,12 +4914,10 @@ void trackConvertedValue(FieldSymbol targetField, Conversion conversion, FieldSy if (AreNullableAndUnderlyingTypes(targetField.Type, valueField.Type, out _)) { int targetFieldSlot = GetOrCreateSlot(targetField, slot); - ValidateLastSlot(); if (targetFieldSlot > 0) { this.State[targetFieldSlot] = NullableFlowState.NotNull; int valueFieldSlot = GetOrCreateSlot(valueField, valueSlot); - ValidateLastSlot(); if (valueFieldSlot > 0) { TrackNullableStateOfNullableValue(targetFieldSlot, targetField.Type, null, valueField.TypeWithAnnotations.ToTypeWithState(), valueFieldSlot); @@ -4988,7 +4941,6 @@ void trackConvertedValue(FieldSymbol targetField, Conversion conversion, FieldSy reportRemainingWarnings: reportWarnings, diagnosticLocation: (conversionOpt ?? convertedNode).Syntax.GetLocation()); int targetFieldSlot = GetOrCreateSlot(targetField, slot); - ValidateLastSlot(); if (targetFieldSlot > 0) { this.State[targetFieldSlot] = convertedType.State; @@ -6840,12 +6792,11 @@ private Symbol VisitMemberAccess(BoundExpression node, BoundExpression receiverO private int GetNullableOfTValueSlot(TypeSymbol containingType, int containingSlot, out Symbol valueProperty, bool forceSlotEvenIfEmpty = false) { Debug.Assert(containingType.IsNullableType()); - Debug.Assert(TypeSymbol.Equals(variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type, containingType, TypeCompareKind.ConsiderEverything2)); + Debug.Assert(TypeSymbol.Equals(NominalSlotType(containingSlot), containingType, TypeCompareKind.ConsiderEverything2)); var getValue = (MethodSymbol)compilation.GetSpecialTypeMember(SpecialMember.System_Nullable_T_get_Value); valueProperty = getValue?.AsMember((NamedTypeSymbol)containingType)?.AssociatedSymbol; var result = (valueProperty is null) ? -1 : GetOrCreateSlot(valueProperty, containingSlot, forceSlotEvenIfEmpty: forceSlotEvenIfEmpty); - ValidateLastSlot(); return result; } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs index 3f3a6fe2f31d3..e467eba5e37e9 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs @@ -217,6 +217,7 @@ protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastS var tempMap = PooledDictionary.GetInstance(); Debug.Assert(originalInputSlot > 0); + Debug.Assert(isDerivedType(NominalSlotType(originalInputSlot), expressionType.Type)); tempMap.Add(rootTemp, (originalInputSlot, expressionType.Type)); var nodeStateMap = PooledDictionary.GetInstance(); @@ -268,8 +269,6 @@ protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastS { case ConversionKind.Identity: case ConversionKind.ImplicitReference: - case ConversionKind.NoConversion: - case ConversionKind.ExplicitReference: outputSlot = inputSlot; break; case ConversionKind.ExplicitNullable when AreNullableAndUnderlyingTypes(inputType, e.Type, out _): @@ -451,14 +450,28 @@ void addToTempMap(BoundDagTemp output, int slot, TypeSymbol type) { // The dag temp has already been allocated on another branch of the dag Debug.Assert(outputSlotAndType.slot == slot); - Debug.Assert(outputSlotAndType.type.Equals(type, TypeCompareKind.AllIgnoreOptions)); + Debug.Assert(isDerivedType(outputSlotAndType.type, type)); } else { + var slotType = NominalSlotType(slot); + Debug.Assert(slotType.IsErrorType() || isDerivedType(slotType, type)); tempMap.Add(output, (slot, type)); } } + bool isDerivedType(TypeSymbol derivedType, TypeSymbol baseType) + { + HashSet discardedDiagnostics = null; + return _conversions.WithNullability(false).ClassifyConversionFromType(derivedType, baseType, ref discardedDiagnostics).Kind switch + { + ConversionKind.Identity => true, + ConversionKind.ImplicitReference => true, + ConversionKind.Boxing => true, + _ => false, + }; + } + void gotoNode(BoundDecisionDagNode node, LocalState state, bool believedReachable) { if (nodeStateMap.TryGetValue(node, out var stateAndReachable)) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs index 489c23db276de..5f87bfe5c02a0 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs @@ -229,7 +229,6 @@ void addArg(RefKind refKind, BoundExpression expression) { // This is an evaluation of an indexed property with a constant int value. // The input type must be ITuple, and the property must be a property of ITuple. - Debug.Assert(e.Property.ContainingSymbol.Equals(input.Type)); Debug.Assert(e.Property.GetMethod.ParameterCount == 1); Debug.Assert(e.Property.GetMethod.Parameters[0].Type.SpecialType == SpecialType.System_Int32); TypeSymbol type = e.Property.GetMethod.ReturnType; diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs index aad21cd3193ae..7c05cd3777c52 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs @@ -1331,43 +1331,40 @@ public void M(Person p) { compilation.VerifyDiagnostics(); var compVerifier = CompileAndVerify(compilation); compVerifier.VerifyIL("C.M", -@" -{ - // Code size 77 (0x4d) +@"{ + // Code size 75 (0x4b) .maxstack 3 .locals init (string V_0, //name - string V_1, //name - Teacher V_2) + string V_1) //name IL_0000: ldarg.1 IL_0001: isinst ""Teacher"" - IL_0006: stloc.2 - IL_0007: ldloc.2 - IL_0008: brfalse.s IL_0013 - IL_000a: ldloc.2 - IL_000b: callvirt ""string Person.Name.get"" - IL_0010: stloc.0 - IL_0011: br.s IL_001f - IL_0013: ldarg.1 - IL_0014: brfalse.s IL_004c - IL_0016: ldarg.1 - IL_0017: callvirt ""string Person.Name.get"" - IL_001c: stloc.0 - IL_001d: br.s IL_0035 - IL_001f: ldstr ""Hello teacher "" - IL_0024: ldloc.0 - IL_0025: ldstr ""!"" - IL_002a: call ""string string.Concat(string, string, string)"" - IL_002f: call ""void System.Console.WriteLine(string)"" - IL_0034: ret - IL_0035: ldloc.0 - IL_0036: stloc.1 - IL_0037: ldstr ""Hello "" - IL_003c: ldloc.1 - IL_003d: ldstr ""!"" - IL_0042: call ""string string.Concat(string, string, string)"" - IL_0047: call ""void System.Console.WriteLine(string)"" - IL_004c: ret -}"); + IL_0006: brfalse.s IL_0011 + IL_0008: ldarg.1 + IL_0009: callvirt ""string Person.Name.get"" + IL_000e: stloc.0 + IL_000f: br.s IL_001d + IL_0011: ldarg.1 + IL_0012: brfalse.s IL_004a + IL_0014: ldarg.1 + IL_0015: callvirt ""string Person.Name.get"" + IL_001a: stloc.0 + IL_001b: br.s IL_0033 + IL_001d: ldstr ""Hello teacher "" + IL_0022: ldloc.0 + IL_0023: ldstr ""!"" + IL_0028: call ""string string.Concat(string, string, string)"" + IL_002d: call ""void System.Console.WriteLine(string)"" + IL_0032: ret + IL_0033: ldloc.0 + IL_0034: stloc.1 + IL_0035: ldstr ""Hello "" + IL_003a: ldloc.1 + IL_003b: ldstr ""!"" + IL_0040: call ""string string.Concat(string, string, string)"" + IL_0045: call ""void System.Console.WriteLine(string)"" + IL_004a: ret +} +"); } [Fact, WorkItem(34933, "https://github.com/dotnet/roslyn/issues/34933")] @@ -1406,47 +1403,41 @@ public void M(Person p) { compilation.VerifyDiagnostics(); var compVerifier = CompileAndVerify(compilation); compVerifier.VerifyIL("C.M", -@" -{ - // Code size 84 (0x54) +@"{ + // Code size 80 (0x50) .maxstack 3 .locals init (string V_0, //name - string V_1, //name - Teacher V_2, - Student V_3) + string V_1) //name IL_0000: ldarg.1 IL_0001: isinst ""Teacher"" - IL_0006: stloc.2 - IL_0007: ldloc.2 - IL_0008: brfalse.s IL_0013 - IL_000a: ldloc.2 - IL_000b: callvirt ""string Person.Name.get"" - IL_0010: stloc.0 - IL_0011: br.s IL_0026 - IL_0013: ldarg.1 - IL_0014: isinst ""Student"" - IL_0019: stloc.3 - IL_001a: ldloc.3 - IL_001b: brfalse.s IL_0053 - IL_001d: ldloc.3 - IL_001e: callvirt ""string Person.Name.get"" - IL_0023: stloc.0 - IL_0024: br.s IL_003c - IL_0026: ldstr ""Hello teacher "" - IL_002b: ldloc.0 - IL_002c: ldstr ""!"" - IL_0031: call ""string string.Concat(string, string, string)"" - IL_0036: call ""void System.Console.WriteLine(string)"" - IL_003b: ret - IL_003c: ldloc.0 - IL_003d: stloc.1 - IL_003e: ldstr ""Hello student "" - IL_0043: ldloc.1 - IL_0044: ldstr ""!"" - IL_0049: call ""string string.Concat(string, string, string)"" - IL_004e: call ""void System.Console.WriteLine(string)"" - IL_0053: ret -}"); + IL_0006: brfalse.s IL_0011 + IL_0008: ldarg.1 + IL_0009: callvirt ""string Person.Name.get"" + IL_000e: stloc.0 + IL_000f: br.s IL_0022 + IL_0011: ldarg.1 + IL_0012: isinst ""Student"" + IL_0017: brfalse.s IL_004f + IL_0019: ldarg.1 + IL_001a: callvirt ""string Person.Name.get"" + IL_001f: stloc.0 + IL_0020: br.s IL_0038 + IL_0022: ldstr ""Hello teacher "" + IL_0027: ldloc.0 + IL_0028: ldstr ""!"" + IL_002d: call ""string string.Concat(string, string, string)"" + IL_0032: call ""void System.Console.WriteLine(string)"" + IL_0037: ret + IL_0038: ldloc.0 + IL_0039: stloc.1 + IL_003a: ldstr ""Hello student "" + IL_003f: ldloc.1 + IL_0040: ldstr ""!"" + IL_0045: call ""string string.Concat(string, string, string)"" + IL_004a: call ""void System.Console.WriteLine(string)"" + IL_004f: ret +} +"); } [Fact, WorkItem(34933, "https://github.com/dotnet/roslyn/issues/34933")] diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 1cf608d0fbfa6..070604fadfc9a 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -59831,10 +59831,10 @@ static void F(string x, string? y) (object a, string?) u = ((string, string?))t; (object?, object b) v = ((object?, object))t; t.Item1.ToString(); - t.Item2.ToString(); // 2 + t.Item2.ToString(); u.Item1.ToString(); - u.Item2.ToString(); // 3 - v.Item1.ToString(); // 4 + u.Item2.ToString(); // 2 + v.Item1.ToString(); // 3 v.Item2.ToString(); } }"; @@ -59846,14 +59846,11 @@ static void F(string x, string? y) // (5,52): warning CS8600: Converting null literal or possible null value to non-nullable type. // (object, string) t = ((object, string))(x, y)/*T:(string! x, string? y)*/; // 1 Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "y").WithLocation(5, 52), - // (9,9): warning CS8602: Dereference of a possibly null reference. - // t.Item2.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t.Item2").WithLocation(9, 9), // (11,9): warning CS8602: Dereference of a possibly null reference. - // u.Item2.ToString(); // 3 + // u.Item2.ToString(); // 2 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "u.Item2").WithLocation(11, 9), // (12,9): warning CS8602: Dereference of a possibly null reference. - // v.Item1.ToString(); // 4 + // v.Item1.ToString(); // 3 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "v.Item1").WithLocation(12, 9)); comp.VerifyTypes(); } @@ -68708,7 +68705,7 @@ static void F() A a = new B() { FA = 1 }; a.FA.ToString(); a = new B() { FA = 2, FB = null }; // 1 - ((B)a).FA.ToString(); + ((B)a).FA.ToString(); // 2 ((B)a).FB.ToString(); } }"; @@ -68716,7 +68713,10 @@ static void F() comp.VerifyDiagnostics( // (15,36): warning CS8625: Cannot convert null literal to non-nullable reference type. // a = new B() { FA = 2, FB = null }; // 1 - Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(15, 36)); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(15, 36), + // (16,9): warning CS8602: Dereference of a possibly null reference. + // ((B)a).FA.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((B)a).FA").WithLocation(16, 9)); } [Fact] @@ -68771,17 +68771,14 @@ static void F(IB b) b.PA = 1; b.PB = null; // 1 ((IA)b).PA.ToString(); - ((IB)(object)b).PB.ToString(); // 2 + ((IB)(object)b).PB.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); comp.VerifyDiagnostics( // (14,16): warning CS8625: Cannot convert null literal to non-nullable reference type. // b.PB = null; // 1 - Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(14, 16), - // (16,9): warning CS8602: Dereference of a possibly null reference. - // ((IB)(object)b).PB.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((IB)(object)b).PB").WithLocation(16, 9)); + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(14, 16)); } [Fact] @@ -68842,13 +68839,20 @@ class Program static void F(object x, object? y) { if (((C)x).F != null) - ((C)x).F.ToString(); + ((C)x).F.ToString(); // 1 if (((C?)y)?.F != null) - ((C)y).F.ToString(); + ((C)y).F.ToString(); // 2 } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); - comp.VerifyDiagnostics(); + comp.VerifyDiagnostics( + // (11,13): warning CS8602: Dereference of a possibly null reference. + // ((C)x).F.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((C)x).F").WithLocation(11, 13), + // (13,13): warning CS8602: Dereference of a possibly null reference. + // ((C)y).F.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((C)y).F").WithLocation(13, 13) + ); } [Fact] @@ -68886,13 +68890,20 @@ class Program static void F(object x, object? y) { if (((I)x).P != null) - ((I)x).P.ToString(); + ((I)x).P.ToString(); // 1 if (((I?)y)?.P != null) - ((I)y).P.ToString(); + ((I)y).P.ToString(); // 2 } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); - comp.VerifyDiagnostics(); + comp.VerifyDiagnostics( + // (10,13): warning CS8602: Dereference of a possibly null reference. + // ((I)x).P.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((I)x).P").WithLocation(10, 13), + // (12,13): warning CS8602: Dereference of a possibly null reference. + // ((I)y).P.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "((I)y).P").WithLocation(12, 13) + ); } [Fact] @@ -69496,7 +69507,7 @@ static void F(A a, B b) { var t = ((B, B))(b, a); // 1, 2 t.Item1.ToString(); - t.Item2.ToString(); // 3 + t.Item2.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -69506,10 +69517,7 @@ static void F(A a, B b) Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "((B, B))(b, a)").WithArguments("(B b, B? a)", "(B, B)").WithLocation(12, 17), // (12,29): warning CS8600: Converting null literal or possible null value to non-nullable type. // var t = ((B, B))(b, a); // 1, 2 - Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "a").WithLocation(12, 29), - // (14,9): warning CS8602: Possible dereference of a null reference. - // t.Item2.ToString(); // 3 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t.Item2").WithLocation(14, 9)); + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "a").WithLocation(12, 29)); } [Fact] @@ -69588,20 +69596,20 @@ class Program static void F1() { var t1 = ((S, B))(new S() { F = 1 }, new A()); // 1, 2 - t1.Item1.F.ToString(); - t1.Item2.ToString(); // 3 + t1.Item1.F.ToString(); // 3 + t1.Item2.ToString(); } static void F2() { var t2 = ((S?, A))(new S() { F = 2 }, new A()); - t2.Item1.Value.F.ToString(); + t2.Item1.Value.F.ToString(); // 4, 5 t2.Item2.ToString(); } static void F3() { - var t3 = ((S?, B))(new S() { F = 3 }, new A()); // 4, 5 - t3.Item1.Value.F.ToString(); - t3.Item2.ToString(); // 6 + var t3 = ((S?, B))(new S() { F = 3 }, new A()); // 6, 7 + t3.Item1.Value.F.ToString(); // 8, 9 + t3.Item2.ToString(); } }"; var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); @@ -69612,18 +69620,27 @@ static void F3() // (16,46): warning CS8600: Converting null literal or possible null value to non-nullable type. // var t1 = ((S, B))(new S() { F = 1 }, new A()); // 1, 2 Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "new A()").WithLocation(16, 46), - // (18,9): warning CS8602: Possible dereference of a null reference. - // t1.Item2.ToString(); // 3 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t1.Item2").WithLocation(18, 9), + // (17,9): warning CS8602: Dereference of a possibly null reference. + // t1.Item1.F.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t1.Item1.F").WithLocation(17, 9), + // (23,9): warning CS8629: Nullable value type may be null. + // t2.Item1.Value.F.ToString(); // 4, 5 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "t2.Item1").WithLocation(23, 9), + // (23,9): warning CS8602: Dereference of a possibly null reference. + // t2.Item1.Value.F.ToString(); // 4, 5 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t2.Item1.Value.F").WithLocation(23, 9), // (28,18): warning CS8619: Nullability of reference types in value of type '(S?, B?)' doesn't match target type '(S?, B)'. - // var t3 = ((S?, B))(new S() { F = 3 }, new A()); // 4, 5 + // var t3 = ((S?, B))(new S() { F = 3 }, new A()); // 6, 7 Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "((S?, B))(new S() { F = 3 }, new A())").WithArguments("(S?, B?)", "(S?, B)").WithLocation(28, 18), // (28,47): warning CS8600: Converting null literal or possible null value to non-nullable type. - // var t3 = ((S?, B))(new S() { F = 3 }, new A()); // 4, 5 + // var t3 = ((S?, B))(new S() { F = 3 }, new A()); // 6, 7 Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "new A()").WithLocation(28, 47), - // (30,9): warning CS8602: Possible dereference of a null reference. - // t3.Item2.ToString(); // 6 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t3.Item2").WithLocation(30, 9)); + // (29,9): warning CS8629: Nullable value type may be null. + // t3.Item1.Value.F.ToString(); // 8, 9 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "t3.Item1").WithLocation(29, 9), + // (29,9): warning CS8602: Dereference of a possibly null reference. + // t3.Item1.Value.F.ToString(); // 8, 9 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t3.Item1.Value.F").WithLocation(29, 9)); } [Fact] diff --git a/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs b/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs index dd033467f8027..b17b318e42b5b 100644 --- a/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs +++ b/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs @@ -26,6 +26,9 @@ public sealed class SymbolEqualityComparer : IEqualityComparer // Internal only comparisons: internal readonly static SymbolEqualityComparer ConsiderEverything = new SymbolEqualityComparer(TypeCompareKind.ConsiderEverything); + // Internal only comparisons: + internal readonly static SymbolEqualityComparer IgnoreEverything = new SymbolEqualityComparer(TypeCompareKind.AllIgnoreOptions); + internal TypeCompareKind CompareKind { get; } internal SymbolEqualityComparer(TypeCompareKind compareKind) From 0126b8fce96519aa36de0f634782d6194f41c471 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Fri, 10 Jan 2020 17:19:28 -0800 Subject: [PATCH 04/10] Stop using syntax for semantics in NullableWalker. --- .../Portable/FlowAnalysis/NullableWalker.cs | 2 +- .../FlowAnalysis/NullableWalker_Patterns.cs | 2 +- .../Semantics/NullableReferenceTypesTests.cs | 70 ++++++++++++++++++- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 8d7238e46f3f9..321260da9b2fc 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -7163,7 +7163,7 @@ public override void VisitForEachIterationVariables(BoundForEachStatement node) ReportNullabilityMismatchInAssignment(foreachSyntax.Type, sourceType, destinationType); } } - else if (node.Syntax is ForEachStatementSyntax { Type: { IsVar: true } }) + else if (iterationVariable is SourceLocalSymbol { IsVar: true }) { // foreach (var variable in collection) _variableTypes[iterationVariable] = sourceState.ToTypeWithAnnotations(); diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs index 5c1dd13d172fc..6f4d2068985fc 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs @@ -399,7 +399,7 @@ protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastS if (_variableTypes.TryGetValue(local, out var existingType)) { // merge inferred nullable annotation from different branches of the decision tree - _variableTypes[local] = TypeWithAnnotations.Create(existingType.Type, existingType.NullableAnnotation.Join(inferredType.NullableAnnotation)); + _variableTypes[local] = TypeWithAnnotations.Create(inferredType.Type, existingType.NullableAnnotation.Join(inferredType.NullableAnnotation)); } else { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 0f0bbd2530b1d..00b5b7fc10dee 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -120268,7 +120268,7 @@ public void M7(C? a, bool b) [Fact] [WorkItem(39220, "https://github.com/dotnet/roslyn/issues/39220")] - public void GotoMayCauseAnotherAnalysisPass() + public void GotoMayCauseAnotherAnalysisPass_01() { var source = @"#nullable enable @@ -120307,5 +120307,73 @@ class C Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "prop").WithLocation(11, 9) ); } + + [Fact] + [WorkItem(40904, "https://github.com/dotnet/roslyn/issues/40904")] + public void GotoMayCauseAnotherAnalysisPass_02() + { + var source = +@"#nullable enable +public class C { + public void M(bool b) { + string? s = ""x""; + if (b) goto L2; +L1: + var x = Create(s); + x.F.ToString(); // warning: x.F might be null. +L2: + s = null; + goto L1; + } + + private G Create(T t) where T : class? => new G(t); +} + +class G where T : class? +{ + public G(T f) => F = f; + public T F; +}"; + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (8,9): warning CS8602: Dereference of a possibly null reference. + // x.F.ToString(); // warning: x.F might be null. + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x.F").WithLocation(8, 9) + ); + } + + [Fact(Skip = "https://github.com/dotnet/roslyn/issues/40904")] + [WorkItem(40904, "https://github.com/dotnet/roslyn/issues/40904")] + public void GotoMayCauseAnotherAnalysisPass_03() + { + var source = +@"#nullable enable +public class C { + public void M(bool b) { + string? s = ""x""; + if (b) goto L2; +L1: + _ = Create(s) is var x; + x.F.ToString(); // warning: x.F might be null. +L2: + s = null; + goto L1; + } + + private G Create(T t) where T : class? => new G(t); +} + +class G where T : class? +{ + public G(T f) => F = f; + public T F; +}"; + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (8,9): warning CS8602: Dereference of a possibly null reference. + // x.F.ToString(); // warning: x.F might be null. + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x.F").WithLocation(8, 9) + ); + } } } From cadffb608c8b93ddfe1aab766c0b1c82f1325585 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Mon, 13 Jan 2020 15:10:02 -0800 Subject: [PATCH 05/10] Hangle an error recovery scenario more gracefully. --- src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs | 2 +- src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 91fcd884dc576..6b657b897eef9 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1120,7 +1120,7 @@ protected BoundLocalDeclaration BindVariableDeclaration( syntax: associatedSyntaxNode, localSymbol: localSymbol, declaredTypeOpt: boundDeclType, - initializerOpt: hasErrors ? BindToTypeForErrorRecovery(initializerOpt) : initializerOpt, + initializerOpt: hasErrors ? BindToTypeForErrorRecovery(initializerOpt).WithHasErrors() : initializerOpt, argumentsOpt: arguments, inferredType: isVar, hasErrors: hasErrors); diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 321260da9b2fc..4297ca9aeb3fa 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -868,7 +868,7 @@ protected override int MakeSlot(BoundExpression node) TypeSymbol nodeType = node.Type; HashSet discardedUseSiteDiagnostics = null; var conversionsWithoutNullability = this.compilation.Conversions; - Debug.Assert(nodeType.IsErrorType() || + Debug.Assert(node.HasErrors || nodeType.IsErrorType() || conversionsWithoutNullability.HasIdentityOrImplicitReferenceConversion(slotType, nodeType, ref discardedUseSiteDiagnostics) || conversionsWithoutNullability.HasBoxingConversion(slotType, nodeType, ref discardedUseSiteDiagnostics)); } From f3c22b0bdb737171e727d86b90b7c0f41b338c2f Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Mon, 13 Jan 2020 16:05:40 -0800 Subject: [PATCH 06/10] j'adoube --- src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 6b657b897eef9..aa63c75f88d4d 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1120,7 +1120,7 @@ protected BoundLocalDeclaration BindVariableDeclaration( syntax: associatedSyntaxNode, localSymbol: localSymbol, declaredTypeOpt: boundDeclType, - initializerOpt: hasErrors ? BindToTypeForErrorRecovery(initializerOpt).WithHasErrors() : initializerOpt, + initializerOpt: hasErrors ? BindToTypeForErrorRecovery(initializerOpt)?.WithHasErrors() : initializerOpt, argumentsOpt: arguments, inferredType: isVar, hasErrors: hasErrors); From f140eb3878f046289442d0542184eed7746f3ec6 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Mon, 13 Jan 2020 17:21:10 -0800 Subject: [PATCH 07/10] Adjust tests to stop expecting cascaded warnings in unassigned variable analysis. --- .../Semantic/Semantics/BindingAsyncTests.cs | 5 +---- .../Semantic/Semantics/NameCollisionTests.cs | 3 --- .../Test/Semantic/Semantics/OutVarTests.cs | 18 ------------------ .../Semantics/PatternMatchingTests_Scope.cs | 18 ------------------ .../Semantic/Semantics/SemanticErrorTests.cs | 15 +++------------ .../Semantic/Semantics/SpanStackSafetyTests.cs | 10 ++-------- .../Semantics/TargetTypedDefaultTests.cs | 5 +---- 7 files changed, 7 insertions(+), 67 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/BindingAsyncTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/BindingAsyncTests.cs index a46747408358e..bcff2924c6973 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/BindingAsyncTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/BindingAsyncTests.cs @@ -3192,10 +3192,7 @@ async Task M1(bool truth) CreateCompilationWithMscorlib45(source).VerifyDiagnostics( // (9,9): error CS4012: Parameters or locals of type 'System.TypedReference' cannot be declared in async methods or lambda expressions // var tr = new TypedReference(); - Diagnostic(ErrorCode.ERR_BadSpecialByRefLocal, "var").WithArguments("System.TypedReference"), - // (9,13): warning CS0219: The variable 'tr' is assigned but its value is never used - // var tr = new TypedReference(); - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "tr").WithArguments("tr")); + Diagnostic(ErrorCode.ERR_BadSpecialByRefLocal, "var").WithArguments("System.TypedReference")); } [Fact] diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs index f34603f7cb998..c06a6bba1c934 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs @@ -1335,9 +1335,6 @@ void Method1(Class name2 = null) // (20,20): error CS0136: A local or parameter named 'name2' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // using (var name2 = new Class()) // 0136 on name2. Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "name2").WithArguments("name2").WithLocation(20, 20), - // (17,21): warning CS0219: The variable 'name1' is assigned but its value is never used - // int name1 = 2; // 0136 on name1 - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "name1").WithArguments("name1").WithLocation(17, 21), // (22,17): warning CS0219: The variable 'name1' is assigned but its value is never used // int name1 = 2; Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "name1").WithArguments("name1").WithLocation(22, 17), diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs index 6fee025f24a46..ad9801ba23c95 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs @@ -4856,9 +4856,6 @@ static bool TakeOutParam(object y, out int x) // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,79): error CS0128: A local variable named 'x8' is already defined in this scope // Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 79), @@ -13156,9 +13153,6 @@ static bool TakeOutParam(T y, out T x) // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,86): error CS0128: A local variable named 'x8' is already defined in this scope // return Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 86), @@ -15077,9 +15071,6 @@ static bool TakeOutParam(T y, out T x) // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,85): error CS0128: A local variable named 'x8' is already defined in this scope // throw Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 85), @@ -16921,9 +16912,6 @@ static bool TakeOutParam(T y, out T x) // (41,13): error CS0128: A local variable named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(41, 13), - // (41,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(41, 13), // (61,92): error CS0128: A local variable named 'x8' is already defined in this scope // yield return Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(61, 92), @@ -17334,9 +17322,6 @@ static bool TakeOutParam(object y, out int x) // (38,1): warning CS0164: This label has not been referenced // a: Dummy(TakeOutParam(true, out var x5), x5); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(38, 1), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,79): error CS0128: A local variable named 'x8' is already defined in this scope // a: Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 79), @@ -19614,9 +19599,6 @@ static void Test2(object x, System.ArgIterator y) // (9,9): error CS4012: Parameters or locals of type 'ArgIterator' cannot be declared in async methods or lambda expressions. // var x = default(System.ArgIterator); Diagnostic(ErrorCode.ERR_BadSpecialByRefLocal, "var").WithArguments("System.ArgIterator").WithLocation(9, 9), - // (9,13): warning CS0219: The variable 'x' is assigned but its value is never used - // var x = default(System.ArgIterator); - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x").WithArguments("x").WithLocation(9, 13), // (6,16): warning CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread. // async void Test() Diagnostic(ErrorCode.WRN_AsyncLacksAwaits, "Test").WithLocation(6, 16) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs index 14a90f0b4d53c..5010b7ebd4722 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs @@ -184,9 +184,6 @@ void Test12() // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,48): error CS0128: A local variable named 'x8' is already defined in this scope // Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 48), @@ -566,9 +563,6 @@ object Test12() // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,55): error CS0128: A local variable named 'x8' is already defined in this scope // return Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 55), @@ -836,9 +830,6 @@ void Test12() // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,54): error CS0128: A local variable named 'x8' is already defined in this scope // throw Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 54), @@ -10069,9 +10060,6 @@ IEnumerable Test12() // (41,13): error CS0128: A local variable named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(41, 13), - // (41,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(41, 13), // (61,61): error CS0128: A local variable named 'x8' is already defined in this scope // yield return Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(61, 61), @@ -10566,9 +10554,6 @@ void Test12() // (38,1): warning CS0164: This label has not been referenced // a: Dummy(true is var x5, x5); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(38, 1), - // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), // (59,48): error CS0128: A local variable named 'x8' is already defined in this scope // a: Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 48), @@ -13921,9 +13906,6 @@ void Test11() // (37,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(37, 13), - // (37,13): warning CS0219: The variable 'x5' is assigned but its value is never used - // var x5 = 11; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(37, 13), // (38,12): error CS0270: Array size cannot be specified in a variable declaration (try initializing with a 'new' expression) // int[x5] _9; Diagnostic(ErrorCode.ERR_ArraySizeInDeclaration, "[x5]").WithLocation(38, 12), diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs index dd19461cdb340..c452d0cc86204 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs @@ -3678,10 +3678,7 @@ public static void Main() VerifyDiagnostics( // (9,14): error CS0128: A local variable named 'i' is already defined in this scope // int i = 2; // CS0128 - Diagnostic(ErrorCode.ERR_LocalDuplicate, "i").WithArguments("i"), - // (9,14): warning CS0219: The variable 'i' is assigned but its value is never used - // int i = 2; // CS0128 - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "i").WithArguments("i") + Diagnostic(ErrorCode.ERR_LocalDuplicate, "i").WithArguments("i") ); } @@ -11871,10 +11868,7 @@ static void M() {} Diagnostic(ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, "del = delegate(string a) { return -1; }").WithArguments("anonymous method"), // (11,13): error CS0815: Cannot assign void to an implicitly-typed variable // var v = M(); // CS0815 - Diagnostic(ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, "v = M()").WithArguments("void"), - // (9,13): warning CS0219: The variable 'p' is assigned but its value is never used - // var p = null;//CS0815 - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "p").WithArguments("p")); + Diagnostic(ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, "v = M()").WithArguments("void")); } [Fact] @@ -17366,10 +17360,7 @@ static void Test() where T : class comp.VerifyDiagnostics( // (6,25): error CS1959: 'x' is of type 'T'. The type specified in a constant declaration must be sbyte, byte, short, ushort, int, uint, long, ulong, char, float, double, decimal, bool, string, an enum-type, or a reference-type. // const T x = null; // CS1959 - Diagnostic(ErrorCode.ERR_InvalidConstantDeclarationType, "null").WithArguments("x", "T"), - // (6,21): warning CS0219: The variable 'x' is assigned but its value is never used - // const T x = null; // CS1959 - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x").WithArguments("x") + Diagnostic(ErrorCode.ERR_InvalidConstantDeclarationType, "null").WithArguments("x", "T") ); } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/SpanStackSafetyTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/SpanStackSafetyTests.cs index 8c5cf6deca6d9..97efe25a94f54 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/SpanStackSafetyTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/SpanStackSafetyTests.cs @@ -988,10 +988,7 @@ public static async Task M1() comp.VerifyDiagnostics( // (13,9): error CS4012: Parameters or locals of type 'Span' cannot be declared in async methods or lambda expressions. // Span local = default(Span); - Diagnostic(ErrorCode.ERR_BadSpecialByRefLocal, "Span").WithArguments("System.Span").WithLocation(13, 9), - // (13,19): warning CS0219: The variable 'local' is assigned but its value is never used - // Span local = default(Span); - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "local").WithArguments("local").WithLocation(13, 19) + Diagnostic(ErrorCode.ERR_BadSpecialByRefLocal, "Span").WithArguments("System.Span").WithLocation(13, 9) ); comp = CreateCompilationWithMscorlibAndSpan(text, TestOptions.DebugExe); @@ -999,10 +996,7 @@ public static async Task M1() comp.VerifyDiagnostics( // (13,9): error CS4012: Parameters or locals of type 'Span' cannot be declared in async methods or lambda expressions. // Span local = default(Span); - Diagnostic(ErrorCode.ERR_BadSpecialByRefLocal, "Span").WithArguments("System.Span").WithLocation(13, 9), - // (13,19): warning CS0219: The variable 'local' is assigned but its value is never used - // Span local = default(Span); - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "local").WithArguments("local").WithLocation(13, 19) + Diagnostic(ErrorCode.ERR_BadSpecialByRefLocal, "Span").WithArguments("System.Span").WithLocation(13, 9) ); } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs index ce7b4dfa73ab9..4e5e791a2a7b6 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs @@ -695,10 +695,7 @@ static void M() Diagnostic(ErrorCode.ERR_DefaultLiteralNoTargetType, "default").WithLocation(6, 17), // (7,13): error CS0815: Cannot assign to an implicitly-typed variable // var y = null; - Diagnostic(ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, "y = null").WithArguments("").WithLocation(7, 13), - // (7,13): warning CS0219: The variable 'y' is assigned but its value is never used - // var y = null; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "y").WithArguments("y").WithLocation(7, 13) + Diagnostic(ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, "y = null").WithArguments("").WithLocation(7, 13) ); var tree = comp.SyntaxTrees.First(); From 16972a562dc2482bc9e0ab560a684d96f5f69e1c Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Tue, 14 Jan 2020 15:01:01 -0800 Subject: [PATCH 08/10] Refine hasErrors cascading to accomodate IDE tests. --- .../Portable/Binder/Binder_Statements.cs | 5 +- .../Semantic/Semantics/NameCollisionTests.cs | 3 + .../Test/Semantic/Semantics/OutVarTests.cs | 67 ++++++++++------- .../Semantics/PatternMatchingTests_Scope.cs | 71 ++++++++++++------- .../Semantic/Semantics/SemanticErrorTests.cs | 7 +- 5 files changed, 96 insertions(+), 57 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index aa63c75f88d4d..990cc41ac4d79 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -943,7 +943,8 @@ protected BoundLocalDeclaration BindVariableDeclaration( // Check for variable declaration errors. // Use the binder that owns the scope for the local because this (the current) binder // might own nested scope. - bool hasErrors = localSymbol.ScopeBinder.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics); + bool nameConflict = localSymbol.ScopeBinder.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics); + bool hasErrors = false; var containingMethod = this.ContainingMemberOrLambda as MethodSymbol; if (containingMethod != null && containingMethod.IsAsync && localSymbol.RefKind != RefKind.None) @@ -1123,7 +1124,7 @@ protected BoundLocalDeclaration BindVariableDeclaration( initializerOpt: hasErrors ? BindToTypeForErrorRecovery(initializerOpt)?.WithHasErrors() : initializerOpt, argumentsOpt: arguments, inferredType: isVar, - hasErrors: hasErrors); + hasErrors: hasErrors | nameConflict); } internal ImmutableArray BindDeclaratorArguments(VariableDeclaratorSyntax declarator, DiagnosticBag diagnostics) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs index c06a6bba1c934..17302bd507747 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NameCollisionTests.cs @@ -1332,6 +1332,9 @@ void Method1(Class name2 = null) // (17,21): error CS0136: A local or parameter named 'name1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // int name1 = 2; // 0136 on name1 Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "name1").WithArguments("name1").WithLocation(17, 21), + // (17,21): warning CS0219: The variable 'name1' is assigned but its value is never used + // int name1 = 2; // 0136 on name1 + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "name1").WithArguments("name1").WithLocation(17, 21), // (20,20): error CS0136: A local or parameter named 'name2' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // using (var name2 = new Class()) // 0136 on name2. Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "name2").WithArguments("name2").WithLocation(20, 20), diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs index ad9801ba23c95..da99a85b8fbfb 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs @@ -4841,7 +4841,7 @@ static bool TakeOutParam(object y, out int x) // (14,46): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 46), - // (16,42): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,42): error CS0128: A local variable or function named 'x1' is already defined in this scope // Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 42), // (21,15): error CS0841: Cannot use local variable 'x2' before it is declared @@ -4850,13 +4850,16 @@ static bool TakeOutParam(object y, out int x) // (26,42): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x3), x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(26, 42), - // (33,42): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,42): error CS0128: A local variable or function named 'x4' is already defined in this scope // Dummy(TakeOutParam(true, out var x4), x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 42), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), - // (59,79): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,79): error CS0128: A local variable or function named 'x8' is already defined in this scope // Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 79), // (79,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -13132,7 +13135,7 @@ static bool TakeOutParam(T y, out T x) // (14,53): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // return Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 53), - // (16,49): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,49): error CS0128: A local variable or function named 'x1' is already defined in this scope // return Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 49), // (14,13): warning CS0162: Unreachable code detected @@ -13144,16 +13147,19 @@ static bool TakeOutParam(T y, out T x) // (26,49): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // return Dummy(TakeOutParam(true, out var x3), x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(26, 49), - // (33,49): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,49): error CS0128: A local variable or function named 'x4' is already defined in this scope // return Dummy(TakeOutParam(true, out var x4), x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 49), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (59,86): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,86): error CS0128: A local variable or function named 'x8' is already defined in this scope // return Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 86), // (79,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -15053,7 +15059,7 @@ static bool TakeOutParam(T y, out T x) // (14,52): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // throw Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 52), - // (16,48): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,48): error CS0128: A local variable or function named 'x1' is already defined in this scope // throw Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 48), // (21,21): error CS0841: Cannot use local variable 'x2' before it is declared @@ -15062,16 +15068,19 @@ static bool TakeOutParam(T y, out T x) // (26,48): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // throw Dummy(TakeOutParam(true, out var x3), x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(26, 48), - // (33,48): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,48): error CS0128: A local variable or function named 'x4' is already defined in this scope // throw Dummy(TakeOutParam(true, out var x4), x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 48), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (59,85): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,85): error CS0128: A local variable or function named 'x8' is already defined in this scope // throw Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 85), // (79,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -16897,7 +16906,7 @@ static bool TakeOutParam(T y, out T x) // (16,59): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // yield return Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(16, 59), - // (18,55): error CS0128: A local variable named 'x1' is already defined in this scope + // (18,55): error CS0128: A local variable or function named 'x1' is already defined in this scope // yield return Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(18, 55), // (23,28): error CS0841: Cannot use local variable 'x2' before it is declared @@ -16906,13 +16915,16 @@ static bool TakeOutParam(T y, out T x) // (28,55): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // yield return Dummy(TakeOutParam(true, out var x3), x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(28, 55), - // (35,55): error CS0128: A local variable named 'x4' is already defined in this scope + // (35,55): error CS0128: A local variable or function named 'x4' is already defined in this scope // yield return Dummy(TakeOutParam(true, out var x4), x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(35, 55), - // (41,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (41,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(41, 13), - // (61,92): error CS0128: A local variable named 'x8' is already defined in this scope + // (41,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(41, 13), + // (61,92): error CS0128: A local variable or function named 'x8' is already defined in this scope // yield return Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(61, 92), // (72,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -17277,16 +17289,10 @@ static bool TakeOutParam(object y, out int x) var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); compilation.VerifyDiagnostics( - // (65,1): error CS1023: Embedded statement cannot be a declaration or labeled statement - // a: Dummy(TakeOutParam(true, out var x9), x9); - Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(TakeOutParam(true, out var x9), x9);").WithLocation(65, 1), - // (73,1): error CS1023: Embedded statement cannot be a declaration or labeled statement - // a: Dummy(TakeOutParam(true, out var x10), x10); - Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(TakeOutParam(true, out var x10), x10);").WithLocation(73, 1), // (14,46): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // b: Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 46), - // (16,42): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,42): error CS0128: A local variable or function named 'x1' is already defined in this scope // c: Dummy(TakeOutParam(true, out var x1), x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 42), // (12,1): warning CS0164: This label has not been referenced @@ -17310,27 +17316,36 @@ static bool TakeOutParam(object y, out int x) // (26,1): warning CS0164: This label has not been referenced // a: Dummy(TakeOutParam(true, out var x3), x3); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(26, 1), - // (33,42): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,42): error CS0128: A local variable or function named 'x4' is already defined in this scope // a: Dummy(TakeOutParam(true, out var x4), x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 42), // (33,1): warning CS0164: This label has not been referenced // a: Dummy(TakeOutParam(true, out var x4), x4); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(33, 1), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), // (38,1): warning CS0164: This label has not been referenced // a: Dummy(TakeOutParam(true, out var x5), x5); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(38, 1), - // (59,79): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,79): error CS0128: A local variable or function named 'x8' is already defined in this scope // a: Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 79), // (59,1): warning CS0164: This label has not been referenced // a: Dummy(TakeOutParam(true, out var x8), x8, TakeOutParam(false, out var x8), x8); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(59, 1), + // (65,1): error CS1023: Embedded statement cannot be a declaration or labeled statement + // a: Dummy(TakeOutParam(true, out var x9), x9); + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(TakeOutParam(true, out var x9), x9);").WithLocation(65, 1), // (65,1): warning CS0164: This label has not been referenced // a: Dummy(TakeOutParam(true, out var x9), x9); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(65, 1), + // (73,1): error CS1023: Embedded statement cannot be a declaration or labeled statement + // a: Dummy(TakeOutParam(true, out var x10), x10); + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(TakeOutParam(true, out var x10), x10);").WithLocation(73, 1), // (73,1): warning CS0164: This label has not been referenced // a: Dummy(TakeOutParam(true, out var x10), x10); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(73, 1), diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs index 5010b7ebd4722..235621da90123 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs @@ -169,7 +169,7 @@ void Test12() // (14,31): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 31), - // (16,27): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,27): error CS0128: A local variable or function named 'x1' is already defined in this scope // Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 27), // (21,15): error CS0841: Cannot use local variable 'x2' before it is declared @@ -178,13 +178,16 @@ void Test12() // (26,27): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x3, x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(26, 27), - // (33,27): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,27): error CS0128: A local variable or function named 'x4' is already defined in this scope // Dummy(true is var x4, x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 27), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), - // (59,48): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,48): error CS0128: A local variable or function named 'x8' is already defined in this scope // Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 48), // (79,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -542,7 +545,7 @@ object Test12() // (14,38): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // return Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 38), - // (16,34): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,34): error CS0128: A local variable or function named 'x1' is already defined in this scope // return Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 34), // (14,13): warning CS0162: Unreachable code detected @@ -554,16 +557,19 @@ object Test12() // (26,34): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // return Dummy(true is var x3, x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(26, 34), - // (33,34): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,34): error CS0128: A local variable or function named 'x4' is already defined in this scope // return Dummy(true is var x4, x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 34), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (59,55): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,55): error CS0128: A local variable or function named 'x8' is already defined in this scope // return Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 55), // (79,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -812,7 +818,7 @@ void Test12() // (14,37): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // throw Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 37), - // (16,33): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,33): error CS0128: A local variable or function named 'x1' is already defined in this scope // throw Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 33), // (21,21): error CS0841: Cannot use local variable 'x2' before it is declared @@ -821,16 +827,19 @@ void Test12() // (26,33): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // throw Dummy(true is var x3, x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(26, 33), - // (33,33): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,33): error CS0128: A local variable or function named 'x4' is already defined in this scope // throw Dummy(true is var x4, x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 33), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), // (39,9): warning CS0162: Unreachable code detected // var x5 = 11; Diagnostic(ErrorCode.WRN_UnreachableCode, "var").WithLocation(39, 9), - // (59,54): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,54): error CS0128: A local variable or function named 'x8' is already defined in this scope // throw Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 54), // (79,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -10045,7 +10054,7 @@ IEnumerable Test12() // (16,44): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // yield return Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(16, 44), - // (18,40): error CS0128: A local variable named 'x1' is already defined in this scope + // (18,40): error CS0128: A local variable or function named 'x1' is already defined in this scope // yield return Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(18, 40), // (23,28): error CS0841: Cannot use local variable 'x2' before it is declared @@ -10054,13 +10063,16 @@ IEnumerable Test12() // (28,40): error CS0136: A local or parameter named 'x3' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // yield return Dummy(true is var x3, x3); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x3").WithArguments("x3").WithLocation(28, 40), - // (35,40): error CS0128: A local variable named 'x4' is already defined in this scope + // (35,40): error CS0128: A local variable or function named 'x4' is already defined in this scope // yield return Dummy(true is var x4, x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(35, 40), - // (41,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (41,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(41, 13), - // (61,61): error CS0128: A local variable named 'x8' is already defined in this scope + // (41,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(41, 13), + // (61,61): error CS0128: A local variable or function named 'x8' is already defined in this scope // yield return Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(61, 61), // (72,15): error CS0841: Cannot use local variable 'x11' before it is declared @@ -10512,16 +10524,9 @@ void Test12() var compilation = CreateCompilation(source, options: TestOptions.DebugExe); compilation.VerifyDiagnostics( - // (65,1): error CS1023: Embedded statement cannot be a declaration or labeled statement - // a: Dummy(true is var x9, x9); - Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(true is var x9, x9);").WithLocation(65, 1), - // (73,1): error CS1023: Embedded statement cannot be a declaration or labeled statement - // a: Dummy(true is var x10, x10); - Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(true is var x10, x10);").WithLocation(73, 1), - // (14,31): error CS0136: A local or parameter named 'x1' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // b: Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x1").WithArguments("x1").WithLocation(14, 31), - // (16,27): error CS0128: A local variable named 'x1' is already defined in this scope + // (16,27): error CS0128: A local variable or function named 'x1' is already defined in this scope // c: Dummy(true is var x1, x1); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x1").WithArguments("x1").WithLocation(16, 27), // (12,1): warning CS0164: This label has not been referenced @@ -10542,27 +10547,36 @@ void Test12() // (26,1): warning CS0164: This label has not been referenced // a: Dummy(true is var x3, x3); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(26, 1), - // (33,27): error CS0128: A local variable named 'x4' is already defined in this scope + // (33,27): error CS0128: A local variable or function named 'x4' is already defined in this scope // a: Dummy(true is var x4, x4); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(33, 27), // (33,1): warning CS0164: This label has not been referenced // a: Dummy(true is var x4, x4); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(33, 1), - // (39,13): error CS0128: A local variable named 'x5' is already defined in this scope + // (39,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(39, 13), // (38,1): warning CS0164: This label has not been referenced // a: Dummy(true is var x5, x5); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(38, 1), - // (59,48): error CS0128: A local variable named 'x8' is already defined in this scope + // (39,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(39, 13), + // (59,48): error CS0128: A local variable or function named 'x8' is already defined in this scope // a: Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(59, 48), // (59,1): warning CS0164: This label has not been referenced // a: Dummy(true is var x8, x8, false is var x8, x8); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(59, 1), + // (65,1): error CS1023: Embedded statement cannot be a declaration or labeled statement + // a: Dummy(true is var x9, x9); + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(true is var x9, x9);").WithLocation(65, 1), // (65,1): warning CS0164: This label has not been referenced // a: Dummy(true is var x9, x9); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(65, 1), + // (73,1): error CS1023: Embedded statement cannot be a declaration or labeled statement + // a: Dummy(true is var x10, x10); + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "a: Dummy(true is var x10, x10);").WithLocation(73, 1), // (73,1): warning CS0164: This label has not been referenced // a: Dummy(true is var x10, x10); Diagnostic(ErrorCode.WRN_UnreferencedLabel, "a").WithLocation(73, 1), @@ -13906,6 +13920,9 @@ void Test11() // (37,13): error CS0128: A local variable or function named 'x5' is already defined in this scope // var x5 = 11; Diagnostic(ErrorCode.ERR_LocalDuplicate, "x5").WithArguments("x5").WithLocation(37, 13), + // (37,13): warning CS0219: The variable 'x5' is assigned but its value is never used + // var x5 = 11; + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x5").WithArguments("x5").WithLocation(37, 13), // (38,12): error CS0270: Array size cannot be specified in a variable declaration (try initializing with a 'new' expression) // int[x5] _9; Diagnostic(ErrorCode.ERR_ArraySizeInDeclaration, "[x5]").WithLocation(38, 12), diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs index c452d0cc86204..f9a28b67822cc 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs @@ -3676,9 +3676,12 @@ public static void Main() }"; CreateCompilation(text). VerifyDiagnostics( - // (9,14): error CS0128: A local variable named 'i' is already defined in this scope + // (9,14): error CS0128: A local variable or function named 'i' is already defined in this scope // int i = 2; // CS0128 - Diagnostic(ErrorCode.ERR_LocalDuplicate, "i").WithArguments("i") + Diagnostic(ErrorCode.ERR_LocalDuplicate, "i").WithArguments("i").WithLocation(9, 14), + // (9,14): warning CS0219: The variable 'i' is assigned but its value is never used + // int i = 2; // CS0128 + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "i").WithArguments("i").WithLocation(9, 14) ); } From 5217bea7a3cd834dcec959ec543440a1e7b69d93 Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Tue, 14 Jan 2020 15:14:09 -0800 Subject: [PATCH 09/10] Fix typo --- src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index df9635cab4b5b..a408d541bf1c1 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -1789,7 +1789,7 @@ private void DeclareLocal(LocalSymbol local) if (slot > 0) { PopulateOneSlot(ref this.State, slot); - InheritDefaultState(_variableTypes.TryGetValue(local, out var typeWithAnootations) ? typeWithAnootations.Type : local.Type, slot); + InheritDefaultState(_variableTypes.TryGetValue(local, out var typeWithAnnotations) ? typeWithAnnotations.Type : local.Type, slot); } } } From 35797ed9000e68961624bdae6338a357cd85e5be Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Thu, 16 Jan 2020 13:09:49 -0800 Subject: [PATCH 10/10] Minor changes per code review, plus fix a concurrency bug. --- .../Binder/Semantics/Conversions/ConversionsBase.cs | 3 ++- .../DefiniteAssignment.VariableIdentifier.cs | 2 +- .../CSharp/Portable/FlowAnalysis/NullableWalker.cs | 13 ++++++------- .../FlowAnalysis/NullableWalker_Patterns.cs | 3 +-- .../Semantics/NullableReferenceTypesTests.cs | 2 +- .../Core/Portable/Symbols/SymbolEqualityComparer.cs | 3 --- 6 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs index fdacd27123008..b253d32a71283 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Threading; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; @@ -49,7 +50,7 @@ internal ConversionsBase WithNullability(bool includeNullability) } if (_lazyOtherNullability == null) { - _lazyOtherNullability = WithNullabilityCore(includeNullability); + Interlocked.CompareExchange(ref _lazyOtherNullability, WithNullabilityCore(includeNullability), null); } Debug.Assert(_lazyOtherNullability.IncludeNullability == includeNullability); Debug.Assert(_lazyOtherNullability._lazyOtherNullability == this); diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs index 06ee5ff7b195b..e7bed0fc0359b 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs @@ -80,7 +80,7 @@ public bool Equals(VariableIdentifier other) return true; } - return Symbol.Equals(other.Symbol, SymbolEqualityComparer.IgnoreEverything.CompareKind); + return Symbol.Equals(other.Symbol, TypeCompareKind.AllIgnoreOptions); } public override bool Equals(object obj) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index a408d541bf1c1..a03a29c5a17c4 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -167,7 +167,7 @@ public VisitArgumentResult(VisitResult visitResult, Optional stateFo /// private readonly SnapshotManager.Builder? _snapshotBuilderOpt; -#nullable disable +#nullable restore // https://github.com/dotnet/roslyn/issues/35043: remove this when all expression are supported private bool _disableNullabilityAnalysis; @@ -864,7 +864,7 @@ protected override int MakeSlot(BoundExpression node) if (result != -1) { // Check that the slot represents a value of an equivalent type to the node - TypeSymbol slotType = variableBySlot[result].Symbol.GetTypeOrReturnType().Type; + TypeSymbol slotType = NominalSlotType(result); TypeSymbol nodeType = node.Type; HashSet discardedUseSiteDiagnostics = null; var conversionsWithoutNullability = this.compilation.Conversions; @@ -1283,7 +1283,7 @@ private bool IsSlotMember(int slot, Symbol possibleMember) TypeSymbol possibleBase = possibleMember.ContainingType; TypeSymbol possibleDerived = NominalSlotType(slot); HashSet discardedUseSiteDiagnostics = null; - var conversionsWithoutNullability = this.compilation.Conversions; + var conversionsWithoutNullability = _conversions.WithNullability(false); return conversionsWithoutNullability.HasIdentityOrImplicitReferenceConversion(possibleDerived, possibleBase, ref discardedUseSiteDiagnostics) || conversionsWithoutNullability.HasBoxingConversion(possibleDerived, possibleBase, ref discardedUseSiteDiagnostics); @@ -1426,7 +1426,7 @@ protected override LocalState ReachableBottomState() private void EnterParameters() { - if (!(CurrentSymbol is MethodSymbol methodSymbol)) + if (!(_symbol is MethodSymbol methodSymbol)) { return; } @@ -1789,7 +1789,7 @@ private void DeclareLocal(LocalSymbol local) if (slot > 0) { PopulateOneSlot(ref this.State, slot); - InheritDefaultState(_variableTypes.TryGetValue(local, out var typeWithAnnotations) ? typeWithAnnotations.Type : local.Type, slot); + InheritDefaultState(GetDeclaredLocalResult(local).Type, slot); } } } @@ -6968,8 +6968,7 @@ private int GetNullableOfTValueSlot(TypeSymbol containingType, int containingSlo var getValue = (MethodSymbol)compilation.GetSpecialTypeMember(SpecialMember.System_Nullable_T_get_Value); valueProperty = getValue?.AsMember((NamedTypeSymbol)containingType)?.AssociatedSymbol; - var result = (valueProperty is null) ? -1 : GetOrCreateSlot(valueProperty, containingSlot, forceSlotEvenIfEmpty: forceSlotEvenIfEmpty); - return result; + return (valueProperty is null) ? -1 : GetOrCreateSlot(valueProperty, containingSlot, forceSlotEvenIfEmpty: forceSlotEvenIfEmpty); } protected override void VisitForEachExpression(BoundForEachStatement node) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs index b24fe9ece447c..3071e2c2aa3d1 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs @@ -465,8 +465,7 @@ void addToTempMap(BoundDagTemp output, int slot, TypeSymbol type) } else { - var slotType = NominalSlotType(slot); - Debug.Assert(slotType.IsErrorType() || isDerivedType(slotType, type)); + Debug.Assert(NominalSlotType(slot) is var slotType && (slotType.IsErrorType() || isDerivedType(slotType, type))); tempMap.Add(output, (slot, type)); } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 2d589ea5b24cd..efde23564a3e5 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -120635,7 +120635,7 @@ static void Test(string? s) heck: var c = GetC(s); var prop = c.Property; - prop.ToString(); // BOOM (after goto) + prop.ToString(); // Dereference of null value (after goto) s = null; goto heck; } diff --git a/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs b/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs index b17b318e42b5b..dd033467f8027 100644 --- a/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs +++ b/src/Compilers/Core/Portable/Symbols/SymbolEqualityComparer.cs @@ -26,9 +26,6 @@ public sealed class SymbolEqualityComparer : IEqualityComparer // Internal only comparisons: internal readonly static SymbolEqualityComparer ConsiderEverything = new SymbolEqualityComparer(TypeCompareKind.ConsiderEverything); - // Internal only comparisons: - internal readonly static SymbolEqualityComparer IgnoreEverything = new SymbolEqualityComparer(TypeCompareKind.AllIgnoreOptions); - internal TypeCompareKind CompareKind { get; } internal SymbolEqualityComparer(TypeCompareKind compareKind)