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

recursive-patterns(19) Distinguish irrefutable and refutable patterns in the is expression #26035

Merged
merged 4 commits into from
Apr 11, 2018

Conversation

gafter
Copy link
Member

@gafter gafter commented Apr 9, 2018

Both refutable and irrefutable is-pattern matches affect definite assignment:

Also, we check to see if a constant input makes the result refutable or irrefutable:

Both refutable and irrefutable is-pattern matches affect definite assignment:
- an irrefutable match leaves pattern variables definitely assigned
- a refutable match is an error
Fixes dotnet#25890

Also, we check to see if a constant input makes the result refutable or irrefutable:
- If a constant cannot match the pattern, we issue a warning
- If a constant always matches a constant pattern, we issue a warning
The latter two are subject to compat council approval.
Fixes dotnet#16099
@gafter gafter added Area-Compilers 4 - In Review A fix for the issue is submitted for review. New Language Feature - Pattern Matching Pattern Matching labels Apr 9, 2018
@gafter gafter added this to the 16.0 milestone Apr 9, 2018
@gafter gafter self-assigned this Apr 9, 2018
@gafter gafter requested review from agocke and cston April 9, 2018 18:57
@gafter gafter requested a review from a team as a code owner April 9, 2018 18:57
@gafter gafter changed the title Distinguish irrefutable and refutable patterns in the is expression recursive-patterns(19) Distinguish irrefutable and refutable patterns in the is expression Apr 9, 2018
@gafter
Copy link
Member Author

gafter commented Apr 9, 2018

@agocke @cston Please review these pattern-matching changes.
@dotnet/roslyn-compiler FYI
#Closed

}

operand = BadExpression(node, operand).MakeCompilerGenerated();
Copy link
Member

@agocke agocke Apr 9, 2018

Choose a reason for hiding this comment

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

Why are we doing this unconditionally now? #ByDesign

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it is unconditionally illegal to have a lambda or method group here. This line just gives the expression an error type so later code doesn't see a typeless expression.


In reply to: 180249518 [](ancestors = 180249518)

if (!reachableLabels.Contains(node.WhenTrueLabel))
{
SetState(this.StateWhenFalse);
SetConditionalState(UnreachableState(), this.State);
Copy link
Member

@agocke agocke Apr 9, 2018

Choose a reason for hiding this comment

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

If SetConditionalState zeroes out the current state, what's the point of calling SetState before this? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

The call to UnreachableState() requires as a precondition that the current state is not a split state.


In reply to: 180252872 [](ancestors = 180252872)

else if (!reachableLabels.Contains(node.WhenFalseLabel))
{
SetState(this.StateWhenTrue);
SetConditionalState(this.State, UnreachableState());
Copy link
Member

@agocke agocke Apr 9, 2018

Choose a reason for hiding this comment

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

Same here. #Resolved

@@ -212,9 +212,6 @@ public static void Main()
// (10,13): error CS8117: Invalid operand for pattern match; value required, but found '<null>'.
// if (null is dynamic t) { } // null not allowed
Copy link
Member

@agocke agocke Apr 9, 2018

Choose a reason for hiding this comment

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

Looks like this text is out of date. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in a new iteration.


In reply to: 180253333 [](ancestors = 180253333)

Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

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

LGTM

{
diagnostics.Add(ErrorCode.WRN_GivenExpressionNeverMatchesPattern, node.Location);
}
else if (!decisionDag.ReachableLabels.Contains(whenFalseLabel) && pattern.Kind == BoundKind.ConstantPattern)
Copy link
Member

@cston cston Apr 10, 2018

Choose a reason for hiding this comment

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

&& pattern.Kind == BoundKind.ConstantPattern [](start = 83, length = 44)

When is the pattern not a ConstantPattern? #Resolved

Copy link
Member Author

@gafter gafter Apr 11, 2018

Choose a reason for hiding this comment

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

        const string s = null;
        System.Console.WriteLine(s is string { Length: 3 });

#Resolved

@gafter gafter merged commit 63f9fa1 into dotnet:features/recursive-patterns Apr 11, 2018
@gafter gafter removed 4 - In Review A fix for the issue is submitted for review. labels Dec 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants