From f975e80a8803a00cf445bffe0e84883d82dfc680 Mon Sep 17 00:00:00 2001 From: Alireza Habibi Date: Fri, 29 Oct 2021 03:01:09 +0330 Subject: [PATCH] Slice value is always assumed to be not null --- .../Portable/Binder/DecisionDagBuilder.cs | 6 +- .../Semantics/PatternMatchingTestBase.cs | 5 +- .../PatternMatchingTests_ListPatterns.cs | 188 +++++++++++------- 3 files changed, 122 insertions(+), 77 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs index 90906b9dd7634..d8ae952eb2706 100644 --- a/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs +++ b/src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs @@ -421,8 +421,12 @@ private static void MakeCheckNotNull( ArrayBuilder tests) { // Add a null test if needed - if (input.Type.CanContainNull()) + if (input.Type.CanContainNull() && + // The slice value is assumed to be never null + input.Source is not BoundDagSliceEvaluation) + { tests.Add(new Tests.One(new BoundDagNonNullTest(syntax, isExplicitTest, input))); + } } /// diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs index 1123370d1d3ed..4f6662aeac125 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTestBase.cs @@ -441,12 +441,13 @@ protected static void AssertEmpty(SymbolInfo info) Assert.Equal(CandidateReason.None, info.CandidateReason); } - protected static void VerifyDecisionDagDump(Compilation comp, string expectedDecisionDag) + [Conditional("DEBUG")] + protected static void VerifyDecisionDagDump(Compilation comp, string expectedDecisionDag, int index = 0) where T : CSharpSyntaxNode { #if DEBUG var tree = comp.SyntaxTrees.First(); - var node = tree.GetRoot().DescendantNodes().OfType().First(); + var node = tree.GetRoot().DescendantNodes().OfType().ElementAt(index); var model = (CSharpSemanticModel)comp.GetSemanticModel(tree); var binder = model.GetEnclosingBinder(node.SpanStart); var decisionDag = node switch diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs index 366eda650bad3..972a10fa2bf31 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs @@ -3436,9 +3436,6 @@ public void M() // Note: we don't try to explain nested slice patterns right now so all these just produce a fallback example var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range }); compilation.VerifyEmitDiagnostics( - // (20,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. - // _ = this switch // we didn't test for [.. null] but we're looking for an example with Length=1. // 1 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(20, 18), // (27,18): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '_' is not covered. // _ = this switch // didn't test for [.. [not null]] // // 2 Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(27, 18), @@ -3451,9 +3448,6 @@ public void M() // (46,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. // _ = this switch // didn't test for [_, .. null, _, _, _] // we're trying to construct an example with Length=4, the slice may not be null // 5 Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(46, 18), - // (52,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. - // _ = this switch // we should consider this switch exhaustive // 6 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(52, 18), // (58,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. // _ = this switch // didn't test for [_, .. [_, null], _] // 7 Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(58, 18), @@ -3462,10 +3456,7 @@ public void M() Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(64, 18), // (70,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. // _ = this switch // didn't test for [_, .. [_, null, _], _] // 9 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(70, 18), - // (76,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. - // _ = this switch // didn't test for [.. null, _] // we're trying to construct an example with Length=1 but a null slice // 10 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(76, 18) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(70, 18) ); } @@ -4840,46 +4831,72 @@ void Test(int[] a) { Length: not 1 } => 0, [<0, ..] => 0, [..[>= 0]] or [..null] => 1, - [_] => 2, // unreachable + [_] => 2, // unreachable 1 }; - _ = a switch // exhaustive + _ = a switch { { Length: not 1 } => 0, [<0, ..] => 0, [..[>= 0]] => 1, - [_] => 2, + [_] => 2, // unreachable 2 }; } }" + TestSources.GetSubArray; var comp = CreateCompilationWithIndexAndRange(src); comp.VerifyEmitDiagnostics( - // (11,13): error CS8510: The pattern is unreachable. It has already been handled by a previous arm of the switch expression or it is impossible to match. - // [_] => 2, // unreachable - Diagnostic(ErrorCode.ERR_SwitchArmSubsumed, "[_]").WithLocation(11, 13)); - - VerifyDecisionDagDump(comp, + // (11,13): error CS8510: The pattern is unreachable. It has already been handled by a previous arm of the switch expression or it is impossible to match. + // [_] => 2, // unreachable + Diagnostic(ErrorCode.ERR_SwitchArmSubsumed, "[_]").WithLocation(11, 13), + // (18,13): error CS8510: The pattern is unreachable. It has already been handled by a previous arm of the switch expression or it is impossible to match. + // [_] => 2, + Diagnostic(ErrorCode.ERR_SwitchArmSubsumed, "[_]").WithLocation(18, 13) + ); -@"[0]: t0 != null ? [1] : [13] + AssertEx.Multiple( + () => VerifyDecisionDagDump(comp, +@"[0]: t0 != null ? [1] : [12] [1]: t1 = t0.Length; [2] -[2]: t1 == 1 ? [3] : [12] +[2]: t1 == 1 ? [3] : [11] [3]: t2 = t0[0]; [4] [4]: t2 < 0 ? [5] : [6] [5]: leaf `[<0, ..] => 0` [6]: t3 = DagSliceEvaluation(t0); [7] -[7]: t3 != null ? [8] : [11] -[8]: t4 = t3.Length; [9] -[9]: t5 = t3[0]; [10] -[10]: t5 <-- t2; [11] -[11]: leaf `[..[>= 0]] or [..null] => 1` -[12]: leaf `{ Length: not 1 } => 0` -[13]: leaf `a switch +[7]: t4 = t3.Length; [8] +[8]: t5 = t3[0]; [9] +[9]: t5 <-- t2; [10] +[10]: leaf `[..[>= 0]] or [..null] => 1` +[11]: leaf `{ Length: not 1 } => 0` +[12]: leaf `a switch { { Length: not 1 } => 0, [<0, ..] => 0, [..[>= 0]] or [..null] => 1, - [_] => 2, // unreachable + [_] => 2, // unreachable 1 }` -"); +", index: 0), + + () => VerifyDecisionDagDump(comp, +@"[0]: t0 != null ? [1] : [12] +[1]: t1 = t0.Length; [2] +[2]: t1 == 1 ? [3] : [11] +[3]: t2 = t0[0]; [4] +[4]: t2 < 0 ? [5] : [6] +[5]: leaf `[<0, ..] => 0` +[6]: t3 = DagSliceEvaluation(t0); [7] +[7]: t4 = t3.Length; [8] +[8]: t5 = t3[0]; [9] +[9]: t5 <-- t2; [10] +[10]: leaf `[..[>= 0]] => 1` +[11]: leaf `{ Length: not 1 } => 0` +[12]: leaf `a switch + { + { Length: not 1 } => 0, + [<0, ..] => 0, + [..[>= 0]] => 1, + [_] => 2, // unreachable 2 + }` +", index: 1) + ); } [Fact] @@ -5268,52 +5285,39 @@ class Derived : Base, I2 [Theory] [CombinatorialData] public void Subsumption_Slice_00( - [CombinatorialValues( - "[1,2,3]", - "[1,2,3,..[]]", - "[1,2,..[],3]", - "[1,..[],2,3]", - "[..[],1,2,3]", - "[1,..[2,3]]", - "[..[1,2],3]", - "[..[1,2,3]]", - "[..[..[1,2,3]]]", - "[..[1,2,3,..[]]]", - "[..[1,2,..[],3]]", - "[..[1,..[],2,3]]", - "[..[..[],1,2,3]]", - "[..[1,..[2,3]]]", - "[..[..[1,2],3]]", - "[1, ..[2], 3]", - "[1, ..[2, ..[3]]]", - "[1, ..[2, ..[], 3]]")] - string case1, - [CombinatorialValues( - "[1,2,3]", - "[1,2,3,..[]]", - "[1,2,..[],3]", - "[1,..[],2,3]", - "[..[],1,2,3]", - "[1,..[2,3]]", - "[..[1,2],3]", - "[..[1,2,3]]", - "[..[..[1,2,3]]]", - "[..[1,2,3,..[]]]", - "[..[1,2,..[],3]]", - "[..[1,..[],2,3]]", - "[..[..[],1,2,3]]", - "[..[1,..[2,3]]]", - "[..[..[1,2],3]]", - "[1, ..[2], 3]", - "[1, ..[2, ..[3]]]", - "[1, ..[2, ..[], 3]]")] - string case2) + [CombinatorialRange(0, 18)] int c1, + [CombinatorialRange(0, 18)] int c2, + [CombinatorialValues("System.Span", "int[]")] string type) { + var cases = new string[18] + { + "[1,2,3]", + "[1,2,3,..[]]", + "[1,2,..[],3]", + "[1,..[],2,3]", + "[..[],1,2,3]", + "[1,..[2,3]]", + "[..[1,2],3]", + "[..[1,2,3]]", + "[..[..[1,2,3]]]", + "[..[1,2,3,..[]]]", + "[..[1,2,..[],3]]", + "[..[1,..[],2,3]]", + "[..[..[],1,2,3]]", + "[..[1,..[2,3]]]", + "[..[..[1,2],3]]", + "[1, ..[2], 3]", + "[1, ..[2, ..[3]]]", + "[1, ..[2, ..[], 3]]" + }; + + var case1 = cases[c1]; + var case2 = cases[c2]; + var src = @" -using System; class C { - void Test(Span a) + void Test(" + type + @" a) { switch (a) { @@ -5323,10 +5327,10 @@ void Test(Span a) } } }"; - var comp = CreateCompilationWithIndexAndRangeAndSpan(src, parseOptions: TestOptions.RegularWithListPatterns); + var comp = CreateCompilationWithIndexAndRangeAndSpan(new[] { src, TestSources.GetSubArray }, parseOptions: TestOptions.RegularWithListPatterns); comp.VerifyEmitDiagnostics( - // (10,18): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. - Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, case2).WithLocation(10, 18) + // (9,18): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. + Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, case2).WithLocation(9, 18) ); } @@ -5352,6 +5356,42 @@ public static void Test(System.Span a) Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, "[..[var v]]").WithLocation(9, 18)); } + [Fact] + public void Subsumption_Slice_02() + { + var source = @" +using System; + +IOuter outer = null; +switch (outer) +{ + case [..[..[10],20]]: + break; + case [..[10],20]: // 1 + break; +} + +interface IOuter +{ + int Length { get; } + IInner Slice(int a, int b); + object this[int i] { get; } +} +interface IInner +{ + int Count { get; } + IOuter this[Range r] { get; } + object this[Index i] { get; } +} +"; + var comp = CreateCompilationWithIndexAndRangeAndSpan(new[] { source, TestSources.GetSubArray }, options: TestOptions.DebugExe); + comp.VerifyEmitDiagnostics( + // (9,10): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. + // case [..[10],20]: // 1 + Diagnostic(ErrorCode.ERR_SwitchCaseSubsumed, "[..[10],20]").WithLocation(9, 10) + ); + } + [Fact] public void Exhaustiveness_01() {