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

Champion "User-defined Positional Patterns" #1047

Open
5 tasks
gafter opened this issue Oct 25, 2017 · 58 comments
Open
5 tasks

Champion "User-defined Positional Patterns" #1047

gafter opened this issue Oct 25, 2017 · 58 comments
Assignees
Milestone

Comments

@gafter
Copy link
Member

gafter commented Oct 25, 2017

  • Proposal added
  • Discussed in LDM
  • Decision in LDM
  • Finalized (done, rejected, inactive)
  • Spec'ed

This championed proposal is for that part of the pattern-matching specification that permits a user-defined positional pattern. As specified, it is a user-defined operator is. Given the current shape of the language post tuples, an alternative might be a bool-returning static Deconstruct method.

@gafter gafter self-assigned this Oct 25, 2017
@orthoxerox
Copy link

This will be great for struct-based Option<T> and friends.

@alrz
Copy link
Member

alrz commented Oct 25, 2017

to be clear, this is not about "extension patterns", right? or this would be the only way to have a user-defined pattern?

@gafter
Copy link
Member Author

gafter commented Oct 25, 2017

to be clear, this is not about "extension patterns", right?

I don't care what they are called.

or this would be the only way to have a user-defined pattern?

This proposal is not meant to exclude the possibility that there would be other things done to the language too.

@orthoxerox
Copy link

orthoxerox commented Oct 26, 2017

@alrz Since the proposal shows additional is deconstructors being declared in a separate static class, the patterns look extension enough to me.

@DavidArno
Copy link

@orthoxerox,

Not quite extension enough for me. Active patterns (which I think is what I think @alrz is referring to with "extension patterns") would complete the set. But this feature would certainly be a huge step toward that goal.

@orthoxerox
Copy link

@DavidArno it looks like only the input parameters are really missing from the proposal. You can write str is Integer() with a positional pattern, or even str is Integer(var i).

@DavidArno
Copy link

@orthoxerox,

Input parameters and an active pattern name, eg:

