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

Reconcile syntax of "match" expression based on LDM feedback #8818

Closed
gafter opened this issue Feb 17, 2016 · 63 comments
Closed

Reconcile syntax of "match" expression based on LDM feedback #8818

gafter opened this issue Feb 17, 2016 · 63 comments

Comments

@gafter
Copy link
Member

gafter commented Feb 17, 2016

We need the LDM (C# language design meeting attendees) to decide what the syntax of a "match expression" should be, and then we need to implement that.

Are we using switch or match?
default: or case *: or both?
commas between cases?
Curly braces or parens?
Must a match expression be complete? If not, what happens when it isn't?
What about a single-case (irrefutable) match expression?

@gafter
Copy link
Member Author

gafter commented Feb 17, 2016

@MadsTorgersen Can we meet sometime this week to make some tentative calls for the prototype?

@DavidArno
Copy link

Not sure if you really want the opinions on non-LDM members here, but I'll offer them anyway (I assume it can be deleted if this is an unhelpful comment):

Are we using switch or match?

If match can be used in a non-breaking fashion, please use that. It'll make teaching the concept of match expressions to others easier if it has a different name to switch

default: or case *: or both?

Please, please, please don't use case *. This closes off the option for using * as a throw-away variable (the equivalent to F#'s _) in later releases of C#.

@HaloFour
Copy link

Another peanut-gallery opinion, feel free to forward to /dev/null.

I think I might prefer match to switch as it allows avoiding the baggage that may otherwise be inherited by reusing switch, even though the syntax and context would be quite different.

I prefer case * to default for much of the same reason, although if you go with match it probably doesn't matter much. I don't see why it would preclude the possibility of implementing a feature like #8074 in the future considering that the syntactic contexts are different.

Does it make sense for an incomplete match in an expression to result in anything other than an exception? I'm not sure that it does. As such I think that the compiler should try to enforce that the match is complete and silently emit a wildcard pattern that throws unless one is already defined.

@alrz
Copy link
Member

alrz commented Feb 18, 2016

I think I'd prefer switch instead of match as long as they feature similar possibilities. Currently as it is specified, it would be not possible to write multiple cases (and default case) in the switch expression even though you have chosen switch to keep them closer, syntactically. While case * is something that would be expected in a pattern-matching construct, a default case is idiomatic C# and IMO shouldn't be excluded from switch expression. I think it's more of a preference but it doesn't mean that one should be discarded in favor of the other.

@gafter
Copy link
Member Author

gafter commented Feb 18, 2016

The main reason that I resist default: in a match is that we want the match cases to be placed in order. In a switch statement, you can put the default anywhere among the cases, but it always matches last. We want to force it to be last. But I suppose we could just require it to be in the last position only for a match expression.

I think we're likely to change the keyword from switch to the contextual keyword match for the match expression.

@alrz
Copy link
Member

alrz commented Feb 18, 2016

If it's likely to change the keyword to match, I can tell there would be no need for commas and case expressions can be represented by match <pattern> which doesn't need to disallow chaining.

Just a quick question, ordering problem doesn't apply to switch statement already? I mean the following would be useless, because name woudn't be definitely assigned anyway,

switch(...) {
  default:
  case Student(var name):
    break;
}

Why it cannot be applied to match expressions as well?

@DavidArno
Copy link

@gafter,

The main reason that I resist default: in a match is that we want the match cases to be placed in order. In a switch statement, you can put the default anywhere among the cases, but it always matches last. We want to force it to be last. But I suppose we could just require it to be in the last position only for a match expression.

This surely has to apply to a match statement too? If the switch is using the new pattern-matching features, then it's a match statement and thus the order of the cases becomes important and the default must therefore be last. This isn't just an issue for match expressions.

@gafter
Copy link
Member Author

gafter commented Feb 18, 2016

@DavidArno No, we're not going to change the fact that a switch statement treats the default case as a "last fallback" no matter where it appears in the syntax. The addition of a pattern-matching construct somewhere in the switch won't change that.

@alrz

If it's likely to change the keyword to match, I can tell there would be no need for commas and case expressions can be represented by match which doesn't need to disallow chaining.

I have no idea what this means. What syntax are you imagining?

@alrz
Copy link
Member

alrz commented Feb 18, 2016

// as originally proposed
var result = t match(case P1: e1 case P2: e2); // no comma

var result = t match P : e;
// instead of
var result = t case P : e;

case-expression:
shift-expression case pattern : shift-expression

Becomes,

case-expression:
relational-expression match pattern : shift-expression

@DavidArno
Copy link

@gafter,

So, taking the example from xxx:

switch (e) {
    case Mult(Const(0), *): return Const(0);
    case Mult(*, Const(0)): return Const(0);
    case Mult(Const(1), var x): return Simplify(x);
    case Mult(var x, Const(1)): return Simplify(x);
    case Mult(Const(var l), Const(var r)): return Const(l*r);
    case Add(Const(0), var x): return Simplify(x);
    case Add(var x, Const(0)): return Simplify(x);
    case Add(Const(var l), Const(var r)): return Const(l+r);
    case Neg(Const(var k)): return Const(-k);
    default: return e;
  }

