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

Open LDM Issues in Pattern-Matching (v2) #2095

Closed
gafter opened this issue Dec 20, 2018 · 34 comments
Closed

Open LDM Issues in Pattern-Matching (v2) #2095

gafter opened this issue Dec 20, 2018 · 34 comments
Assignees
Milestone

Comments

@gafter
Copy link
Member

gafter commented Dec 20, 2018

Open LDM Issues in Pattern-Matching

This is a summary of the remaining open issues for pattern-matching in C# 8.


Reserve and and or in patterns (open)

In anticipation of possibly permitting and and or as pattern combinators in the future, we should forbid (or at least warn) when these identifiers are used as the designator in a declaration or recursive pattern.

See also dotnet/roslyn#33425

Resolution: We did not do this in C# 8.0.


Propose to change precedence of switch expression to primary (open)

The switch expression is currently at relational precedence. I propose to change it to primary precedence. See https://github.com/dotnet/csharplang/issues/2331 for details.

Resolution: It gets a new precedence just looser than primary.


Single-element positional deconstruction (open)

The current LDM decision on single-element positional deconstruction pattern is that the type is required.

This is particularly inconvenient for tuple types and other generic types, and perhaps impossible for values deconstructed by a dynamic check via ITuple.

We should consider if other disambiguation should be permitted (e.g. the presence of a designation).

if (o is (3) _)

Resolution 2018-10-10: Discussed but not resolved.


Disambiguating deconstruction using parameter names (open)

We could permit more than one Deconstruct method if we permit disambiguating by parameter names.

class Point
{
    public void Deconstruct(double Angle, double Length) => ...;
    public void Deconstruct(int X, int Y) => ...;
}

Point p = ...;
if (p is (X: 3, Y: 4)) ...;

Should we do this?

Resolution: No, not at this time.


var pattern for 0 and 1 elements (open)

The var pattern currently requires 2 or more elements because it inherits the grammar for variable_designation. However, both 0-element and 1-element var patterns would make sense and are syntactically unambiguous.

public class C {
    public void Deconstruct() => throw null;
    public void Deconstruct(out int i) => throw null;
    public void Deconstruct(out int i, out int j) => throw null;
    void M() {
        if (this is var ()) { }          // error
        if (this is var (x1)) { }        // error
        if (this is var (x2, y2)) { }    // ok
    }
}

I propose we relax the grammar to permit 0-element and 1-element var patterns.

We could do the same for deconstruction.

Resolution: Approved for patterns. Left to future consideration for deconstruction.


Kind of member in a property pattern (open)

A property pattern { PropName : Pattern } is used to check if the value of a property or field matches the given pattern.

Besides readable properties and fields, what other kinds of members should be permitted here?

  • An indexed property? Possible indexed with a constant?
  • An event reference?
  • Anything else?

Resolution: None of these at this time.


Restricted types (open)

