Handle dependent slots in pattern-matching null tests.

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

@@ -44,7 +44,7 @@ internal abstract partial class AbstractFlowPass<TLocalState, TLocalFunctionStat
         /// May be a top-level member or a lambda or local function. It is used for
         /// references to method parameters. Thus, '_symbol' should not be used directly, but
         /// 'MethodParameters', 'MethodThisParameter' and 'AnalyzeOutParameters(...)' should be used
-        /// instead.
+        /// instead. _symbol is null during speculative binding.
         /// </summary>
         protected readonly Symbol _symbol;
 

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -2702,19 +2702,87 @@ private int LearnFromNullTest(BoundExpression expression, ref LocalState state)
             // We should not blindly strip conversions here. Tracked by https://github.com/dotnet/roslyn/issues/36164
             var expressionWithoutConversion = RemoveConversion(expression, includeExplicitConversions: true).expression;
             var slot = MakeSlot(expressionWithoutConversion);
-            return LearnFromNullTest(slot, expressionWithoutConversion.Type, ref state);
+
+            // Since we know for sure the slot is null (we just tested it), we know that dependent slots are not
+            // reachable and therefore can be treated as not null.  However, we have not computed the proper
+            // (inferred) type for the expression, so we cannot compute the correct symbols for the member slots here
+            // (using the incorrect symbols would result in computing an incorrect default state for them).
+            // Therefore we do not mark dependent slots not null.  See https://github.com/dotnet/roslyn/issues/39624
+            return LearnFromNullTest(slot, expressionWithoutConversion.Type, ref state, markDependentSlotsNotNull: false);
         }
 
-        private int LearnFromNullTest(int slot, TypeSymbol expressionType, ref LocalState state)
+        private int LearnFromNullTest(int slot, TypeSymbol expressionType, ref LocalState state, bool markDependentSlotsNotNull)
         {
             if (slot > 0 && PossiblyNullableType(expressionType))
             {
                 state[slot] = NullableFlowState.MaybeNull;
+                if (markDependentSlotsNotNull)
+                {
+                    MarkDependentSlotsNotNull(slot, expressionType, ref state);
+                }
             }
 
             return slot;
         }
 
+        // If we know for sure that a slot contains a null value, then we know for sure that dependent slots
+        // are "unreachable" so we might as well treat them as not null.  That way when this state is merged
+        // with another state, those dependent states won't pollute values from the other state.
+        private void MarkDependentSlotsNotNull(int slot, TypeSymbol expressionType, ref LocalState state, int depth = 2)
+        {
+            if (depth <= 0)
+                return;
+
+            foreach (var member in getMembers(expressionType))
+            {
+                HashSet<DiagnosticInfo> discardedUseSiteDiagnostics = null;
+                NamedTypeSymbol containingType = this._symbol?.ContainingType;
+                if ((member is PropertySymbol { IsIndexedProperty: false } || member.Kind == SymbolKind.Field) &&
+                    member.RequiresInstanceReceiver() &&
+                    (containingType is null || AccessCheck.IsSymbolAccessible(member, containingType, ref discardedUseSiteDiagnostics)))
+                {
+                    int childSlot = GetOrCreateSlot(member, slot, true);
+                    if (childSlot > 0)
+                    {
+                        state[childSlot] = NullableFlowState.NotNull;
+                        MarkDependentSlotsNotNull(childSlot, member.GetTypeOrReturnType().Type, ref state, depth - 1);
+                    }
+                }
+            }
+
+            static IEnumerable<Symbol> getMembers(TypeSymbol type)
+            {
+                // First, return the direct members
+                foreach (var member in type.GetMembers())
+                    yield return member;
+
+                // All types inherit members from their effective bases
+                for (NamedTypeSymbol baseType = effectiveBase(type); !(baseType is null); baseType = baseType.BaseTypeNoUseSiteDiagnostics)
+                    foreach (var member in baseType.GetMembers())
+                        yield return member;
+
+                // Interfaces and type parameters inherit from their effective interfaces
+                foreach (NamedTypeSymbol interfaceType in inheritedInterfaces(type))
+                    foreach (var member in interfaceType.GetMembers())
+                        yield return member;
+
+                yield break;
+
+                static NamedTypeSymbol effectiveBase(TypeSymbol type) => type switch
+                {
+                    TypeParameterSymbol tp => tp.EffectiveBaseClassNoUseSiteDiagnostics,
+                    var t => t.BaseTypeNoUseSiteDiagnostics,
+                };
+
+                static ImmutableArray<NamedTypeSymbol> inheritedInterfaces(TypeSymbol type) => type switch
+                {
+                    TypeParameterSymbol tp => tp.AllEffectiveInterfacesNoUseSiteDiagnostics,
+                    { TypeKind: TypeKind.Interface } => type.AllInterfacesNoUseSiteDiagnostics,
+                    _ => ImmutableArray<NamedTypeSymbol>.Empty,
+                };
+            }
+        }
+
         private static BoundExpression SkipReferenceConversions(BoundExpression possiblyConversion)
         {
             while (possiblyConversion.Kind == BoundKind.Conversion)

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs

@@ -98,7 +98,9 @@ private void LearnFromAnyNullPatterns(
                     bool isExplicitNullCheck = cp.Value.ConstantValue == ConstantValue.Null;
                     if (isExplicitNullCheck)
                     {
-                        LearnFromNullTest(inputSlot, inputType, ref this.State);
+                        // Since we're not branching on this null test here, we just infer the top level
+                        // nullability.  We'll branch on it later.
+                        LearnFromNullTest(inputSlot, inputType, ref this.State, markDependentSlotsNotNull: false);
                     }
                     break;
                 case BoundDeclarationPattern _:
@@ -349,6 +351,7 @@ protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastS
                                 case BoundDagNonNullTest t:
                                     if (inputSlot > 0)
                                     {
+                                        MarkDependentSlotsNotNull(inputSlot, inputType, ref this.StateWhenFalse);
                                         learnFromNonNullTest(inputSlot, ref this.StateWhenTrue);
                                     }
                                     gotoNode(p.WhenTrue, this.StateWhenTrue, nodeBelievedReachable);
@@ -357,7 +360,7 @@ protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastS
                                 case BoundDagExplicitNullTest t:
                                     if (inputSlot > 0)
                                     {
-                                        LearnFromNullTest(inputSlot, inputType, ref this.StateWhenTrue);
+                                        LearnFromNullTest(inputSlot, inputType, ref this.StateWhenTrue, markDependentSlotsNotNull: true);
                                         learnFromNonNullTest(inputSlot, ref this.StateWhenFalse);
                                     }
                                     gotoNode(p.WhenTrue, this.StateWhenTrue, nodeBelievedReachable);

src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesVsPatterns.cs

@@ -1933,5 +1933,127 @@ public interface IOut<out T> { }
                 //         _ = i switch { 1 => default, _ => x }/*T:T*/; // 6
                 Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "default").WithLocation(46, 29));
         }
+
+        [Fact]
+        [WorkItem(39264, "https://github.com/dotnet/roslyn/issues/39264")]
+        public void IsPatternSplitState_01()
+        {
+            CSharpCompilation c = CreateNullableCompilation(@"
+#nullable enable
+class C
+{
+    string? field = string.Empty;
+    string? otherField = string.Empty;
+
+    void M1(C c)
+    {
+        if (c.field == null) return;
+
+        c.field.ToString();
+    }
+
+    void M2(C c)
+    {
+        if (c is { field: null }) return;
+
+        c.field.ToString();
+    }
+
+    void M3(C c)
+    {
+        switch (c)
+        {
+            case { field: null }:
+                break;
+            default:
+                c.field.ToString();
+                break;
+        }
+    }
+
+    void M4(C c)
+    {
+        _ = c switch
+        {
+            { field: null } => string.Empty,
+            _ => c.field.ToString(),
+        };
+    }
+
+    void M5(C c)
+    {
+        if (c is { field: null }) return;
+
+        c.otherField.ToString(); // W
+    }
+}
+");
+            c.VerifyDiagnostics(
+                // (47,9): warning CS8602: Dereference of a possibly null reference.
+                //         c.otherField.ToString(); // W
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.otherField").WithLocation(47, 9)
+                );
+        }
+
+        [Fact]
+        [WorkItem(39264, "https://github.com/dotnet/roslyn/issues/39264")]
+        public void IsPatternSplitState_02()
+        {
+            CSharpCompilation c = CreateNullableCompilation(@"
+#nullable enable
+class C
+{
+    C? c = null;
+
+    void M1(C c)
+    {
+        if (c is { c: null })
+        {
+            if (c.c != null)
+            {
+                c.c.c.c.ToString();
+            }
+        }
+    }
+}
+");
+            c.VerifyDiagnostics(
+                );
+        }
+
+        [Fact]
+        [WorkItem(39264, "https://github.com/dotnet/roslyn/issues/39264")]
+        public void IsPatternSplitState_03()
+        {
+            CSharpCompilation c = CreateNullableCompilation(@"
+#nullable enable
+public class C {
+    C? c = null;
+
+    public static void Main()
+    {
+        C c = new C();
+        M1(c, new C());
+    }
+
+    static void M1(C c, C c2)
+    {
+        if (c is { c : null } && c2 is { c: null })
+        {
+            c.c = c2;
+            if (c.c != null)
+            {
+                c.c.c.ToString(); // warning
+            }
+        }
+    }
+}
+");
+            c.VerifyDiagnostics(
+                // (19,17): warning CS8602: Dereference of a possibly null reference.
+                //                 c.c.c.ToString(); // warning
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.c.c").WithLocation(19, 17)
+                );
+        }
     }
 }