diff --git a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs index 880542b05b017..90906b9dd7634 100644 --- a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs +++ b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs @@ -1110,6 +1110,7 @@ private static (BoundDagTemp? lengthTemp, int offset) TryGetTopLevelLengthTemp(B BoundDagTemp? lengthTemp = null; while (input.Source is BoundDagSliceEvaluation slice) { + Debug.Assert(input.Index == 0); offset += slice.StartIndex - slice.EndIndex; lengthTemp = slice.LengthTemp; input = slice.Input; @@ -1124,6 +1125,7 @@ private static (BoundDagTemp input, BoundDagTemp lengthTemp, int index) GetCanon BoundDagTemp lengthTemp = e.LengthTemp; while (input.Source is BoundDagSliceEvaluation slice) { + Debug.Assert(input.Index == 0); index = index < 0 ? index - slice.EndIndex : index + slice.StartIndex; lengthTemp = slice.LengthTemp; input = slice.Input; @@ -1184,17 +1186,6 @@ private void CheckConsistentDecision( trueTestImpliesTrueOther = false; falseTestImpliesTrueOther = false; - // For null tests or type tests we just need to make sure we are looking at the same instance, - // for other tests the projected type should also match - if (test is not (BoundDagNonNullTest or BoundDagExplicitNullTest) && - other is not (BoundDagNonNullTest or BoundDagExplicitNullTest) && - (test is not BoundDagTypeTest || other is not BoundDagTypeTest) && - !test.Input.Type.Equals(other.Input.Type, TypeCompareKind.AllIgnoreOptions)) - { - Debug.Assert(!test.Input.Equals(other.Input)); - return; - } - switch (test) { case BoundDagNonNullTest _: @@ -1363,68 +1354,85 @@ private static bool CheckInputRelation( return true; } + // For null tests or type tests we just need to make sure we are looking at the same instance, + // for other tests the projected type should also match + if (test is not (BoundDagNonNullTest or BoundDagExplicitNullTest) && + other is not (BoundDagNonNullTest or BoundDagExplicitNullTest) && + (test is not BoundDagTypeTest || other is not BoundDagTypeTest) && + !test.Input.Type.Equals(other.Input.Type, TypeCompareKind.AllIgnoreOptions)) + { + return false; + } + var conditions = ArrayBuilder.GetInstance(); // Loop through the input chain for both tests at the same time and check if there's // any pair of indexers in the path that could relate depending on the length value. do { - switch (s1Input.Source, s2Input.Source) + if (s1Input.IsSameValue(s2Input)) { - // Even though the two tests appear unrelated (with different inputs), - // it is possible that they are in fact related under certain conditions. - // For instance, the inputs [0] and [^1] point to the same element when length is 1. - case (BoundDagIndexerEvaluation s1, BoundDagIndexerEvaluation s2): - // Take the top-level input and normalize indices to account for indexer accesses inside a slice. - // For instance [0] in nested list pattern [ 0, ..[$$], 2 ] refers to [1] in the containing list. - (s1Input, BoundDagTemp s1LengthTemp, int s1Index) = GetCanonicalInput(s1); - (s2Input, BoundDagTemp s2LengthTemp, int s2Index) = GetCanonicalInput(s2); - Debug.Assert(s1LengthTemp.Syntax is ListPatternSyntax); - Debug.Assert(s2LengthTemp.Syntax is ListPatternSyntax); - // Ignore input source as it will be matched in the subsequent iterations. - if (s1Input.IsEquivalentTo(s2Input) && - // We don't want to pair two indices within the same pattern. - s1LengthTemp.Syntax != s2LengthTemp.Syntax) - { - Debug.Assert(s1LengthTemp.IsEquivalentTo(s2LengthTemp)); - Debug.Assert(s1LengthTemp.Equals(s2LengthTemp) == s1Input.Equals(s2Input)); - if (s1Index == s2Index) - { - continue; - } - - if (s1Index < 0 != s2Index < 0) + switch (s1Input.Source, s2Input.Source) + { + // We should've skipped all type evaluations at this point. + case (BoundDagTypeEvaluation, _): + case (_, BoundDagTypeEvaluation): + throw ExceptionUtilities.Unreachable; + + // Even though the two tests appear unrelated (with different inputs), + // it is possible that they are in fact related under certain conditions. + // For instance, the inputs [0] and [^1] point to the same element when length is 1. + case (BoundDagIndexerEvaluation s1, BoundDagIndexerEvaluation s2): + // Take the top-level input and normalize indices to account for indexer accesses inside a slice. + // For instance [0] in nested list pattern [ 0, ..[$$], 2 ] refers to [1] in the containing list. + (s1Input, BoundDagTemp s1LengthTemp, int s1Index) = GetCanonicalInput(s1); + (s2Input, BoundDagTemp s2LengthTemp, int s2Index) = GetCanonicalInput(s2); + Debug.Assert(s1LengthTemp.Syntax is ListPatternSyntax); + Debug.Assert(s2LengthTemp.Syntax is ListPatternSyntax); + // Ignore input source as it will be matched in the subsequent iterations. + if (s1Input.IsSameValue(s2Input) && + // We don't want to pair two indices within the same pattern. + s1LengthTemp.Syntax != s2LengthTemp.Syntax) { - Debug.Assert(state.RemainingValues.ContainsKey(s1LengthTemp)); - var lengthValues = (IValueSet)state.RemainingValues[s1LengthTemp]; - // We do not expect an empty set here because an indexer evaluation is always preceded by - // a length test of which an impossible match would have made the rest of the tests unreachable. - Debug.Assert(!lengthValues.IsEmpty); - - // Compute the length value that would make these two indices point to the same element. - int lengthValue = s1Index < 0 ? s2Index - s1Index : s1Index - s2Index; - if (lengthValues.All(BinaryOperatorKind.Equal, lengthValue)) + Debug.Assert(s1LengthTemp.IsEquivalentTo(s2LengthTemp)); + if (s1Index == s2Index) { - // If the length is known to be exact, the two are considered to point to the same element. continue; } - if (lengthValues.Any(BinaryOperatorKind.Equal, lengthValue)) + if (s1Index < 0 != s2Index < 0) { - // Otherwise, we add a test to make the result conditional on the length value. - conditions.Add(new Tests.One(new BoundDagValueTest(syntax, ConstantValue.Create(lengthValue), s1LengthTemp))); - continue; + Debug.Assert(state.RemainingValues.ContainsKey(s1LengthTemp)); + var lengthValues = (IValueSet)state.RemainingValues[s1LengthTemp]; + // We do not expect an empty set here because an indexer evaluation is always preceded by + // a length test of which an impossible match would have made the rest of the tests unreachable. + Debug.Assert(!lengthValues.IsEmpty); + + // Compute the length value that would make these two indices point to the same element. + int lengthValue = s1Index < 0 ? s2Index - s1Index : s1Index - s2Index; + if (lengthValues.All(BinaryOperatorKind.Equal, lengthValue)) + { + // If the length is known to be exact, the two are considered to point to the same element. + continue; + } + + if (lengthValues.Any(BinaryOperatorKind.Equal, lengthValue)) + { + // Otherwise, we add a test to make the result conditional on the length value. + conditions.Add(new Tests.One(new BoundDagValueTest(syntax, ConstantValue.Create(lengthValue), s1LengthTemp))); + continue; + } } } - } - break; + break; - // If the sources are equivalent (ignoring their input), it's still possible to find a pair of indexers that could relate. - // For example, the subpatterns in `[.., { E: subpat }] or [{ E: subpat }]` are being applied to the same element in the list. - // To account for this scenario, we walk up all the inputs as long as we see equivalent evaluation nodes in the path. - case (BoundDagEvaluation s1, BoundDagEvaluation s2) when s1.IsEquivalentTo(s2) && s1Input.IsEquivalentTo(s2Input): - s1Input = OriginalInput(s1.Input); - s2Input = OriginalInput(s2.Input); - continue; + // If the sources are equivalent (ignoring their input), it's still possible to find a pair of indexers that could relate. + // For example, the subpatterns in `[.., { E: subpat }] or [{ E: subpat }]` are being applied to the same element in the list. + // To account for this scenario, we walk up all the inputs as long as we see equivalent evaluation nodes in the path. + case (BoundDagEvaluation s1, BoundDagEvaluation s2) when s1.IsEquivalentTo(s2): + s1Input = OriginalInput(s1.Input); + s2Input = OriginalInput(s2.Input); + continue; + } } // tests are unrelated diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs index 40e0907e66e26..013dd12aa614a 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundDagTemp.cs @@ -40,6 +40,16 @@ public bool IsEquivalentTo(BoundDagTemp other) this.Index == other.Index; } + /// + /// Determine if two s represent the same value for the purpose of a pattern evaluation. + /// + public bool IsSameValue(BoundDagTemp other) + { + Debug.Assert(this.Source is not BoundDagTypeEvaluation); + Debug.Assert(other.Source is not BoundDagTypeEvaluation); + return (object)this == other || this.Index == other.Index; + } + public override int GetHashCode() { return Hash.Combine(this.Type.GetHashCode(), Hash.Combine(this.Source?.GetHashCode() ?? 0, this.Index)); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs index 31dfe70cb7baf..366eda650bad3 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs @@ -5172,13 +5172,38 @@ public static void Main() case [I2 and Base]: // 12 break; } + + object obj = null; + switch (obj) + { + case I1 and Base and [.., I1 and Base]: + break; + case Derived and [Derived s]: // 13 + break; + } + + switch (obj) + { + case Base and I1 and [.., Base and I1]: + break; + case Derived and [Derived s]: // OK, we're calling into different indexers + break; + } + + switch (obj) + { + case Base and not null and [.., Base and not null]: + break; + case Derived and [Derived s]: // 14 + break; + } } } -interface I1 {} +interface I1 : System.Collections.IList {} interface I2 {} -class Base : I1 +class Base : System.Collections.ArrayList, I1 { public int F1 = 0; public int F2 = 0; @@ -5230,7 +5255,13 @@ class Derived : Base, I2 Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, "[Base and I2]").WithLocation(103, 18), // (111,18): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. // case [I2 and Base]: // 12 - Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, "[I2 and Base]").WithLocation(111, 18) + Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, "[I2 and Base]").WithLocation(111, 18), + // (120,18): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. + // case Derived and [Derived s]: // 13 + Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, "Derived and [Derived s]").WithLocation(120, 18), + // (136,18): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. + // case Derived and [Derived s]: // 14 + Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, "Derived and [Derived s]").WithLocation(136, 18) ); }