I could change that to:

switch (e) {
    case Mult(Const(0), *): return Const(0);
    default: return e;
    case Mult(*, Const(0)): return Const(0);
    case Mult(Const(1), var x): return Simplify(x);
    case Mult(var x, Const(1)): return Simplify(x);
    case Mult(Const(var l), Const(var r)): return Const(l*r);
    case Add(Const(0), var x): return Simplify(x);
    case Add(var x, Const(0)): return Simplify(x);
    case Add(Const(var l), Const(var r)): return Const(l+r);
    case Neg(Const(var k)): return Const(-k);
  }

and it will not affect the functionality? Will default just be shifted to the end of the list by the compiler therefore? That will be highly confusing: "pattern matching switch statements test each case in order, stopping when a pattern matches, except for default, which will be treated as the last expression, no matter where you put it in the list" That's nasty.

@HaloFour
Copy link

@DavidArno That's the baggage inherited from switch and default. It could be argued that pattern matching doesn't fit well with the semantics of switch considering that order is not supposed to matter. Maybe match should be used for statement pattern matching also. Ditch the baggage altogether.

@DavidArno
Copy link

@HaloFour,

Yes, that's exactly what I'd like to see. The current switch statement is a wholly different thing to a match statement. So don't try and merge the two into some amorphous mess: make them two distinct things.

Further I'm sure that any breaking change concerns around using the match keyword will prove easier to solve than trying to make pattern matching work fully with goto and the like.

@gafter
Copy link
Member Author

gafter commented Feb 18, 2016

@DavidArno Those are already the semantics of the existing switch: it treats the cases as if in order (since they cannot overlap, this is trivially so), except default which is always handled last. The funny order of default is one of two slightly unfortunate effects of using the existing switch statement syntax for pattern-matching, the other being the treatment of goto case. I would be fine adding a warning (or perhaps even an error) when a default appears anywhere but the last position in a switch that contains any pattern-matching syntax.

@bondsbw
Copy link

bondsbw commented Feb 18, 2016

Would match provide exhaustive matching (except in the case that case *: or default: is provided)? And how would this be implemented, using sealed (to ensure it cannot be subclassed elsewhere)?

I don't think I care for match unless it fundamentally provides value such as this. I would hate to use up that keyword and deny a future implementation that can properly guarantee exhaustive matching.

@gafter gafter self-assigned this Feb 18, 2016
@gafter gafter added this to the 2.0 (Preview) milestone Feb 18, 2016
@gafter
Copy link
Member Author

gafter commented Feb 18, 2016

@bondsbw That is precisely the question asked in this issue (fifth question in the list).

@AdamSpeight2008
Copy link
Contributor

