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.

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

List-patterns: mitigate break with Length property patterns #56721

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## This document lists known breaking changes in Roslyn after .NET 6 all the way to .NET 7.

1. Beginning with C# 11.0, `Length` and `Count` properties on countable and indexable types
are assumed to be non-negative for purpose of subsumption and exhaustiveness analysis of patterns and switches.
Those types can be used with implicit Index indexer and list patterns.

```csharp
void M(int[] i)
{
if (i is { Length: -1 }) {} // warning: impossible under assumption of non-negative length
}
```
1 change: 1 addition & 0 deletions docs/compilers/CSharp/readme.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
Breaking changes (newest release on top):

- [.NET 7](https://github.com/dotnet/roslyn/blob/main/docs/compilers/CSharp/Compiler%20Breaking%20Changes%20-%20DotNet%207.md)
- [.NET 6 and Visual Studio 2022](https://github.com/dotnet/roslyn/blob/main/docs/compilers/CSharp/Compiler%20Breaking%20Changes%20-%20DotNet%206.md)
- [Minor releases following .NET 5 and Visual Studio 2019 version 16.9](https://github.com/dotnet/roslyn/blob/main/docs/compilers/CSharp/Compiler%20Breaking%20Changes%20-%20post%20DotNet%205.md)
- [.NET 5 and Visual Studio 2019 version 16.9](https://github.com/dotnet/roslyn/blob/main/docs/compilers/CSharp/Compiler%20Breaking%20Changes%20-%20DotNet%205.md)
Expand Down
35 changes: 26 additions & 9 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,20 @@ private BoundExpression MakeIsPatternExpression(
BoundDecisionDag decisionDag = DecisionDagBuilder.CreateDecisionDagForIsPattern(
this.Compilation, pattern.Syntax, expression, innerPattern, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, diagnostics);

if (!hasErrors && getConstantResult(decisionDag, negated, whenTrueLabel, whenFalseLabel) is { } constantResult)
if (!hasErrors && getConstantResult(decisionDag, negated, whenTrueLabel, whenFalseLabel) is ({ } constantResult, var isWeakConstantResult))
{
if (!constantResult)
{
Debug.Assert(expression.Type is object);
diagnostics.Add(ErrorCode.ERR_IsPatternImpossible, node.Location, expression.Type);
hasErrors = true;
if (isWeakConstantResult)
{
diagnostics.Add(ErrorCode.WRN_IsPatternImpossibleIfNonNegativeLength, node.Location, expression.Type);
}
else
{
diagnostics.Add(ErrorCode.ERR_IsPatternImpossible, node.Location, expression.Type);
hasErrors = true;
}
}
else
{
Expand All @@ -79,7 +86,7 @@ private BoundExpression MakeIsPatternExpression(
case BoundNegatedPattern _:
case BoundBinaryPattern _:
Debug.Assert(expression.Type is object);
diagnostics.Add(ErrorCode.WRN_IsPatternAlways, node.Location, expression.Type);
diagnostics.Add(isWeakConstantResult ? ErrorCode.WRN_IsPatternAlwaysIfNonNegativeLength : ErrorCode.WRN_IsPatternAlways, node.Location, expression.Type);
break;
case BoundDiscardPattern _:
// we do not give a warning on this because it is an existing scenario, and it should
Expand All @@ -94,8 +101,9 @@ private BoundExpression MakeIsPatternExpression(
}
else if (expression.ConstantValue != null)
{
// TODO2
decisionDag = decisionDag.SimplifyDecisionDagIfConstantInput(expression);
if (!hasErrors && getConstantResult(decisionDag, negated, whenTrueLabel, whenFalseLabel) is { } simplifiedResult)
if (!hasErrors && getConstantResult(decisionDag, negated, whenTrueLabel, whenFalseLabel) is ({ } simplifiedResult, var isWeakSimplifiedResult))
{
if (!simplifiedResult)
{
Expand Down Expand Up @@ -125,17 +133,26 @@ private BoundExpression MakeIsPatternExpression(
return new BoundIsPatternExpression(
node, expression, pattern, negated, decisionDag, whenTrueLabel: whenTrueLabel, whenFalseLabel: whenFalseLabel, boolType, hasErrors);

static bool? getConstantResult(BoundDecisionDag decisionDag, bool negated, LabelSymbol whenTrueLabel, LabelSymbol whenFalseLabel)
static (bool? Result, bool IsWeakResult) getConstantResult(BoundDecisionDag decisionDag, bool negated, LabelSymbol whenTrueLabel, LabelSymbol whenFalseLabel)
{
// TODO2 discount null tests from analysis
if (!decisionDag.ReachableLabels.Contains(whenTrueLabel))
{
return negated;
return (negated, false);
}
else if (!decisionDag.ReachableLabels.Contains(whenFalseLabel))
{
return !negated;
return (!negated, false);
}
else if (!decisionDag.ReachableWithoutNegativeBranchLabels.Contains(whenTrueLabel))
{
return (negated, true);
}
else if (!decisionDag.ReachableWithoutNegativeBranchLabels.Contains(whenFalseLabel))
{
return (!negated, true);
}
return null;
return (null, false);
}
}

Expand Down
Loading