From 4ff038d071cd755d735c18a5434e1e1445de5fdd Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 18 Oct 2021 11:39:30 -0700 Subject: [PATCH 1/5] Explainer for list-patterns --- .../Portable/Binder/PatternExplainer.cs | 90 +++++++++++ .../PatternMatchingTests_ListPatterns.cs | 153 +++++++++++++----- 2 files changed, 203 insertions(+), 40 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs index 5fc1ac2de34fe..7ddb42e7e126f 100644 --- a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs +++ b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs @@ -177,6 +177,7 @@ private static string SamplePatternForTemp( tryHandleTuplePattern(ref unnamedEnumValue) ?? tryHandleNumericLimits(ref unnamedEnumValue) ?? tryHandleRecursivePattern(ref unnamedEnumValue) ?? + tryHandleListPattern(ref unnamedEnumValue) ?? produceFallbackPattern(); static ImmutableArray getArray(Dictionary> map, BoundDagTemp temp) @@ -319,6 +320,95 @@ void addRelation(BinaryOperatorKind relation, ConstantValue value) return null; } + // Handle the special case of a list pattern + string tryHandleListPattern(ref bool unnamedEnumValue) + { + if (constraints.IsEmpty && evaluations.IsEmpty) + return null; + + if (!constraints.All(c => c switch + { + // not-null tests are implicitly incorporated into a recursive pattern + (test: BoundDagNonNullTest _, sense: true) => true, + (test: BoundDagExplicitNullTest _, sense: false) => true, + _ => false, + })) + { + return null; + } + + var indexes = new Dictionary(); + int length = -1; + + foreach (var eval in evaluations) + { + switch (eval) + { + case BoundDagPropertyEvaluation e when e.IsLengthOrCount: + { + Debug.Assert(length == -1); + var subInput = new BoundDagTemp(e.Syntax, e.Property.Type, e); + var lengthValue = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); + if (int.TryParse(lengthValue, out var parsedLength)) + { + Debug.Assert(parsedLength >= 0); + length = parsedLength; + } + } + break; + case BoundDagIndexerEvaluation e: + { + var subInput = new BoundDagTemp(e.Syntax, e.IndexerType, e); + var subPattern = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); + indexes.Add(e.Index, subPattern); + } + break; + case BoundDagSliceEvaluation e: + { + if (e.StartIndex < length + e.EndIndex) + { + var subInput = new BoundDagTemp(e.Syntax, e.SliceType, e); + var subPattern = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); + indexes.Add(e.StartIndex, ".. " + subPattern); + } + } + break; + default: + return null; + } + } + + if (length < 0) + { + Debug.Assert(false); + return null; + } + + var pooledBuilder = PooledStringBuilder.GetInstance(); + var builder = pooledBuilder.Builder; + + builder.Append("["); + for (int i = 0; i < length; i++) + { + string sample = (indexes.TryGetValue(i, out var sampleFromStart), indexes.TryGetValue(i - length, out var sampleFromEnd)) switch + { + (true, false) => sampleFromStart, + (false, true) => sampleFromEnd, + _ => "_" + }; + + if (builder.Length > 1) + { + builder.Append(", "); + } + + builder.Append(sample); + } + builder.Append("]"); + + return pooledBuilder.ToStringAndFree(); + } + // Handle the special case of a recursive pattern string tryHandleRecursivePattern(ref bool unnamedEnumValue) { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs index 9ae9b9cea3793..e941073a95e54 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs @@ -3129,21 +3129,21 @@ public void Pattern_Nullability_Exhaustiveness() { Length: 0 or > 1 } => 0, }; -_ = o switch // didn't test for null +_ = o switch // 1, didn't test for null { [null] => 0, [not null] => 0, { Length: 0 or > 1 } => 0, }; -_ = o switch // didn't test for [null] +_ = o switch // 2, didn't test for [null] { null => 0, [not null] => 0, { Length: 0 or > 1 } => 0, }; -_ = o switch // didn't test for [not null] +_ = o switch // 3, didn't test for [not null] { null => 0, [null] => 0, @@ -3158,14 +3158,14 @@ public void Pattern_Nullability_Exhaustiveness() [.., not null] => 0, }; -_ = o switch // didn't test for [.., null] +_ = o switch // 4, didn't test for [null] { null => 0, [] => 0, [.., not null] => 0, }; -_ = o switch // didn't test for [.., not null] +_ = o switch // 5, didn't test for [not null] { null => 0, [] => 0, @@ -3179,25 +3179,65 @@ public void Pattern_Nullability_Exhaustiveness() [not null, ..] => 0, { Length: 0 or > 1 } => 0, }; + +_ = o switch // 6, didn't test for [_, null] +{ + null => 0, + [] => 0, + [_] => 0, + [.., not null] => 0, +}; + +_ = o switch // 7, didn't test for [null, _] +{ + null => 0, + [] => 0, + [_] => 0, + [not null, ..] => 0, +}; + +_ = o switch // 8, didn't test for { Length: 0 } +{ + null => 0, + [_, ..] => 0, +}; + +_ = o switch // 9, didn't test for [null] +{ + null => 0, + [] => 0, + [..var x, not null] => 0, +}; "; - // PROTOTYPE: incorrect exhaustiveness examples from explainer - var compilation = CreateCompilation(source); + var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range, TestSources.GetSubArray }); compilation.VerifyEmitDiagnostics( // (13,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern 'null' is not covered. - // _ = o switch // didn't test for null + // _ = o switch // 1, didn't test for null Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("null").WithLocation(13, 7), - // (20,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. - // _ = o switch // didn't test for [null] - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(20, 7), - // (27,7): 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. - // _ = o switch // didn't test for [not null] - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(27, 7), - // (42,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. - // _ = o switch // didn't test for [.., null] - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(42, 7), - // (49,7): 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. - // _ = o switch // didn't test for [.., not null] - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(49, 7) + // (20,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[null]' is not covered. + // _ = o switch // 2, didn't test for [null] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[null]").WithLocation(20, 7), + // (27,7): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[not null]' is not covered. + // _ = o switch // 3, didn't test for [not null] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[not null]").WithLocation(27, 7), + // (42,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[null]' is not covered. + // _ = o switch // 4, didn't test for [null] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[null]").WithLocation(42, 7), + // (49,7): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[not null]' is not covered. + // _ = o switch // 5, didn't test for [not null] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[not null]").WithLocation(49, 7), + // (64,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[_, null]' is not covered. + // _ = o switch // 6, didn't test for [_, null] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, null]").WithLocation(64, 7), + // (72,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[null, _]' is not covered. + // _ = o switch // 7, didn't test for [null, _] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[null, _]").WithLocation(72, 7), + // (80,7): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '{ Length: 0 }' is not covered. + // _ = o switch // 8, didn't test for { Length: 0 } + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("{ Length: 0 }").WithLocation(80, 7), + // (86,7): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[null]' is not covered. + // _ = o switch // 9, didn't test for [null] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[null]").WithLocation(86, 7) ); } @@ -3263,15 +3303,52 @@ public void M() } } "; - // PROTOTYPE: incorrect exhaustiveness examples from explainer 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. + // (20,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[.. null]' is not covered. // _ = this switch // no tests for null slice - 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. + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[.. null]").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 '[.. not null]' is not covered. // _ = this switch // no test for not null slice - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(27, 18) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. not null]").WithLocation(27, 18) + ); + } + + [Fact] + public void SlicePattern_Nullability_Exhaustiveness_WithNestedPropertyTest() + { + var source = @" +#nullable enable +using System; +class C +{ + public int Length => throw null!; + public D? this[Index i] => throw null!; + public D? this[Range r] => throw null!; + + public void M() + { + _ = this switch + { + null => 0, + [] => 0, + [_, _, ..] => 0, + [.. null] => 0, + [.. { Property: < 0 }] => 0, + }; + } +} + +class D +{ + public int Property { get; set; } +} +"; + var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range }); + compilation.VerifyEmitDiagnostics( + // (12,18): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[.. { Property: 0 }]' is not covered. + // _ = this switch + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. { Property: 0 }]").WithLocation(12, 18) ); } @@ -3316,6 +3393,7 @@ public void M() } } "; + // TODO2 // PROTOTYPE: unexpected exhaustiveness diagnostic var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range }); compilation.VerifyEmitDiagnostics( @@ -3878,15 +3956,14 @@ class C public int Count => throw null; public int this[int i] => throw null; }"; - // PROTOTYPE bad explanation for 2 var comp = CreateCompilation(src); comp.VerifyEmitDiagnostics( // (2,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '{ Count: 0 }' is not covered. // _ = new C() switch // 1 Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("{ Count: 0 }").WithLocation(2, 13), - // (8,13): 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. + // (8,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[0]' is not covered. // _ = new C() switch // 2 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(8, 13) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[0]").WithLocation(8, 13) ); } @@ -3928,12 +4005,11 @@ class C public int Count => throw null!; public string? this[int i] => throw null!; }"; - // PROTOTYPE bad explanations on 1 and 2 var comp = CreateCompilation(src); comp.VerifyEmitDiagnostics( - // (3,13): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '_' is not covered. + // (3,13): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[null]' is not covered. // _ = new C() switch // 1 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(3, 13), + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[null]").WithLocation(3, 13), // (18,13): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern 'null' is not covered. // _ = new C() switch // 2 Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("null").WithLocation(18, 13) @@ -3956,12 +4032,11 @@ class C public int Count => throw null; public int this[int i] => throw null; }"; - // PROTOTYPE bad explanation var comp = CreateCompilation(src); comp.VerifyEmitDiagnostics( - // (2,13): 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. + // (2,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[_, 0]' is not covered. // _ = new C() switch // 1 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(2, 13) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[_, 0]").WithLocation(2, 13) ); } @@ -3981,12 +4056,11 @@ class C public int Count => throw null; public int this[int i] => throw null; }"; - // PROTOTYPE bad explanation var comp = CreateCompilation(src); comp.VerifyEmitDiagnostics( - // (2,13): 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. + // (2,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[0, _]' is not covered. // _ = new C() switch // 1 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(2, 13) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[0, _]").WithLocation(2, 13) ); } @@ -4006,12 +4080,11 @@ class C public int Count => throw null; public int this[int i] => throw null; }"; - // PROTOTYPE bad explanation var comp = CreateCompilation(src); comp.VerifyEmitDiagnostics( - // (2,13): 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. + // (2,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[0]' is not covered. // _ = new C() switch // 1 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(2, 13) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[0]").WithLocation(2, 13) ); } From 89edb853a38deef5e7396b7b580150bb6a94fe68 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 18 Oct 2021 13:01:35 -0700 Subject: [PATCH 2/5] Use refactoring from alrz --- .../Portable/Binder/PatternExplainer.cs | 67 ++++++++++--------- .../PatternMatchingTests_ListPatterns.cs | 32 +++++++++ 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs index 7ddb42e7e126f..d05745d8ac495 100644 --- a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs +++ b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs @@ -284,33 +284,10 @@ string tryHandleNumericLimits(ref bool unnamedEnumValue) (BoundDagNonNullTest _, true) => true, _ => false }) && - ValueSetFactory.ForType(input.Type) is { } fac) + ValueSetFactory.ForInput(input) is { } fac) { // All we have are numeric constraints. Process them to compute a value not covered. - var remainingValues = fac.AllValues; - foreach (var constraint in constraints) - { - var (test, sense) = constraint; - switch (test) - { - case BoundDagValueTest v: - addRelation(BinaryOperatorKind.Equal, v.Value); - break; - case BoundDagRelationalTest r: - addRelation(r.Relation, r.Value); - break; - } - void addRelation(BinaryOperatorKind relation, ConstantValue value) - { - if (value.IsBad) - return; - var filtered = fac.Related(relation, value); - if (!sense) - filtered = filtered.Complement(); - remainingValues = remainingValues.Intersect(filtered); - } - } - + IValueSet remainingValues = computeRemainingValues(fac, constraints); if (remainingValues.Complement().IsEmpty) return "_"; @@ -320,6 +297,36 @@ void addRelation(BinaryOperatorKind relation, ConstantValue value) return null; } + static IValueSet computeRemainingValues(IValueSetFactory fac, ImmutableArray<(BoundDagTest test, bool sense)> constraints) + { + var remainingValues = fac.AllValues; + foreach (var constraint in constraints) + { + var (test, sense) = constraint; + switch (test) + { + case BoundDagValueTest v: + addRelation(BinaryOperatorKind.Equal, v.Value); + break; + case BoundDagRelationalTest r: + addRelation(r.Relation, r.Value); + break; + } + + void addRelation(BinaryOperatorKind relation, ConstantValue value) + { + if (value.IsBad) + return; + var filtered = fac.Related(relation, value); + if (!sense) + filtered = filtered.Complement(); + remainingValues = remainingValues.Intersect(filtered); + } + } + + return remainingValues; + } + // Handle the special case of a list pattern string tryHandleListPattern(ref bool unnamedEnumValue) { @@ -347,13 +354,9 @@ string tryHandleListPattern(ref bool unnamedEnumValue) case BoundDagPropertyEvaluation e when e.IsLengthOrCount: { Debug.Assert(length == -1); - var subInput = new BoundDagTemp(e.Syntax, e.Property.Type, e); - var lengthValue = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); - if (int.TryParse(lengthValue, out var parsedLength)) - { - Debug.Assert(parsedLength >= 0); - length = parsedLength; - } + var lengthTemp = new BoundDagTemp(e.Syntax, e.Property.Type, e); + var lengthValues = (IValueSet)computeRemainingValues(ValueSetFactory.ForLength, getArray(constraintMap, lengthTemp)); + length = lengthValues.Sample.Int32Value; } break; case BoundDagIndexerEvaluation e: diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs index e941073a95e54..a8fc76799e085 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs @@ -3409,6 +3409,38 @@ public void M() ); } + [Fact] + public void SlicePattern_Nullability_Exhaustiveness_Multiple() + { + var source = @" +#nullable enable +using System; +class C +{ + public int Length => throw null!; + public object? this[Index i] => throw null!; + public C? this[Range r] => throw null!; + + public void M() + { + _ = this switch + { + null => 0, + [] => 0, + [1, .., 2, .., 3] => 0, + { Length: > 1 } => 0, + }; + } +} +"; + var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range }); + compilation.VerifyEmitDiagnostics( + // (16,24): error CS9202: Slice patterns may only be used once and directly inside a list pattern. + // [1, .., 2, .., 3] => 0, + Diagnostic(ErrorCode.ERR_MisplacedSlicePattern, "..").WithLocation(16, 24) + ); + } + [Fact] public void ListPattern_Dynamic() { From 4f6cc89bf958e770bececc608120c9a42a97e86f Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 18 Oct 2021 20:08:31 -0700 Subject: [PATCH 3/5] Simplify logic --- .../Portable/Binder/PatternExplainer.cs | 75 +++++++------- .../PatternMatchingTests_ListPatterns.cs | 99 ++++++++++++------- 2 files changed, 105 insertions(+), 69 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs index d05745d8ac495..2c38a276b16c6 100644 --- a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs +++ b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs @@ -344,61 +344,64 @@ string tryHandleListPattern(ref bool unnamedEnumValue) return null; } - var indexes = new Dictionary(); - int length = -1; + if (evaluations[0] is not BoundDagPropertyEvaluation { IsLengthOrCount: true } lengthOrCount) + { + return null; + } - foreach (var eval in evaluations) + var lengthTemp = new BoundDagTemp(lengthOrCount.Syntax, lengthOrCount.Property.Type, lengthOrCount); + var lengthValues = (IValueSet)computeRemainingValues(ValueSetFactory.ForLength, getArray(constraintMap, lengthTemp)); + var length = lengthValues.Sample.Int32Value; + + BoundDagSliceEvaluation slice = null; + var subpatterns = new ArrayBuilder(length); + subpatterns.AddMany("_", length); + + for (int i = 1; i < evaluations.Length; i++) { - switch (eval) + switch (evaluations[i]) { - case BoundDagPropertyEvaluation e when e.IsLengthOrCount: - { - Debug.Assert(length == -1); - var lengthTemp = new BoundDagTemp(e.Syntax, e.Property.Type, e); - var lengthValues = (IValueSet)computeRemainingValues(ValueSetFactory.ForLength, getArray(constraintMap, lengthTemp)); - length = lengthValues.Sample.Int32Value; - } - break; case BoundDagIndexerEvaluation e: - { - var subInput = new BoundDagTemp(e.Syntax, e.IndexerType, e); - var subPattern = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); - indexes.Add(e.Index, subPattern); - } + var indexerTemp = new BoundDagTemp(e.Syntax, e.IndexerType, e); + int integerIndex = e.Index; + var newPattern = SamplePatternForTemp(indexerTemp, constraintMap, evaluationMap, requireExactType: false, ref unnamedEnumValue); + var index = integerIndex >= 0 ? integerIndex : length + integerIndex; + + Debug.Assert(subpatterns[index] == "_"); + subpatterns[index] = newPattern; break; case BoundDagSliceEvaluation e: - { - if (e.StartIndex < length + e.EndIndex) - { - var subInput = new BoundDagTemp(e.Syntax, e.SliceType, e); - var subPattern = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); - indexes.Add(e.StartIndex, ".. " + subPattern); - } - } + Debug.Assert(slice is null); + slice = e; break; default: return null; } } - if (length < 0) - { - Debug.Assert(false); - return null; - } - var pooledBuilder = PooledStringBuilder.GetInstance(); var builder = pooledBuilder.Builder; builder.Append("["); for (int i = 0; i < length; i++) { - string sample = (indexes.TryGetValue(i, out var sampleFromStart), indexes.TryGetValue(i - length, out var sampleFromEnd)) switch + if (slice?.StartIndex == i) { - (true, false) => sampleFromStart, - (false, true) => sampleFromEnd, - _ => "_" - }; + int endIndex = slice.EndIndex >= 0 ? slice.EndIndex : length + slice.EndIndex - 1; + var subInput = new BoundDagTemp(slice.Syntax, slice.SliceType, slice); + var sample2 = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); + if (sample2 != "_") + { + if (builder.Length > 1) + { + builder.Append(", "); + } + + builder.Append(".. " + sample2); + } + } + + string sample = subpatterns[i]; if (builder.Length > 1) { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs index a8fc76799e085..68298621b8545 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs @@ -3263,14 +3263,14 @@ public void M() [.. not null] => 0, }; - _ = this switch // no tests for null slice + _ = this switch // no tests for [.. null, _] { null => 0, [] => 0, [.. not null] => 0, }; - _ = this switch // no test for not null slice + _ = this switch // no test for [.. not null, _] { null => 0, [] => 0, @@ -3305,12 +3305,12 @@ public void M() "; 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 '[.. null]' is not covered. - // _ = this switch // no tests for null slice - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[.. null]").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 '[.. not null]' is not covered. - // _ = this switch // no test for not null slice - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. not null]").WithLocation(27, 18) + // (20,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[.. null, _]' is not covered. + // _ = this switch // no tests for [.. null, _] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[.. null, _]").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 '[.. not null, _]' is not covered. + // _ = this switch // no test for [.. not null, _] + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. not null, _]").WithLocation(27, 18) ); } @@ -3346,9 +3346,9 @@ class D "; var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range }); compilation.VerifyEmitDiagnostics( - // (12,18): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[.. { Property: 0 }]' is not covered. + // (12,18): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[.. { Property: 0 }, _]' is not covered. // _ = this switch - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. { Property: 0 }]").WithLocation(12, 18) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. { Property: 0 }, _]").WithLocation(12, 18) ); } @@ -3366,29 +3366,41 @@ class C public void M() { - _ = this switch - { - null => 0, - [] => 0, - [.. [null]] => 0, - [not null] => 0, - { Length: > 1 } => 0, - }; - - _ = this switch // didn't test for not null first element - { - null => 0, - [] => 0, - [.. [null]] => 0, - { Length: > 1 } => 0, - }; - - _ = this switch // didn't test for null first element + //_ = this switch + //{ + // null => 0, + // [] => 0, + // [.. [null]] => 0, + // [not null] => 0, + // { Length: > 1 } => 0, + //}; + + //_ = this switch // didn't test for not null first element + //{ + // null => 0, + // [] => 0, + // [.. [null]] => 0, + // { Length: > 1 } => 0, + //}; + + //_ = this switch // didn't test for null first element + //{ + // null => 0, + // [] => 0, + // [.. [not null]] => 0, + // { Length: > 1 } => 0, + //}; + + //_ = this switch // didn't test for null slice + //{ + // null or { Length: not 4 } => 0, + // [_, .. [_, not null], _] => 0, + //}; + + _ = this switch // didn't test for { - null => 0, - [] => 0, - [.. [not null]] => 0, - { Length: > 1 } => 0, + null or { Length: not 4 } => 0, + [_, .. null or [_, not null], _] => 0, }; } } @@ -4107,6 +4119,21 @@ public void ListPattern_Exhaustiveness_LastPosition() { Count: 0 } => 3, }; +_ = new C() switch // 2 +{ + { Count: <= 2 } => 1, + [.., > 0] => 2, + [.., < 0] => 3, +}; + +_ = new C() switch // 3 +{ + { Count: <= 2 } => 1, + [0, ..] => 2, + [.., > 0] => 3, + [.., < 0] => 4, +}; + class C { public int Count => throw null; @@ -4116,7 +4143,13 @@ class C comp.VerifyEmitDiagnostics( // (2,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[0]' is not covered. // _ = new C() switch // 1 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[0]").WithLocation(2, 13) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[0]").WithLocation(2, 13), + // (9,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[_, _, 0]' is not covered. + // _ = new C() switch // 2 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[_, _, 0]").WithLocation(9, 13), + // (16,13): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[1, _, 0]' is not covered. + // _ = new C() switch // 3 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[1, _, 0]").WithLocation(16, 13) ); } From 3144a3da1f0fbdda3e5825350e12ef91246cab3f Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 18 Oct 2021 21:17:54 -0700 Subject: [PATCH 4/5] Compute example length for list pattern in slice --- .../Portable/Binder/PatternExplainer.cs | 51 ++-- .../PatternMatchingTests_ListPatterns.cs | 256 +++++++++++++++--- 2 files changed, 249 insertions(+), 58 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs index 2c38a276b16c6..fd0f230761cdd 100644 --- a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs +++ b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs @@ -348,10 +348,7 @@ string tryHandleListPattern(ref bool unnamedEnumValue) { return null; } - - var lengthTemp = new BoundDagTemp(lengthOrCount.Syntax, lengthOrCount.Property.Type, lengthOrCount); - var lengthValues = (IValueSet)computeRemainingValues(ValueSetFactory.ForLength, getArray(constraintMap, lengthTemp)); - var length = lengthValues.Sample.Int32Value; + int length = computeLength(new BoundDagTemp(lengthOrCount.Syntax, lengthOrCount.Property.Type, lengthOrCount)); BoundDagSliceEvaluation slice = null; var subpatterns = new ArrayBuilder(length); @@ -387,34 +384,54 @@ string tryHandleListPattern(ref bool unnamedEnumValue) { if (slice?.StartIndex == i) { - int endIndex = slice.EndIndex >= 0 ? slice.EndIndex : length + slice.EndIndex - 1; var subInput = new BoundDagTemp(slice.Syntax, slice.SliceType, slice); - var sample2 = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); + var sample2 = SamplePatternForTemp(subInput, constraintMap, evaluationMap, false, ref unnamedEnumValue); if (sample2 != "_") { - if (builder.Length > 1) - { - builder.Append(", "); - } + appendWithSeparatorIfNeeded(builder, ".. " + sample2); + } - builder.Append(".. " + sample2); + if (sample2.StartsWith("[")) // TODO2 + { + // The nested slice is handling a section of the list + int endIndex = slice.EndIndex >= 0 ? slice.EndIndex : length + slice.EndIndex - 1; + i += endIndex - slice.StartIndex; + continue; } } string sample = subpatterns[i]; - if (builder.Length > 1) - { - builder.Append(", "); - } - - builder.Append(sample); + appendWithSeparatorIfNeeded(builder, sample); } builder.Append("]"); return pooledBuilder.ToStringAndFree(); } + static void appendWithSeparatorIfNeeded(StringBuilder builder, string sample) + { + if (builder.Length > 1) + { + builder.Append(", "); + } + + builder.Append(sample); + } + + int computeLength(BoundDagTemp lengthTemp) + { + if (lengthTemp is { Source: BoundDagPropertyEvaluation { Input: { Source: BoundDagSliceEvaluation slice } } }) + { + var outerLength = computeLength(slice.LengthTemp); + return outerLength - slice.StartIndex + slice.EndIndex; + } + + var lengthValues = (IValueSet)computeRemainingValues(ValueSetFactory.ForLength, getArray(constraintMap, lengthTemp)); + var length = lengthValues.Sample.Int32Value; + return length; + } + // Handle the special case of a recursive pattern string tryHandleRecursivePattern(ref bool unnamedEnumValue) { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs index 68298621b8545..4b40d93010abf 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs @@ -3355,6 +3355,7 @@ class D [Fact] public void SlicePattern_Nullability_Exhaustiveness_NestedSlice() { + // TODO2 verify execution/DAG var source = @" #nullable enable using System; @@ -3366,42 +3367,69 @@ class C public void M() { - //_ = this switch - //{ - // null => 0, - // [] => 0, - // [.. [null]] => 0, - // [not null] => 0, - // { Length: > 1 } => 0, - //}; - - //_ = this switch // didn't test for not null first element - //{ - // null => 0, - // [] => 0, - // [.. [null]] => 0, - // { Length: > 1 } => 0, - //}; - - //_ = this switch // didn't test for null first element - //{ - // null => 0, - // [] => 0, - // [.. [not null]] => 0, - // { Length: > 1 } => 0, - //}; - - //_ = this switch // didn't test for null slice - //{ - // null or { Length: not 4 } => 0, - // [_, .. [_, not null], _] => 0, - //}; - - _ = this switch // didn't test for + _ = this switch + { + null or { Length: not 1 } => 0, + [.. null] => 0, + [.. [null]] => 0, + [not null] => 0, + }; + + _ = this switch // TODO2 we didn't test for [.. null] but we're looking for an example with Length=1. incorrect explanation [.. null, null] // 1 + { + null or { Length: not 1 } => 0, + [.. [null]] => 0, + [not null] => 0, + }; + + _ = this switch // didn't test for [.. [not null]] // TODO2 // 2 + { + null or { Length: not 1 } => 0, + [.. [null]] => 0, + }; + + _ = this switch // didn't test for [.. [not null]] // TODO2 // 3 + { + null or { Length: not 1 } => 0, + [.. null] => 0, + [.. [null]] => 0, + }; + + _ = this switch // didn't test for [.. null, _] // TODO2 we're trying to construct an example with Length=1, the slice may not be null // 4 + { + null or { Length: not 1 } => 0, + [.. [not null]] => 0, + }; + + _ = this switch // didn't test for [_, .. null, _, _, _] // TODO2 we're trying to construct an example with Length=4, the slice may not be null // 5 + { + null or { Length: not 4 } => 0, + [_, .. [_, not null], _] => 0, + }; + + _ = this switch // TODO2 we should consider this switch exhaustive // 6 + { + null or { Length: not 4 } => 0, + [_, .. [_, _], _] => 0, + }; + + _ = this switch // didn't test for [_, .. [_, null], _] // 7 { null or { Length: not 4 } => 0, [_, .. null or [_, not null], _] => 0, }; + + _ = this switch // didn't test for [_, .. [_, null], _, _] // 8 + { + null or { Length: not 5 } => 0, + [_, .. null or [_, not null], _, _] => 0, + }; + + _ = this switch // didn't test for [_, .. [_, null, _], _] // 9 + { + null or { Length: not 5 } => 0, + [_, .. null or [_, not null, _], _] => 0, + }; } } "; @@ -3409,15 +3437,33 @@ public void M() // PROTOTYPE: unexpected exhaustiveness diagnostic var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range }); compilation.VerifyEmitDiagnostics( - // (12,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 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(12, 18), - // (21,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 first element - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(21, 18), - // (29,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 first element - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("_").WithLocation(29, 18) + // (20,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[.. null, null]' is not covered. + // _ = this switch // TODO2 we didn't test for [.. null] but we're looking for an example with Length=1. incorrect explanation [.. null, null] // 1 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[.. null, null]").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 '[.. [not null]]' is not covered. + // _ = this switch // didn't test for [.. [not null]] // TODO2 // 2 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. [not null]]").WithLocation(27, 18), + // (33,18): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '[.. [not null]]' is not covered. + // _ = this switch // didn't test for [.. [not null]] // TODO2 // 3 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("[.. [not null]]").WithLocation(33, 18), + // (40,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[.. null, _]' is not covered. + // _ = this switch // didn't test for [.. null, _] // TODO2 we're trying to construct an example with Length=1, the slice may not be null // 4 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[.. null, _]").WithLocation(40, 18), + // (46,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[_, .. null, _, _, _]' is not covered. + // _ = this switch // didn't test for [_, .. null, _, _, _] // TODO2 we're trying to construct an example with Length=4, the slice may not be null // 5 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. null, _, _, _]").WithLocation(46, 18), + // (52,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[_, .. null, _, _, _]' is not covered. + // _ = this switch // TODO2 we should consider this switch exhaustive // 6 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. null, _, _, _]").WithLocation(52, 18), + // (58,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[_, .. [_, null], _]' is not covered. + // _ = this switch // didn't test for [_, .. [_, null], _] // 7 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. [_, null], _]").WithLocation(58, 18), + // (64,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[_, .. [_, null], _, _]' is not covered. + // _ = this switch // didn't test for [_, .. [_, null], _, _] // 8 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. [_, null], _, _]").WithLocation(64, 18), + // (70,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[_, .. [_, null, _], _]' is not covered. + // _ = this switch // didn't test for [_, .. [_, null, _], _] // 9 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. [_, null, _], _]").WithLocation(70, 18) ); } @@ -5411,4 +5457,132 @@ .locals init (C V_0, IL_0068: ret }"); } + + [Fact] + public void SlicePattern_OnConsList() + { + var source = @" +#nullable enable +using System; + +record ConsList(object Head, ConsList? Tail) +{ + public int Length => throw null!; + public object this[Index i] => throw null!; + public ConsList? this[Range r] => throw null!; + + public static void Print(ConsList? list) + { + switch (list) + { + case null: + return; + case [var head, .. var tail]: + System.Console.Write(head.ToString() + "" ""); + Print(tail); + return; + } + } +} +"; + // Note: this pattern doesn't work well because list-patterns needs a functional Length + var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range, IsExternalInitTypeDefinition }); + compilation.VerifyDiagnostics(); + var verifier = CompileAndVerify(compilation); + verifier.VerifyIL("ConsList.Print", @" +{ + // Code size 84 (0x54) + .maxstack 4 + .locals init (object V_0, //head + ConsList V_1) //tail + IL_0000: ldarg.0 + IL_0001: brfalse.s IL_0036 + IL_0003: ldarg.0 + IL_0004: callvirt ""int ConsList.Length.get"" + IL_0009: ldc.i4.1 + IL_000a: blt.s IL_0053 + IL_000c: ldarg.0 + IL_000d: ldc.i4.0 + IL_000e: ldc.i4.0 + IL_000f: newobj ""System.Index..ctor(int, bool)"" + IL_0014: callvirt ""object ConsList.this[System.Index].get"" + IL_0019: stloc.0 + IL_001a: ldarg.0 + IL_001b: ldc.i4.1 + IL_001c: ldc.i4.0 + IL_001d: newobj ""System.Index..ctor(int, bool)"" + IL_0022: ldc.i4.0 + IL_0023: ldc.i4.1 + IL_0024: newobj ""System.Index..ctor(int, bool)"" + IL_0029: newobj ""System.Range..ctor(System.Index, System.Index)"" + IL_002e: callvirt ""ConsList ConsList.this[System.Range].get"" + IL_0033: stloc.1 + IL_0034: br.s IL_0037 + IL_0036: ret + IL_0037: ldloc.0 + IL_0038: callvirt ""string object.ToString()"" + IL_003d: ldstr "" "" + IL_0042: call ""string string.Concat(string, string)"" + IL_0047: call ""void System.Console.Write(string)"" + IL_004c: ldloc.1 + IL_004d: call ""void ConsList.Print(ConsList)"" + IL_0052: ret + IL_0053: ret +} +"); + } + + [Fact] + public void PositionalPattern_OnConsList() + { + var source = @" +#nullable enable + +var list = new ConsList(1, new ConsList(2, new ConsList(3, null))); +ConsList.Print(list); + +record ConsList(object Head, ConsList? Tail) +{ + public static void Print(ConsList? list) + { + switch (list) + { + case null: + return; + case (var head, var tail): + System.Console.Write(head.ToString() + "" ""); + Print(tail); + return; + } + } +} +"; + var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range, IsExternalInitTypeDefinition }); + compilation.VerifyDiagnostics(); + var verifier = CompileAndVerify(compilation, expectedOutput: "1 2 3"); + verifier.VerifyIL("ConsList.Print", @" +{ + // Code size 44 (0x2c) + .maxstack 3 + .locals init (object V_0, //head + ConsList V_1) //tail + IL_0000: ldarg.0 + IL_0001: brfalse.s IL_000f + IL_0003: ldarg.0 + IL_0004: ldloca.s V_0 + IL_0006: ldloca.s V_1 + IL_0008: callvirt ""void ConsList.Deconstruct(out object, out ConsList)"" + IL_000d: br.s IL_0010 + IL_000f: ret + IL_0010: ldloc.0 + IL_0011: callvirt ""string object.ToString()"" + IL_0016: ldstr "" "" + IL_001b: call ""string string.Concat(string, string)"" + IL_0020: call ""void System.Console.Write(string)"" + IL_0025: ldloc.1 + IL_0026: call ""void ConsList.Print(ConsList)"" + IL_002b: ret +} +"); + } } From 3f133d4bc9d0b66a0f3e94da1500887959e69c3f Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 19 Oct 2021 00:07:49 -0700 Subject: [PATCH 5/5] tests --- .../Portable/Binder/PatternExplainer.cs | 6 +-- .../PatternMatchingTests_ListPatterns.cs | 38 ++++++++++++++++++- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs index fd0f230761cdd..df952b8ec32c4 100644 --- a/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs +++ b/src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs @@ -391,7 +391,7 @@ string tryHandleListPattern(ref bool unnamedEnumValue) appendWithSeparatorIfNeeded(builder, ".. " + sample2); } - if (sample2.StartsWith("[")) // TODO2 + if (sample2.StartsWith("[")) { // The nested slice is handling a section of the list int endIndex = slice.EndIndex >= 0 ? slice.EndIndex : length + slice.EndIndex - 1; @@ -400,9 +400,7 @@ string tryHandleListPattern(ref bool unnamedEnumValue) } } - string sample = subpatterns[i]; - - appendWithSeparatorIfNeeded(builder, sample); + appendWithSeparatorIfNeeded(builder, subpatterns[i]); } builder.Append("]"); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs index 4b40d93010abf..51841cf528c4f 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_ListPatterns.cs @@ -3430,6 +3430,12 @@ public void M() null or { Length: not 5 } => 0, [_, .. null or [_, not null, _], _] => 0, }; + + _ = this switch // didn't test for [.. null, _] // TODO2 we're trying to construct an example with Length=1 but a null slice // 10 + { + null or { Length: not 1 } => 0, + [.. { Length: 1 }] => 0, + }; } } "; @@ -3463,7 +3469,10 @@ public void M() Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. [_, null], _, _]").WithLocation(64, 18), // (70,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[_, .. [_, null, _], _]' is not covered. // _ = this switch // didn't test for [_, .. [_, null, _], _] // 9 - Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. [_, null, _], _]").WithLocation(70, 18) + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[_, .. [_, null, _], _]").WithLocation(70, 18), + // (76,18): warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[.. null, _]' is not covered. + // _ = this switch // didn't test for [.. null, _] // TODO2 we're trying to construct an example with Length=1 but a null slice // 10 + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, "switch").WithArguments("[.. null, _]").WithLocation(76, 18) ); } @@ -4240,6 +4249,33 @@ class C comp.VerifyEmitDiagnostics(); } + [Fact] + public void ListPattern_Exhaustiveness_Conjunction() + { + var src = @" +_ = new C() switch +{ + { Count: not 1 } => 0, + [0] or not Derived => 0, +}; + +class C +{ + public int Count => throw null; + public int this[int i] => throw null; + public C Slice(int i, int j) => throw null; +} +class Derived : C { } +"; + // Note: we don't know how to explain `Derived and [1]` + var comp = CreateCompilation(src); + comp.VerifyEmitDiagnostics( + // (2,13): 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. + // _ = new C() switch + Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(2, 13) + ); + } + [Fact] public void ListPattern_UintCount() {