match gets my vote. (eg #5202)

match ( e )
{

 |: Mult(Const(0), *),
    Mult(*, Const(0)) => Const(0);

 |: Mult(Const(1), var x),
    Mult(var x, Const(1)) => Simplify(x);

 |: Mult(Const(var l), Const(var r)) => Const(l*r);

 |: Add(Const(0), var x),
    Add(var x, Const(0)) => Simplify(x);

 |: Add(Const(var l), Const(var r)) => Const(l+r);

 |: Neg(Const(var k)) => Const(-k);

 default:
   return e;
}

'default:required in cases where compiler can prove completeness of the matches. If the compiler proves completeness and there is adefault:that section section of code is mark asunreachable?orunnecessary'.

@bondsbw
Copy link

bondsbw commented Feb 18, 2016

@gafter Sorry I realized later that's what you meant by "complete", but I did want to throw my (perhaps unsolicited and humble) opinion in that non-exhaustive/incomplete matching isn't worth locking down a new keyword.

@gafter gafter modified the milestones: 2.0 (RC), 2.0 (Preview) Feb 25, 2016
@JohnnyBravo75
Copy link

I vote for 'switch' because it is an already used construct in c#.
Its syntax is not the best, but its better to reuse it and give it more power with matching than introducing match as an extra keyword.

@AdamSpeight2008
Copy link
Contributor

My concerns with using switch is that it changes the semantics of it, potentially break compatibility with existing code. Using match doesn't as well as indicating that this block is a pattern-match.

@gafter
Copy link
Member Author

gafter commented Mar 21, 2016

@AdamSpeight2008 We will not change the meaning of existing code. This issue is about a new syntactic form for an expression.

@AdamSpeight2008
Copy link
Contributor

@gafter Say if I have pre-existing code using switch then recompile it with a compiler that uses pattern-matching switch. Would it compile to the same code, or different.

@alrz
Copy link
Member

alrz commented Mar 22, 2016

Trailing comma is an embarrassingly bad design decision. Noted.

@AdamSpeight2008
Copy link
Contributor

The final trailing comma shouldn't be needed.

@alrz
Copy link
Member

alrz commented Mar 22, 2016

You guys know that it's optional right? And I'm not proposing it, it is from spec draft.

@DavidArno
Copy link

@alrz,

I'm aware it's optional; that's it's problem. When looking at a piece of code with a trailing comma, one must ask: is it there because the author likes to use them, because they forgot to refactor, or because something has been missed and thus it could be a bug. They create a serious hindrance to readability just to pander to developers who want their lives made fractionally easier when writing the code.

@alrz
Copy link
Member

alrz commented Mar 23, 2016

@orthoxerox Do I need a sarcasm sign? 😄

@DavidArno Perhaps you should research why it is useful in the first place and why do we care about it (or not). This is nothing new in C# or programming language design overall. I don't think that I need to explain the basics. Again, I'm not proposing it and my suggestion is based on the current design (of C# not just match).

@orthoxerox
Copy link
Contributor

@alrz Mornings are never kind to me 😪

I like both trailing commas and curlies.

@DavidArno
Copy link

@alrz,

You may rest assured that I have researched it. There are three main use cases:

  1. It makes it easier to write and edit lists of data;
  2. It makes it easier to write code generators;
  3. It makes it easier to view source change diffs using command line tools.

The first two are only of use to developers who value making their lives easier when writing code over making the lives of readers of their code, easier. The third doesn't really apply to 99.9%of C# developers.

Mistakes will always be made in designing languages and decisions taken 10-15 years ago will often be viewed as wrong with the benefit of hindsight. C# is a conservative language and avoids breaking changes. So we are stuck with trailing commas being supported for some existing features. That doesn't mean they should be used though. And "we have always done it like that" is a very silly reason to repeat those mistakes with new features.

Ergo, what C# currently does isn't important; doing match properly, is.

@HaloFour
Copy link

@DavidArno Being internally consistent is even more important. The syntax doesn't affect the behavior of match (or switch) in the least, this is a personal style choice. If you don't like it, you're free to both not use it and to write an analyzer which errors if it encounters it.

@DavidArno
Copy link

@HaloFour,

Consistency is important, so use () for match as the convention there is not to have trailing commas 😀

@alrz
Copy link
Member

alrz commented Mar 23, 2016

@DavidArno without comma it will be ambiguous with case expressions. There is a reason for every bit of the syntax. You should try to understand this.

@DavidArno
Copy link

@alrz,

How would a trailing comma remove ambiguity?

@HaloFour
Copy link

@DavidArno That works for me.

I'm kind of on the fence about the whole brace v. parenthesis bit. I think parenthesis looks fine for a handful of patterns but beyond that I think braces would look nicer. It's like how a method with a huge number of arguments can look awkward. But it's really a minor concern.

I'm much more concerned about the behavior and features of the expression than it's specific syntax. I'd be thrilled with guillemets and poop emojis if they delivered active patterns and proper AND/OR patterns.

@alrz
Copy link
Member

alrz commented Mar 23, 2016

@DavidArno Oh you are talking about just the trailing comma. Well, that doesn't seem to be up for discussion here, it is about using commas or not at all. And if they prefer to not use it, perhaps they should redesign case expressions to be distinguishable from case labels.

case_expression
-     : shift_expression 'case' pattern ':' shift_expression
+     : relational_expression 'match' pattern ':' shift_expression
      ;

@bondsbw
Copy link

bondsbw commented Mar 23, 2016

Braces are used in C# when the contents are typically expected to be provided on multiple lines. Parentheses are used otherwise.

And I believe that most code formatters make the assumption that parentheses are intended to span one line (assuming it's not too long). I made a fluent syntax for an area of my project that creates a hierarchical outline:

TableOfContents._
(
    BeforeYouBegin,
    Introduction,
    CompilerConcepts._
    (
        Lexer,
        Parser,
        Checker,
        Emitter
    ),
    CreatingYourOwnLanguage._
    (
        Design,
        Optimization,
        Tooling
    )
);

One annoyance is that Resharper wants to reformat the parentheses every time I edit anything, because in most other cases that's the expectation. I suspect other tooling would be similar.

So for match expressions, I feel braces are definitely more consistent than parentheses.

@AdamSpeight2008
Copy link
Contributor

Didn't you say the comma was optional? Hence no abiquity.

-------- Original Message --------
From:David Arno [email protected]
Sent:Wed, 23 Mar 2016 14:29:53 +0000
To:dotnet/roslyn [email protected]
Cc:Adam Speight [email protected]
Subject:Re: [roslyn] Reconcile syntax of "match" expression based on LDM feedback (#8818)

@alrz,

How would a trailing comma remove ambiguity?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@aluanhaddad
Copy link

Such syntax was already proposed and dismissed in #206, specifically the "hijacking" of the return statement.

The phrase "hijacking the meaning of the return statement" is forever etched into my brain. I will never live it down 😝

If possible I would prefer that there be no delimiting ;s or ,s between cases in a match expression. For one thing, consider that the presence of any ; tokens in a switch statement is incidental, they are not part of the syntax of switch. For another, consider the syntax of the ternary operator.

Additionally, I don't think Resharper's, or any tool's, code formatting behavior should impact this proposal.

@gafter
Copy link
Member Author

gafter commented Apr 21, 2017

Issue moved to dotnet/csharplang #487 via ZenHub

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

10 participants