Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

List patterns: counterexample for non-exhaustive switch expressions #55327

Merged
merged 14 commits into from
Oct 27, 2021
Merged

List patterns: counterexample for non-exhaustive switch expressions #55327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cc00129
WIP
alrz Oct 18, 2021
e9eb399
WIP
alrz Oct 18, 2021
7dc6ac4
Cherry-pick tests from #57210
alrz Oct 19, 2021
4b29226
Update baseline
alrz Oct 19, 2021
abd7676
Skip discarded slice pattern
alrz Oct 19, 2021
3012db0
Bail if there's a slice but only one length value is remained
alrz Oct 20, 2021
b5b117e
Reorder
alrz Oct 20, 2021
2bf9fc4
Remove comments
alrz Oct 20, 2021
ddbc0c6
Add test
alrz Oct 22, 2021
7121a44
Bound check
alrz Oct 22, 2021
7dc82d4
Reword
alrz Oct 22, 2021
b3db969
Bail if the sample value is less than the required minimum length by …
alrz Oct 22, 2021
dcaac93
Add comments
alrz Oct 22, 2021
01ea646
PR feedback
alrz Oct 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 147 additions & 39 deletions src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ private static string SamplePatternForTemp(
tryHandleTuplePattern(ref unnamedEnumValue) ??
tryHandleNumericLimits(ref unnamedEnumValue) ??
tryHandleRecursivePattern(ref unnamedEnumValue) ??
tryHandleListPattern(ref unnamedEnumValue) ??
produceFallbackPattern();

static ImmutableArray<T> getArray<T>(Dictionary<BoundDagTemp, ArrayBuilder<T>> map, BoundDagTemp temp)
Expand Down Expand Up @@ -236,6 +237,104 @@ constraints[0] is (BoundDagNonNullTest _, true) &&
return null;
}

// Handle the special case of a list pattern
string tryHandleListPattern(ref bool unnamedEnumValue)
{
if (constraints.IsEmpty && evaluations.IsEmpty)
return null;

// not-null tests are implicitly incorporated into a list pattern
if (!constraints.All(isNotNullTest))
{
return null;
}

if (evaluations[0] is BoundDagPropertyEvaluation { IsLengthOrCount: true } lengthOrCount)
{
BoundDagSliceEvaluation slice = null;
for (int i = 1; i < evaluations.Length; i++)
{
switch (evaluations[i])
{
case BoundDagIndexerEvaluation:
continue;
case BoundDagSliceEvaluation e:
if (slice != null)
{
// A list pattern can only support a single slice within.
// We won't try to generate a list pattern if there's more.
return null;
}
slice = e;
continue;
default:
return null;
}
}

var lengthTemp = new BoundDagTemp(lengthOrCount.Syntax, lengthOrCount.Property.Type, lengthOrCount);
var lengthValues = (IValueSet<int>)computeRemainingValues(ValueSetFactory.ForLength, getArray(constraintMap, lengthTemp));
int lengthValue = lengthValues.Sample.Int32Value;
if (slice != null)
{
if (lengthValues.All(BinaryOperatorKind.Equal, lengthValue))
{
// Bail if there's a slice but only one length value is remained.
// That could happen with nested slice patterns or length tests
// and also with very long list patterns in certain conditions.
return null;
}

if (slice.StartIndex - slice.EndIndex > lengthValue)
{
// Bail if the sample value is less than the required minimum length by the slice
// to avoid generating an incorrect example.
return null;
Copy link
Member

@jcouv jcouv Oct 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this reachable? If not, consider throwing UnreachableException instead.
Ah, saw your comment about the explainer being defensive. #Closed

}
}

var subpatterns = new ArrayBuilder<string>(lengthValue);
subpatterns.AddMany("_", lengthValue);
for (int i = 1; i < evaluations.Length; i++)
{
switch (evaluations[i])
{
case BoundDagIndexerEvaluation e:
var indexerTemp = new BoundDagTemp(e.Syntax, e.IndexerType, e);
int index = e.Index;
int effectiveIndex = index < 0 ? lengthValue + index : index;
if (effectiveIndex < 0 || effectiveIndex >= lengthValue)
return null;
var oldPattern = subpatterns[effectiveIndex];
var newPattern = SamplePatternForTemp(indexerTemp, constraintMap, evaluationMap, requireExactType: false, ref unnamedEnumValue);
subpatterns[effectiveIndex] = makeConjunct(oldPattern, newPattern);
continue;
case BoundDagSliceEvaluation e:
Debug.Assert(e == slice);
continue;
case var v:
throw ExceptionUtilities.UnexpectedValue(v);
}
}

if (slice != null)
{
var sliceTemp = new BoundDagTemp(slice.Syntax, slice.SliceType, slice);
var slicePattern = SamplePatternForTemp(sliceTemp, constraintMap, evaluationMap, requireExactType: false, ref unnamedEnumValue);
if (slicePattern != "_")
{
// If the slice is not matched against any pattern, the slice pattern would
// have no effect on the output given the provided sample length value.
subpatterns.Insert(slice.StartIndex, $".. {slicePattern}");
}
}

return "[" + string.Join(", ", subpatterns) + "]";
}

return null;
}

// Handle the special case of a tuple pattern
string tryHandleTuplePattern(ref bool unnamedEnumValue)
{
Expand All @@ -262,13 +361,6 @@ string tryHandleTuplePattern(ref bool unnamedEnumValue)
}

return null;

static string makeConjunct(string oldPattern, string newPattern) => (oldPattern, newPattern) switch
{
("_", var x) => x,
(var x, "_") => x,
(var x, var y) => x + " and " + y
};
}

// Handle the special case of numeric limits
Expand All @@ -283,33 +375,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 "_";

Expand All @@ -325,13 +394,8 @@ string tryHandleRecursivePattern(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,
}))
// not-null tests are implicitly incorporated into a recursive pattern
if (!constraints.All(isNotNullTest))
{
return null;
}
Expand Down Expand Up @@ -399,6 +463,50 @@ string produceFallbackPattern()
{
return requireExactType ? input.Type.ToDisplayString() : "_";
}

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;
}

static string makeConjunct(string oldPattern, string newPattern) => (oldPattern, newPattern) switch
{
("_", var x) => x,
(var x, "_") => x,
(var x, var y) => x + " and " + y
};

static bool isNotNullTest((BoundDagTest test, bool sense) constraint)
{
return constraint is
(test: BoundDagNonNullTest _, sense: true) or
(test: BoundDagExplicitNullTest _, sense: false);
}
}

private static string SampleValueString(IValueSet remainingValues, TypeSymbol type, bool requireExactType, ref bool unnamedEnumValue)
Expand Down
Loading