public static bool ValidPostcodePattern(this string str) { ...

if (postcode is ValidPostcode) ...

@orthoxerox
Copy link

orthoxerox commented Oct 26, 2017

@DavidArno

public static class ValidPostcode
{
    public static bool operator is(string str) => ...
}

if (postcode is ValidPostcode()) ...

@DavidArno
Copy link

@orthoxerox,

Oh! That's a really neat solution. In that case, you're right: input parameters are indeed the only part that's missing.

@DavidArno
Copy link

DavidArno commented Oct 26, 2017

Unfortunately, I can't find where I originally suggested it, but as a reminder to @gafter, input parameters could be handled by using in. A highly contrived example:

public static class ValidAndFormattedPostcode
{
    public static bool operator is(string str, in countryCode, out formattedPostcode) => ...
}

var postcode = "aa11Aa";
if (postcode is ValidAndFormattedPostcode("gb", var formattedPostcode)) 
{
    // formattedPostcode assigned "AA1 1AA"

@alrz
Copy link
Member

alrz commented Oct 26, 2017

@DavidArno If anything, you should be using in at the call-site - because that's where it's ambigious, although I believe we don't need it if we define pattern syntax as a subset of expression syntax (#277).


Anyways, since this is about plugging fallible positional patterns to types, for each case we'll have to declare a full-blown class,

static class Between {  public static bool Deconstruct(...) {} }
static class Integer {  public static bool Deconstruct(...) {} }
static class MyPattern {  public static bool Deconstruct(...) {} }

while it could be just one type,

static class Patterns { 
    [PatternExtension] public static bool Between(...) {}
    [PatternExtension] public static bool Integer(...) {}
    [PatternExtension] public static bool MyPattern(...) {}
}

In fact, the example given in the proposal doesn't need to be a type either, that is, the Polar class has no other usages besides being a mere container for is operator . So I think in the presence of active patterns, we don't need bool-returning Deconstruct methods. Records can use a void-returning Deconstruct method to enable positional patterns because they wouldn't be fallible.

EDIT: there is actually a use case for bool-returning Deconstruct methods: when you want to define a "fallible conversion" between types.

@Thaina
Copy link

Thaina commented Oct 27, 2017

I think this will be useful but I feel it not good to have is keyword that should be a fast type check mechanism do any work like implicit conversion operator

@DavidArno
Copy link

DavidArno commented Oct 27, 2017

@Thaina,

What keyword would you use? Also bear in mind that we talking patterns here, so, if for example the following were valid:

if (postcode is ValidAndFormattedPostcode("gb", var formattedPostcode)) ...

Then it would work for the following situations too:

let ValidAndFormattedPostcode("gb", var formattedPostcode) = postcode else throw...

switch (postcode)
{
    case ValidAndFormattedPostcode("gb", var formattedPostcode): ...

@HaloFour
Copy link
Contributor

Are is operators still in play? I thought that idea went out when the Deconstruct convention was added to the language:

public static class Polar
{
    // public static bool operator is(Cartesian c, out double R, out double Theta)
    public static bool Deconstruct(this Cartesian c, out double R, out double Theta)
    {
        R = Math.Sqrt(c.X*c.X + c.Y*c.Y);
        Theta = Math.Atan2(c.Y, c.X);
        return c.X != 0 || c.Y != 0;
    }
}

I don't really care that much as to what syntax is eventually adopted, but I do hope that it would allow creating named patterns without having to define (or modify) types specifically for that purpose as was the case with the above Polar example, especially since the example implies that the type must be static. That seems to preclude conversion patterns between types, e.g. if (x is string(var s)) { ... }

The ability to support input expressions would be really nice as well to allow for utility patterns like if (x is between(1, 5)) but I understand the parser ambiguities that creates.

There's also the question of where ADTs stand which probably builds off of both records and "user-defined positional patterns".

@Thaina
Copy link

Thaina commented Oct 28, 2017

@DavidArno Well, I don't know. Maybe a new keyword?

@HaloFour It not about Deconstruct function but the fact it utilize is and case keyword that normally be a fast checking to add a conversion that could be complex and might expensive. That destroy the assumption about usage of is. I wish we could make something more difference

Maybe

if(obj is MyType m) // obj is MyType
{
}
else if(obj is ~MyType m) // obj is deconstructable to MyType
{
}

I too don't care much about syntax. I just don't like it that it is the same as already used syntax

Another problem arise is, with obj is var x keyword I think we would x being the same reference instance of obj but with the deconstruct it could return difference object?

@gafter
Copy link
Member Author

gafter commented Oct 29, 2017

@Thaina http://gafter.blogspot.com/2017/06/making-new-language-features-stand-out.html

People will not be stuck with C# 7 and earlier in their brains forever. They will learn the new features and integrate them into their understanding of the language. In any case, something like obj is MyType m will never be a deconstruction and will always be no more than a type test (assigning the value to a variable of the given type). We have not considered hijacking that syntax to give it a new meaning.

@Richiban
Copy link

Richiban commented Nov 6, 2017

I would like to make sure that we are considering whether this feature would enable / prohibit F#-style active patterns in the future.

@orthoxerox 's solution o

public static class ValidPostcode
{
    public static bool operator is(string str) => ...
}

if (postcode is ValidPostcode()) ...

Works well as a C# equivalent of this partial active pattern in F#:

let (|ValidPostcode|_|) s = ...

Where a pattern can succeed or fail. I can't, however, see how this mechanism can be used to implement the full active pattern, such as this:

let (|GreaterThan|Equal|LessThan|) (x, y) = ...

@alrz
Copy link
Member

alrz commented Nov 9, 2017

From the proposal, so this would be possible?

partial class BlockSyntax {
  public static bool Deconstruct(SyntaxNode node) => node.IsKind(SyntaxKind.Block);
}

if (node is BlockSyntax()) {}
if (BlockSyntax.Deconstruct(node)) {}

However, it's not specified that what happens if you add a pattern variable there, e.g.

if (node is BlockSyntax() block) {}

--

Also I noticed that the example given in the proposal conflates "polar deconstruction" and "cartesian to polar conversion". Assuming that the two are defined as records, deconstruction is compiler-generated as void-returning Deconstruct methods, so the only thing that is left is a fallible conversion, e.g.

struct Cartesian(int X, int Y) {
  // fallible conversion
  public static explicit operator Polar?(Cartesian c) => c.X == 0 || c.Y == 0 ? null
    : new Polar(Math.Sqrt(c.X*c.X + c.Y*c.Y), Math.Atan2(c.Y, c.X));
}

struct Polar(double R, double Theta) {
 // compiler-generated
 // public void Deconstruct(out double R, out double Theta) => (R, Theta) = (this.R, this.Theta);
}

if (cartesian is Polar(var R, _))

Not sure if is operators consider conversions when they are matching against different types.

Note: T? accounts for an incomplete pattern in F# e.g. let (A|_) = ...

--

@Richiban

First of all, I can't imagine why on earth you would want to write something like this:

public static class ValidPostcode
{
   public static bool operator is(string str) => ...
}

if (postcode is ValidPostcode()) ...

Sure, in F# it might makes sense to define an active pattern for that, but the idiomatic way of doing that in C# is a TryParse method. e.g. public static bool TryParse(string, out Postcode). You might just want to use in a switch case, though I don't think it worth the overhead (see my comment above).

how this mechanism can be used to implement the full active pattern

Active patterns in F# use Core.Choice<..> ADTs under the hood, so no new type is emitted for the output. If is operators consider conversion operators (like the example above) I imagine you can define a discriminated union and declare a conversion operator to that type. However, your specific example needs the pattern to be "parametrized" (check out F# doc on active patterns to see what I mean). In that case, there will be some ambiguities when we're parsing expressions vs. patterns ( #277).

@HaloFour
Copy link
Contributor

HaloFour commented Nov 9, 2017

@alrz

Can't call a TryParse method in the middle of a recursive pattern. Should, you could use a variable pattern and a guard in that case but there's something nice about keeping everything in one place.

@alrz
Copy link
Member

alrz commented Nov 9, 2017

@HaloFour

I don't disagree with the use case. but I believe what makes it possible - according to the current proposal- which is defining a whole type with a single member, is just too much and unnecessarily convolutes the code.

@HaloFour
Copy link
Contributor

HaloFour commented Nov 9, 2017

@alrz

That I agree with. dotnet/roslyn#9005

Especially since in the current proposal that type is static. It makes little sense that anything could be that type since that type isn't actually a value.

@gafter
Copy link
Member Author

gafter commented Nov 9, 2017

@alrz

Also I noticed that the example given in the proposal conflates "polar deconstruction" and "cartesian to polar conversion". Assuming that the two are defined as records...

Polar is not defined as a record or anything like it. It is a static class; there is no "conversion" to conflate with deconstruction.

@alrz
Copy link
Member

alrz commented Nov 9, 2017

@gafter

I understand that it's just an example, but that itself raises the issue. If it is solely to represent a custom pattern (and nothing else), I'd argue that a full-blown type for it would be overkill. If it's not, e.g. Polar is itself a meaningful type or record, it already has a compiler-provided Deconstruct method (or a manually written one, for that matter), and you just need to add the conversion logic.

@gafter
Copy link
Member Author

gafter commented Nov 9, 2017

@alrz I expect such a static class would also contain a pseudo-constructor - for example, in the type Polar to create an actual object of type Cartesian, and other utilities for working with a point conceptually as a Polar, even though it is physically a Cartesian.

@alrz
Copy link
Member

alrz commented Nov 9, 2017

IMO this example is somehow biased on how you would want to handle Polar/Cartesian systems. You have a Cartesian class and only provide Polar utilities in a static class. One might define both as classes and define conversions in either directions.

Let's take a more general example, like a Between pattern. (of course, in the absence of range literals).

static class Between {
  public static bool Deconstruct(this int i, int from, int to) { ... }
}

There is a couple of things to point out here:

  • Between has no meaning as a "type" or even a utility static type, because it's a pattern.
  • I can't imagine that this class could contain any other meaningful members in it because it's a pattern.
  • You don't Deconstruct a Between, you match a value against the Between pattern which is not clearly expressed here.
  • And, of course, we can't accept non-out parameters here while it's crucial for custom patterns to be useful.

I am still trying to think of an actual use for a bool-returning Deconstruct which does not suffer from these issues.

@DavidArno
Copy link

DavidArno commented Nov 10, 2017

@alrz,

I am still trying to think of an actual use for a bool-returning Deconstruct which does not suffer from these issues.

The obvious one for me is with option/maybe types:

struct Option<T> { ... }

static class Some 
{
    public static bool Deconstruct<T>(this Option<T> option, out T value)
    {
        if (option.HasValue) 
        {
            value = option.Value;
            return true;
        }
        value = default;
        return false;
    }
}

static class None 
{
    public static bool Deconstruct<T>(this Option<T> option) => !option.HasValue;
}

var optionalValue = F();
if (optionalValue is Some(var value))
{
    // use value here

Caveat: I've had to introduce Deconstruct<T> here, which I don't think is (yet) part of the scope of the feature.

This is the only use case I can think of, but it's an incredibly compelling use case for me. Not just because it offers a neat solution to using patterns to test and extract the value, but because it simplifies union types in general. For example (totally making up syntax as I go):

struct ColorSpaces is RGB(int red, int green, int blue) | 
                      HSV(int hue, int saturation, int lightness) {}

Could be lowered to:

struct ColorSpaces
{
    public (int red, int green, int blue) Rgb { get; }
    public (int hue, int saturation, int lightness) Hsv { get; }
    public bool IsHsv { get; }  // make the first type in the union the default;

    public static RGB(int red, int green, int blue)
    {
        Rgb = (red, green, blue);
        IsHsv = false;
    }

    public static HSV(int hue, int saturation, int lightness)
    {
        Hsv = (hue, saturation, lightness);
        IsHsv = true;
    }

    ...
}

static class RGB 
{
    public static bool Deconstruct(this ColorSpaces colorSpaces, 
                                   out int red, 
                                   out int green, 
                                   out int blue)
    {
        if (!colorSpaces.IsHsv)
        {
            (red, green, blue) = colorSpaces.Rgb;
            return true;
        }
        (red, green, blue) = default;
        return false;
    }
}

public static class HSV { ...

var colorSpace = GetAColorSpace();
switch (colorSpace)
{
    case RGB(var red, _, _)):
        Console.WriteLine($"RGB. Red is {red}";
        break;
    case HSV(var h, var s, var v):
        ...
}

@DavidArno
Copy link

@alrz,

There is a couple of things to point out here:

I completely agree that the proposal here does not work well for active patterns. But as @gafter said two weeks ago, this proposal does not exclude the idea of adding active patterns later.

What this proposal offers is a much needed way of expanding upon the currently very limited scope if x is T y to support simpler syntax for eg unions. It's not the whole story, but it's a big step in the right direction. And for what it offers, using types to provide those patterns makes complete sense to me.

@alrz
Copy link
Member

alrz commented Nov 18, 2017

The smelly part is this:

static class Some {}
static class None {}
static class RGB {}
static class HSV {}

I think that's too much boilerplate. Those types have no meaning beyond being a sole container for the Deconstruct method and there is absolutely no correspondence between these types/patterns, like

static class OptionExtensions {
  public static bool Some<T>(this Option<T> @this, out T value) {}
  public static bool None<T>(this Option<T> @this) {}
}

Which still has a downside: the compiler does not know if Some and None cover all the possible cases - there will be a false negative/positive exhaustiveness warnings depending on whether or not we want match to produce such warnings and you might still need to have a catch-all case.

Ideally, it should be defined as an ADT so that the compiler have complete info re exhaustiveness. In my opinion, your example is not compelling at all. I won't suggest we try to encode such structures with "user-defined positional patterns" or "custom patterns" etc. For comparison, discriminated unions and active patterns in F# are totally different features. Actually, F# active patterns are built on top of DUs, not the other way around.

@alrz
Copy link
Member

alrz commented Nov 20, 2018

Copying my comment over this relevant issue, (for an approach to user-defined positional patterns).

public static bool BitPatternMatch(this int value. int mask) => (value & mask) == mask;

if (value is BitPatternMatch(0xb_1001))

In a recursive pattern like that, when the type lookup for BitPatternMatch fails, we'll look for a method or an extension method on the target value, e.g. value.BitPatternMatch(0xb_1001) (perhaps with some requirement like an attribute on the method or a name convention to prevent accidental binding).

Each argument can be a:

  • (1) constant pattern,
    • (1a) If the corresponding parameter is not out/ref we'll pass the constant value as the argument.
    • (1b) If the corresponding parameter is out we'll match it against the result.
    • (1c) otherwise, an error is produced.
  • or (2) some other pattern.
    • (2a) If the corresponding parameter is out we'll match it against the result.
    • (2b) otherwise, an error is produced.

This is mostly the same as how we handle Deconstruct, except for the case (1a).

@yaakov-h
Copy link
Member

Both links in the OP are broken, and I can't find the section that the first link's anchor is supposed to be pointing to in this file.

@gafter
Copy link
Member Author

gafter commented Apr 11, 2019

@yaakov-h What we used to have in those documents no longer makes sense now that positional patterns are defined by a Deconstruct method (rather than an operator is).

We would probably do this for a type

public class Cartesian
{
    public int X { get; }
    public int Y { get; }
    public void Deconstruct(out int X, out int Y) => (X = this.X, Y = this.Y);
}

by permitting the definition of a user-defined pattern

public static class Polar
{
    public static bool Deconstruct(Cartesian value, out double R, out double Theta) => ...
}

Which would allow

    void M(Cartesian cart)
    {
        if (cart is Polar(var r, var theta)) ...
    }
}

@HaloFour
Copy link
Contributor

@gafter

So has none of the above conversation been taken into consideration at all or is it just a matter of these design discussions not happening yet?

I seriously hope that these user-defined positional patterns aren't going to be stuck as static methods of static classes both requiring way more boilerplate than seems reasonable and also makes those static classes useless as types themselves.

@gafter
Copy link
Member Author

gafter commented Apr 11, 2019

So has none of the above conversation been taken into consideration

Not sure if #1047 (comment) answers your question.

@HaloFour
Copy link
Contributor

@gafter

Not sure if #1047 (comment) answers your question.

Patterns are adjectives, so perhaps they should be properties? If they have to be types I hope that those types don't have to be static and can be used as proper types. Afterall, what if you wanted to actually instantiate Polar coordinates? And then I would be very disappointed if "extension everywhere" doesn't enable defining user-defined positional patterns for other types.

I'm sure that the team is going to dig into this further something after 8.0 ships, I hope to see some design notes around it with many more use cases than "Cartesian"/"Polar" which would represent how people can expect to work with them.

@alrz
Copy link
Member

alrz commented Jun 26, 2019

how people can expect to work with them.

One entry point I'd like to see is an "Extract Pattern" code action that acts just like "Extract Method" on patterns - to extract out reusable/long patterns into their own.

if (x  is { P : <long pattern> })

from there, you can expect to be able to parameterize the resulting pattern and so on. I believe this helps to understand the potential of such feature. If anything, it demands the resulting code to be concise and flexible, because we're going to see a lot of these and perhaps new APIs will emerge around it.

@Rekkonnect
Copy link
Contributor

I believe that the operator is should never become overloadable because it would induce quite the trouble with existing code doing null-checking. Currently, it's the only and safest way to check for nullity without using a potentially user-overloaded operator (!=, ==). In such a scenario, most overloaders would need to account for the cases where null values are passed, much like how it already happens.

@jcouv jcouv assigned 333fred and unassigned jcouv and gafter Feb 12, 2022
@Atulin
Copy link

Atulin commented Nov 28, 2023

I'm with the above comment. The addition of is made it so I can confidently check that something is actually null, not whatever the provider of a given framework/library/whatever deemed to be null via == operator overloading. This change would mean we're again at square one.

@DavidArno
Copy link

@Atulin,

I believe your concerns are misplaced here. This proposal won't provide a way of changing what x is null means. The nearest it could get to it would be something like x is null() and such a gnarly construct could be easily blocked via an analyzer rule.

@Atulin
Copy link

Atulin commented Nov 30, 2023

Ah, I assumed it was more like other operators. We don't call an overloaded + with 2 +(2) after all.

If that is the case, though, carry on and don't mind me.

@TahirAhmadov
Copy link

Will this not be subsumed by #5497?

@333fred
Copy link
Member

333fred commented Aug 2, 2024

Possibly, but probably not. That won't cover paramerized patterns, like different string comparison operators.

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