diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs index 3a057f433df4ab97d943a8db9ebd7fdb1cb4b43d..28d1df037228203ed921cc49be34bcd5d2e806d9 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs @@ -1231,7 +1231,7 @@ private bool FieldsAllSet(int containingSlot, LocalState state) Debug.Assert(containingSlot != -1); Debug.Assert(!state.IsAssigned(containingSlot)); VariableIdentifier variable = variableBySlot[containingSlot]; - NamedTypeSymbol structType = (NamedTypeSymbol)VariableType(variable.Symbol).TypeSymbol; + TypeSymbol structType = VariableType(variable.Symbol).TypeSymbol; foreach (var field in _emptyStructTypeCache.GetStructInstanceFields(structType)) { if (_emptyStructTypeCache.IsEmptyStructType(field.Type.TypeSymbol)) continue; diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/EmptyStructTypeCache.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/EmptyStructTypeCache.cs index a5677c4bbe0c626cfd599db71d061348cb4bd76d..b51b1887b64a3f42bc9c555c179a2a4bbfec5ed9 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/EmptyStructTypeCache.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/EmptyStructTypeCache.cs @@ -114,8 +114,11 @@ private bool CheckStructInstanceFields(ConsList typesWithMember // unless necessary. foreach (var member in type.OriginalDefinition.GetMembersUnordered()) { - var field = GetActualInstanceField(member, type); - + if (member.IsStatic) + { + continue; + } + var field = GetActualField(member, type); if ((object)field != null) { var actualFieldType = field.Type.TypeSymbol; @@ -141,17 +144,20 @@ public IEnumerable GetStructInstanceFields(TypeSymbol type) return SpecializedCollections.EmptyEnumerable(); } - return GetStructInstanceFields(nts); + return GetStructFields(nts, includeStatic: false); } - public IEnumerable GetStructInstanceFields(NamedTypeSymbol type) + public IEnumerable GetStructFields(NamedTypeSymbol type, bool includeStatic) { // PERF: we get members of the OriginalDefinition to not create substituted members/types // unless necessary. foreach (var member in type.OriginalDefinition.GetMembersUnordered()) { - var field = GetActualInstanceField(member, type); - + if (!includeStatic && member.IsStatic) + { + continue; + } + var field = GetActualField(member, type); if ((object)field != null) { yield return field; @@ -159,29 +165,27 @@ public IEnumerable GetStructInstanceFields(NamedTypeSymbol type) } } - private FieldSymbol GetActualInstanceField(Symbol member, NamedTypeSymbol type) + private FieldSymbol GetActualField(Symbol member, NamedTypeSymbol type) { - if (!member.IsStatic) + switch (member.Kind) { - switch (member.Kind) - { - case SymbolKind.Field: - var field = (FieldSymbol)member; - // Do not report virtual tuple fields. - // They are additional aliases to the fields of the underlying struct or nested extensions. - // and as such are already accounted for via the nonvirtual fields. - if (field.IsVirtualTupleField) - { - return null; - } + case SymbolKind.Field: + var field = (FieldSymbol)member; + // Do not report virtual tuple fields. + // They are additional aliases to the fields of the underlying struct or nested extensions. + // and as such are already accounted for via the nonvirtual fields. + if (field.IsVirtualTupleField) + { + return null; + } - return (field.IsFixedSizeBuffer || ShouldIgnoreStructField(field, field.Type.TypeSymbol)) ? null : field.AsMember(type); + return (field.IsFixedSizeBuffer || ShouldIgnoreStructField(field, field.Type.TypeSymbol)) ? null : field.AsMember(type); - case SymbolKind.Event: - EventSymbol eventSymbol = (EventSymbol)member; - return (!eventSymbol.HasAssociatedField || ShouldIgnoreStructField(eventSymbol, eventSymbol.Type.TypeSymbol)) ? null : eventSymbol.AssociatedField.AsMember(type); - } + case SymbolKind.Event: + var eventSymbol = (EventSymbol)member; + return (!eventSymbol.HasAssociatedField || ShouldIgnoreStructField(eventSymbol, eventSymbol.Type.TypeSymbol)) ? null : eventSymbol.AssociatedField.AsMember(type); } + return null; } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs index 08b57f5ac0cb44a433a234608750a5d8c6f013ed..9c24a9470a1408838cf94452f660cb1febb3bfa2 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs @@ -91,7 +91,7 @@ protected int VariableSlot(Symbol symbol, int containingSlot = 0) /// /// 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) + protected virtual int GetOrCreateSlot(Symbol symbol, int containingSlot = 0) { if (symbol.Kind == SymbolKind.RangeVariable) return -1; @@ -217,8 +217,8 @@ protected virtual int MakeSlot(BoundExpression node) case BoundKind.PropertyAccess: if (TryGetReceiverAndMember(node, out BoundExpression receiver, out Symbol member)) { - int containingSlot = MakeSlot(receiver); - return (containingSlot == -1) ? -1 : GetOrCreateSlot(member, containingSlot); + Debug.Assert((receiver is null) == member.IsStatic); + return MakeMemberSlot(receiver, member); } break; case BoundKind.AssignmentOperator: @@ -227,6 +227,24 @@ protected virtual int MakeSlot(BoundExpression node) return -1; } + protected int MakeMemberSlot(BoundExpression receiverOpt, Symbol member) + { + int containingSlot = -1; + if (!member.IsStatic) + { + if (receiverOpt is null) + { + return -1; + } + containingSlot = MakeSlot(receiverOpt); + if (containingSlot < 0) + { + return -1; + } + } + return GetOrCreateSlot(member, containingSlot); + } + protected static TypeSymbolWithAnnotations VariableType(Symbol s) { switch (s.Kind) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index a9c1fce3ad0eef877969c79b7608c87d889968b1..93c88b398389e2fa79a63a3b6683c21b043d357c 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -328,11 +328,15 @@ protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundE { var fieldAccess = (BoundFieldAccess)expr; var fieldSymbol = fieldAccess.FieldSymbol; - if (fieldSymbol.IsStatic || fieldSymbol.IsFixedSizeBuffer) + member = fieldSymbol; + if (fieldSymbol.IsFixedSizeBuffer) { return false; } - member = fieldSymbol; + if (fieldSymbol.IsStatic) + { + return true; + } receiver = fieldAccess.ReceiverOpt; break; } @@ -340,12 +344,12 @@ protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundE { var eventAccess = (BoundEventAccess)expr; var eventSymbol = eventAccess.EventSymbol; + // https://github.com/dotnet/roslyn/issues/29901 Use AssociatedField for field-like events? + member = eventSymbol; if (eventSymbol.IsStatic) { - return false; + return true; } - // https://github.com/dotnet/roslyn/issues/29901 Use AssociatedField for field-like events? - member = eventSymbol; receiver = eventAccess.ReceiverOpt; break; } @@ -353,16 +357,22 @@ protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundE { var propAccess = (BoundPropertyAccess)expr; var propSymbol = propAccess.PropertySymbol; - if (propSymbol.IsStatic) + member = GetBackingFieldIfStructProperty(propSymbol); + if (member is null) { return false; } - member = GetBackingFieldIfStructProperty(propSymbol); + if (propSymbol.IsStatic) + { + return true; + } receiver = propAccess.ReceiverOpt; break; } } + Debug.Assert(member?.IsStatic != true); + return (object)member != null && (object)receiver != null && receiver.Kind != BoundKind.TypeExpression && @@ -383,7 +393,7 @@ private Symbol GetBackingFieldIfStructProperty(Symbol symbol) // https://github.com/dotnet/roslyn/issues/29619 Relying on field name // will not work for properties declared in other languages. var fieldName = GeneratedNames.MakeBackingFieldName(property.Name); - return _emptyStructTypeCache.GetStructInstanceFields(containingType).FirstOrDefault(f => f.Name == fieldName); + return _emptyStructTypeCache.GetStructFields(containingType, includeStatic: symbol.IsStatic).FirstOrDefault(f => f.Name == fieldName); } } return symbol; @@ -391,7 +401,7 @@ private Symbol GetBackingFieldIfStructProperty(Symbol symbol) // https://github.com/dotnet/roslyn/issues/29619 Temporary, until we're using // properties on structs directly. - private new int GetOrCreateSlot(Symbol symbol, int containingSlot = 0) + protected override int GetOrCreateSlot(Symbol symbol, int containingSlot = 0) { symbol = GetBackingFieldIfStructProperty(symbol); if ((object)symbol == null) @@ -4717,8 +4727,7 @@ private void VisitMemberAccess(BoundExpression node, BoundExpression receiverOpt // accumulate so far. if (!resultType.IsValueType || resultType.IsNullableType()) { - int containingSlot = getReceiverSlot(); - int slot = (containingSlot < 0) ? -1 : GetOrCreateSlot(member, containingSlot); + int slot = MakeMemberSlot(receiverOpt, member); if (slot > 0 && slot < this.State.Capacity) { var annotation = this.State[slot]; @@ -4732,7 +4741,7 @@ private void VisitMemberAccess(BoundExpression node, BoundExpression receiverOpt Debug.Assert(!IsConditionalState); if (nullableOfTMember == SpecialMember.System_Nullable_T_get_HasValue) { - int containingSlot = getReceiverSlot(); + int containingSlot = (receiverOpt is null) ? -1 : MakeSlot(receiverOpt); if (containingSlot > 0) { // https://github.com/dotnet/roslyn/issues/31516: Report HDN_NullCheckIsProbablyAlwaysTrue/False @@ -4745,8 +4754,6 @@ private void VisitMemberAccess(BoundExpression node, BoundExpression receiverOpt _resultType = resultType; } - - int getReceiverSlot() => (receiverOpt is null) ? -1 : MakeSlot(receiverOpt); } private static SpecialMember? GetNullableOfTMember(CSharpCompilation compilation, Symbol member) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 5a451a948864a86bfe246431858c35815bf76352..14191ac4d61ab53105d734c200f587ea3b2c0f80 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -78937,5 +78937,331 @@ static void F(C? c) // c.F = c.F; // 1 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c").WithLocation(10, 9)); } + + [Fact] + [WorkItem(26651, "https://github.com/dotnet/roslyn/issues/26651")] + public void StaticMember_01() + { + var source = +@"class Program +{ + static readonly string? F = null; + static readonly int? G = null; + static void Main() + { + if (F != null) + F.ToString(); + if (G != null) + _ = G.Value; + } +}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics(); + } + + [Fact] + [WorkItem(26651, "https://github.com/dotnet/roslyn/issues/26651")] + public void StaticMember_02() + { + var source = +@"#pragma warning disable 0649 +class C +{ + static T F; + static void M() + { + if (F == null) return; + F.ToString(); + } +}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics(); + } + + [Fact] + [WorkItem(26651, "https://github.com/dotnet/roslyn/issues/26651")] + public void StaticMember_03() + { + var source = +@"#pragma warning disable 0649 +class C +{ + internal static T F; +} +class Program +{ + static void F1() + { + C.F.ToString(); // 1 + C.F.ToString(); + } + static void F2() where T2 : class + { + C.F.ToString(); // 2 + C.F.ToString(); + } + static void F3() where T3 : class + { + C.F.ToString(); + C.F.ToString(); + } + static void F4() where T4 : struct + { + _ = C.F.Value; // 3 + _ = C.F.Value; + } +}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (10,9): warning CS8602: Possible dereference of a null reference. + // C.F.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.F").WithLocation(10, 9), + // (15,9): warning CS8602: Possible dereference of a null reference. + // C.F.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.F").WithLocation(15, 9), + // (25,13): warning CS8629: Nullable value type may be null. + // _ = C.F.Value; // 3 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "C.F.Value").WithLocation(25, 13)); + } + + [Fact] + [WorkItem(26651, "https://github.com/dotnet/roslyn/issues/26651")] + public void StaticMember_04() + { + var source = +@"#pragma warning disable 0649 +class C +{ + internal static T F; +} +class Program +{ + static void F1() + { + C.F = default; // 1 + C.F.ToString(); // 2 + } + static void F2() where T2 : class, new() + { + C.F = new T2(); + C.F.ToString(); + } + static void F3() where T3 : class, new() + { + C.F = new T3(); + C.F.ToString(); + } + static void F4() where T4 : class + { + C.F = null; // 3 + C.F.ToString(); // 4 + } + static void F5() where T5 : struct + { + C.F = default(T5); + _ = C.F.Value; + } + static void F6() where T6 : new() + { + C.F = new T6(); + C.F.ToString(); + } + static void F7() + { + C.F = null; // 5 + _ = C.F.Length; // 6 + } + static void F8() + { + C.F = 3; + _ = C.F.Value; + } +}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (10,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // C.F = default; // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(10, 19), + // (11,9): warning CS8602: Possible dereference of a null reference. + // C.F.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.F").WithLocation(11, 9), + // (25,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // C.F = null; // 3 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(25, 19), + // (26,9): warning CS8602: Possible dereference of a null reference. + // C.F.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.F").WithLocation(26, 9), + // (40,23): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // C.F = null; // 5 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(40, 23), + // (41,13): warning CS8602: Possible dereference of a null reference. + // _ = C.F.Length; // 6 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.F").WithLocation(41, 13)); + } + + [Fact] + [WorkItem(26651, "https://github.com/dotnet/roslyn/issues/26651")] + public void StaticMember_05() + { + var source = +@"class C +{ + internal static T P + { + get => throw null; + set { } + } +} +class Program +{ + static void F1() + { + C.P = default; // 1 + C.P.ToString(); // 2 + } + static void F2() where T2 : class, new() + { + C.P = new T2(); + C.P.ToString(); + } + static void F3() where T3 : class, new() + { + C.P = new T3(); + C.P.ToString(); + } + static void F4() where T4 : class + { + C.P = null; // 3 + C.P.ToString(); // 4 + } + static void F5() where T5 : struct + { + C.P = default(T5); + _ = C.P.Value; + } + static void F6() where T6 : new() + { + C.P = new T6(); + C.P.ToString(); + } + static void F7() + { + C.P = null; // 5 + _ = C.P.Length; // 6 + } + static void F8() + { + C.P = 3; + _ = C.P.Value; + } +}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (13,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // C.P = default; // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(13, 19), + // (14,9): warning CS8602: Possible dereference of a null reference. + // C.P.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.P").WithLocation(14, 9), + // (28,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // C.P = null; // 3 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(28, 19), + // (29,9): warning CS8602: Possible dereference of a null reference. + // C.P.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.P").WithLocation(29, 9), + // (43,23): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // C.P = null; // 5 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(43, 23), + // (44,13): warning CS8602: Possible dereference of a null reference. + // _ = C.P.Length; // 6 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.P").WithLocation(44, 13)); + } + + [Fact] + [WorkItem(26651, "https://github.com/dotnet/roslyn/issues/26651")] + public void StaticMember_06() + { + var source = +@"#pragma warning disable 0649 +struct S +{ + internal static T F; +} +class Program +{ + static void F1() + { + S.F = default; // 1 + S.F.ToString(); // 2 + } + static void F2() where T2 : class, new() + { + S.F = new T2(); + S.F.ToString(); + } + static void F3() where T3 : class + { + S.F = null; // 3 + S.F.ToString(); // 4 + } +}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (10,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // S.F = default; // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(10, 19), + // (11,9): warning CS8602: Possible dereference of a null reference. + // S.F.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "S.F").WithLocation(11, 9), + // (20,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // S.F = null; // 3 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(20, 19), + // (21,9): warning CS8602: Possible dereference of a null reference. + // S.F.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "S.F").WithLocation(21, 9)); + } + + [Fact] + [WorkItem(26651, "https://github.com/dotnet/roslyn/issues/26651")] + public void StaticMember_07() + { + var source = +@"struct S +{ + internal static T P { get; set; } +} +class Program +{ + static void F1() + { + S.P = default; // 1 + S.P.ToString(); // 2 + } + static void F2() where T2 : class, new() + { + S.P = new T2(); + S.P.ToString(); + } + static void F3() where T3 : class + { + S.P = null; // 3 + S.P.ToString(); // 4 + } +}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (9,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // S.P = default; // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "default").WithLocation(9, 19), + // (10,9): warning CS8602: Possible dereference of a null reference. + // S.P.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "S.P").WithLocation(10, 9), + // (19,19): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // S.P = null; // 3 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(19, 19), + // (20,9): warning CS8602: Possible dereference of a null reference. + // S.P.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "S.P").WithLocation(20, 9)); + } } }