A recent bug (dotnet/roslyn#27803) revealed how pattern matching interacts with restricted types in interesting ways (when doing type checks).
We should discuss with LDM on whether this should be banned (like pointer types), and if there are actual use cases that need it.

Resolution: Pointers actually work, e.g. p is null or p is var x. Restricted types work too. No action.


Matching using ITuple (open)

I assume that we only intend to permit matching using ITuple when the type is omitted?

IComparable x = ...;
if (x is ITuple(3, 4)) // (1) permitted?
if (x is object(3, 4)) // (2) permitted?
if (x is SomeTypeThatImplementsITuple(3, 4)) // (3) permitted?

Resolution: Confirmed.


Matching using ITuple in the presence of an extension Deconstruct (needs proposal)

When an extension Deconstruct method is present and a type also implements the ITuple interface, it is not clear which should take priority. I believe the current LDM position (probably not intentional) is that the extension method is used for the deconstruction declaration or deconstruction assignment, and ITuple is used for pattern-matching. We probably want to reconcile these to be the same.

public class C : ITuple
{
    int ITuple.Length => 3;
    object ITuple.this[int i] => i + 3;
    public static void Main()
    {
        var t = new C();
        Console.WriteLine(t is (3, 4)); // true? or a compile-time error?
        Console.WriteLine(t is (3, 4, 5)); // true? or a compile-time error?
    }
}
static class Extensions
{
    public static void Deconstruct(this C c, out int X, out int Y) => (X, Y) = (3, 4);
}

Discussion 2018-10-24: No conclusion. We need a proposal to consider.

Resolution: The current LDM position is confirmed.


Switch expression as a statement expression (closed)

It has been requested that we permit a switch expression to be used as a statement expression. In order to make this work, we'd

  1. Permit its type to be void.
  2. Permit a switch expression to be used as a statement expression if all of the switch arm expressions are statement expressions.
  3. Adjust type inference so that void is more specific than any other type, so it can be inferred as the result type.

Resolution 2018-10-10: Yes! This requires a more precise spec, and it needs to handle a?.M() where M() returns an uncontrained T. Excluded from C# 8.0.


ITuple vs unconstrained type parameter (closed)

Our current rules require an error in the following, and it will not attempt to match using ITuple. Is that what we want?

    public static void M<T>(T t)
    {
        Console.WriteLine(t is (3, 4)); // error: no Deconstruct
    }

Resolution 2018-10-24: Lets keep it an error for now but be open to relaxing it in the future.


Permit a trailing comma in a switch expression (closed)

See #2098

Resolution 2019-01-09: Yes.


Rename deconstruct_pattern to positional_pattern (closed)

Resolution 2019-01-09: Yes.


Disambiguate declaration patterns with (non-nullable) arrays of nullable type (closed)

The introduction of nullable reference types introduced a breaking change in the parsing of array types. It is recommended that we reconsider the order in which nullable annotations are considered in multidimensional arrays to resolve the ambiguity and permit a compatible implementation.

See dotnet/roslyn#32141

Partial Resolution per LDM 2019-01-07: Keep the order of array designators. But a new syntax resolution rule: following an is, as, or new in an expression, an array type may not end with a ? unless the ? is followed by a token that cannot start an expression. The ? token (if not considered to be part of the array specifier) will then naturally be considered to be the conditional operator, which restores compatibility with existing code.

Resolution 2019-01-09: Reverted. We are going with second option in dotnet/roslyn#32141 (comment). See also the "No way to match a multidimensional array with an inner nullable" issue below.


Should SwitchExpressionException capture an unreified tuple input? (closed)

See dotnet/roslyn#31482

Resolution 2019-01-09: Yes.


No way to match a multidimensional array with an inner nullable (closed)

Another small issue with our resolution to See dotnet/roslyn#32141. How do I write a pattern-matching operation where the type is “an array of a nullable array”? That would (un)naturally be written

if (o is string[][]? s)

but we decided that, since there is an identifier after the ?, the ? designates the start of a ternary operator and this code is a syntax error. This is something I think people are likely to want to do (since an array typically may contain nulls), so it would be a shame to disallow it.

Resolution 2019-01-09: This (and its extension to a problem in recursive patterns) was the straw that broke the camel's back, and caused us to revisit the interaction of the nullable annotation and array dimension specifiers. The new resolution is that the ? annotation is a type constructor that can be applied to array types, and an array type does not have any ? among its array specifiers. This is the second option in dotnet/roslyn#32141 (comment).

@alrz
Copy link
Member

alrz commented Dec 20, 2018

if (o is (3) _)

Reiterating my comment on that,

I think the disambiguation is fine as long as we don't have parenthesized patterns. But then, removing the discard would completely change the pattern semantics. Consider another example:

if (o is (var x) _)      // deconstruct and bind anything from deconstruction
if (o is (var x))        // matches everything; equivalent to `o is var x`

I expect this'll be confusing because the designation is essentially optional in recursive patterns.

@jnm2
Copy link
Contributor

jnm2 commented Dec 20, 2018

I think the disambiguation is fine as long as we don't have parenthesized patterns.

Without parenthesized patterns, not, and, and or patterns are going to be less useful.

@gafter
Copy link
Member Author

gafter commented Dec 21, 2018

@alrz Another reason to disallow that is we can't distinguish between (int.MaxValue) x (a recursive/tuple pattern) and (System.Int32)Int32.MaxValue (a constant pattern).

@alrz
Copy link
Member

alrz commented Dec 21, 2018

Right, I think the cast ambiguity has been resolved by LDM before (#1054).

The proposed workaround was to use an empty property pattern clause: (var x) {}

@qrli
Copy link

qrli commented Dec 21, 2018

I believe the current LDM position (probably not intentional) is that the extension method is used for the deconstruction declaration or deconstruction assignment

The good side is that extension methods can be opt-in by using namespaces, so we can still choose which one to use.
Though, adding a using may change the meaning of the code silently, which is a bit risky. And it is different behavior than extension method with same name as instance method, where instance method wins.

@gafter
Copy link
Member Author

gafter commented Dec 21, 2018

Right, I think the cast ambiguity has been resolved by LDM before

Yes, but that resolution no longer "works" with relaxed rules for when a single-element positional pattern is permitted.

@alrz
Copy link
Member

alrz commented Dec 21, 2018

Sure but (var x) {} could work if you don't want to spell out the type (or can't, in case it's unspeakable).

@gafter
Copy link
Member Author

gafter commented Dec 28, 2018

Added

Rename deconstruct_pattern to positional_pattern

/cc @jcouv

@gafter
Copy link
Member Author

gafter commented Jan 3, 2019

Added

Disambiguate declaration patterns with (non-nullable) arrays of nullable type

Due to the strange order of nullable annotations on arrays, we will need some disambiguation rule to resolve the parsing of declaration expressions vs the ?: operator:

if (o is A[]   ? b
                   : c)  // OK, this means `(o is A[]) ? b : c` because nullable types are not permitted for a pattern's type

if (o is A[]   ? b
                   && c) // error: 'cannot use nullable reference type for a pattern' or 'expected :'

if (o is A[][] ? b
                   : c)  // OK, this means `(o is A[][]) ? b : c`

if (o is A[][] ? b
                   && c) // OK, this means `(o is A[][]? b) && c` because `A[][]?` is not a nullable type

See also dotnet/roslyn#31991 and dotnet/roslyn#32025

@gafter
Copy link
Member Author

gafter commented Jan 4, 2019

Changed the last item to

Disambiguate declaration patterns with (non-nullable) arrays of nullable type

The introduction of nullable reference types introduced a breaking change in the parsing of array types. It is recommended that we reconsider the order in which nullable annotations are considered in multidimensional arrays to resolve the ambiguity and permit a compatible implementation.

See dotnet/roslyn#32141

@gafter
Copy link
Member Author

gafter commented Jan 8, 2019

Added

Should SwitchExpressionException capture an unreified tuple input?

See #31482

@gafter
Copy link
Member Author

gafter commented Jan 8, 2019

Added a resolution and a new issue:

Disambiguate declaration patterns with (non-nullable) arrays of nullable type

The introduction of nullable reference types introduced a breaking change in the parsing of array types. It is recommended that we reconsider the order in which nullable annotations are considered in multidimensional arrays to resolve the ambiguity and permit a compatible implementation.

See dotnet/roslyn#32141

Partial Resolution per LDM 2019-01-07: Keep the order of array designators. But a new syntax resolution rule: following an is, as, or new in an expression, an array type may not end with a ? unless the ? is followed by a token that cannot start an expression. The ? token (if not considered to be part of the array specifier) will then naturally be considered to be the conditional operator, which restores compatibility with existing code.

No way to match a multidimensional array with an inner nullable

Another small issue with our resolution to See dotnet/roslyn#32141. How do I write a pattern-matching operation where the type is “an array of a nullable array”? That would (un)naturally be written

if (o is string[][]? s)

but we decided that, since there is an identifier after the ?, the ? designates the start of a ternary operator and this code is a syntax error. This is something I think people are likely to want to do (since an array typically may contain nulls), so it would be a shame to disallow it.

@gafter
Copy link
Member Author

gafter commented Jan 8, 2019

Changed MatchFailureException to SwitchExpressionException per https://github.com/dotnet/corefx/issues/33284

@yaakov-h
Copy link
Member

yaakov-h commented Jan 9, 2019

SwitchExpressionException

Say that ten times fast...

@alrz
Copy link
Member

alrz commented Jan 9, 2019

No way to match a multidimensional array with an inner nullable

Once we have parenthesized patterns, it could be written as o is (string[][]? s). that wouldn't be ambiguous because there's no ternary-like syntax in patterns - this could be suggested in error message.

@jcouv
Copy link
Member

jcouv commented Jan 9, 2019

@alrz Placing parens like o is (string[][]? s) seems wrong. Is this a 1-positional pattern?
Probably needs to be (o is string[][]? s).

@gafter
Copy link
Member Author

gafter commented Jan 9, 2019

that wouldn't be ambiguous because there's no ternary-like syntax in patterns

There is a ternary-like syntax in patterns, because an expression is a pattern (to support constant patterns).

@alrz
Copy link
Member

alrz commented Jan 9, 2019

@jcouv parens around the whole expression probably wouldn't help because a ternary is still possible further ahead (parens around the pattern would be a new feature).

@gafter right, but I think in the presence of the array type, the parser doesn't expect any ternary at this point.

@gafter
Copy link
Member Author

gafter commented Jan 9, 2019

I've marked a number of issues resolved based on today's LDM. Search for Resolution 2019-01-09.

In particular we've changed the interaction of array dimension specifiers and the nullable type syntax. The new resolution is that the ? annotation is a type constructor that can be applied to array types, and an array type does not have any ? among its array specifiers. This is the second option in dotnet/roslyn#32141 (comment).

@gafter
Copy link
Member Author

gafter commented Jan 9, 2019

@gafter Neal Gafter FTE right, but I think in the presence of the array type, the parser doesn't expect any ternary at this point.

This is one ambiguity:

    if (e is int[][]? b // ternary or array of nullable array?
                        : c) 
    if (e is int[][]? b // ternary or array of nullable array?
                        && c) 

it gets worse with recursive patterns

    if (e is int[][]? (3, 4) // ternary or array of nullable array in a recursive pattern?
                        : c) 
    if (e is int[][]? (3, 4) // ternary or array of nullable array in a recursive pattern?
                        && c) 

Note that this could be semantically legal if you write an extension Deconstruct.

With the resolution from today's LDM, the nullable annotation for the inner array goes between the ] and the [.

    if (e is int[][]? b // obviously ternary
                        : c)
    if (e is int[]?[] b // obviously declaration pattern
                        && c)
    if (e is int[][]? (3, 4) // obviously ternary
                        : c) 
    if (e is int[]?[] (3, 4) // obviously recursive pattern
                        && c) 

@alrz
Copy link
Member

alrz commented Jan 9, 2019

This is one ambiguity:

I was saying that in a hypothetical parenthesized pattern, you can't have a ternary after ?.

if (e is (int[][]? x))

The possible token after x could be only a comma for positional patterns, either way, this is definitely a declaration pattern with type int[][]?.

@ufcpp
Copy link

ufcpp commented Jan 24, 2019

Can Deconstruct calls be eliminated if all of the sub-patterns are discard?

using System;

struct X
{
    // Deconstruct methods with side effects
    public void Deconstruct() => Console.WriteLine("side effect in ()");
    public void Deconstruct(out int x)
    {
        Console.WriteLine("side effect in (int)");
        x = 0;
    }

    public void Deconstruct(out int x, out int y)
    {
        Console.WriteLine("side effect in (int, int)");
        (x, y) = (0, 1);
    }
}

class Program
{
    static int Zero() => new X() switch { () => 0 };
    static int One() => new X() switch { var (_) => 0 };
    static int Two() => new X() switch { (_, _) => 0 };

    static int Zero(object x) => x switch { X() => 0, _ => 1 };
    static int One(object x) => x switch { X(_) => 0, _ => 1 };
    static int Two(object x) => x switch { X(_, _) => 0, _ => 1 };

    static void Main()
    {
        // There is no Deconstruct called, no side effect in VS 2019 Preview 2
        Console.WriteLine(Zero());
        Console.WriteLine(One());
        Console.WriteLine(Two());
        Console.WriteLine(Zero(new X()));
        Console.WriteLine(One(new X()));
        Console.WriteLine(Two(new X()));
    }
}

@gafter
Copy link
Member Author

gafter commented Jan 25, 2019

Can Deconstruct calls be eliminated if all of the sub-patterns are discard?

According to the specification, yes. See https://github.com/dotnet/csharplang/blob/master/proposals/patterns.md#order-of-evaluation-in-pattern-matching which says in part

The compiler should not invoke functions that cannot affect the result...

Note "should" is not "must". The compiler is permitted but discouraged from invoking Deconstruct.

@gafter
Copy link
Member Author

gafter commented Feb 15, 2019

Added


Reserve and and or in patterns (open)

In anticipation of possibly permitting and and or as pattern combinators in the future, we should forbid (or at least warn) when these identifiers are used as the designator in a declaration or recursive pattern.

See also dotnet/roslyn#33425

@ghost
Copy link

ghost commented Feb 26, 2019

I posted this in #2268
While I am reading this example in Docs:

static string Quadrant(Point p) => p switch
{
    (0, 0) => "origin",
    (var x, var y) when x > 0 && y > 0 => "Quadrant 1",
    (var x, var y) when x < 0 && y > 0 => "Quadrant 2",
    (var x, var y) when x < 0 && y < 0 => "Quadrant 3",
    (var x, var y) when x > 0 && y < 0 => "Quadrant 4",
    (var x, var y) => "on a border",
    _ => "unknown"
};

I wondered if it can be shorter and more readable to achive the purpose of pattern matching, so I suggest this:

static string Quadrant(Point p) => p switch
{
    (0, 0) => "origin",
    (> 0, > 0) => "Quadrant 1",
    (< 0, > 0) => "Quadrant 2",
    (< 0, < 0) => "Quadrant 3",
    (> 0, < 0) => "Quadrant 4",
    (_, _) => "on a border",
    _ => "unknown"
};

@gafter
Copy link
Member Author

gafter commented Feb 27, 2019

@MohammadHamdyGhanem That idea is already championed by me at #2115 .

@Thaina
Copy link

Thaina commented Feb 27, 2019

class Point
{
    public void Deconstruct(double Angle, double Length) => ...;
    public void Deconstruct(int X, int Y) => ...;
}

Point p = ...;
if (p is (Y: 3, X: 4)) ...; // What will happen?
if (p is (X: 3.0, Y: 4.0)) ...; // What will happen?
if (p is (Angle: 3, Length: 4)) ...; // What will happen?

@gafter
Copy link
Member Author

gafter commented Feb 27, 2019

@Thaina We don't disambiguate or reorder based on parameter names. All three are ambiguous. As a practical matter, you should have only one Deconstruct method for a given arity. If you do give names, they are required to match the name in their positions.

@Thaina
Copy link

Thaina commented Feb 27, 2019

@gafter That what I ask What will happen? should it be warning, error or something?

@gafter
Copy link
Member Author

gafter commented Feb 27, 2019

All three are ambiguity errors at compile-time.

@Thaina
Copy link

Thaina commented Feb 27, 2019

@gafter Also this

if (p is (Angle: 3, Length: 4)) ...;

is ambiguous but should it did implicit conversion?

Or is it not allow implicit conversion? What about this case?

if (p is (Angle: 3f, Length: 4f)) ...;

Also if we encourage that we would have many Deconstruct functions. What would happen to the assignment deconstruction? Do we already discuss about that somewhere?

@gafter
Copy link
Member Author

gafter commented Feb 28, 2019

This is exactly the same behavior as deconstruction. If you have two deconstruct methods of the needed arity, it is just an error.

@gafter
Copy link
Member Author

gafter commented Mar 11, 2019

Added

Propose to change precedence of switch expression to primary (open)

The switch expression is currently at relational precedence. I propose to change it to primary precedence. See https://github.com/dotnet/csharplang/issues/2331 for details.

@gafter
Copy link
Member Author

gafter commented Oct 2, 2019

All of these open issues are addressed. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants