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

Proposal: Avoiding problems with confusing current switch functionality with pattern matching #8548

Closed
DavidArno opened this issue Feb 10, 2016 · 13 comments
Labels
Area-Language Design Resolution-Answered The question has been answered

Comments

@DavidArno
Copy link

An early proposal around pattern matching was to use match to denote the start of a match expression or statement. This idea met with resistance as it risked causing a breaking change to the language, as the compiler, upon encountering:

match(o) 

could not easily distinguish between a method called match or the keyword.

However, using switch brings it's own problems.

Problem 1: accidentally preventing optimisation of a switch
Currently, a typical switch might look like:

switch (o)
{
    case red:
        Red();
        break;
    case green:
        Green();
        break;
    default:
        OtherColor();
}

With C# 7, this could be written as:

switch (o)
{
    case red:
        Red();
        break;
    case green:
        Green();
        break;
    case *:
        OtherColor();
}

Whilst that small change from default to case * might seem innocent enough, it can have performance implications. Currently, the order of the case statements in a switch is unimportant, for only one case can ever be matched for a given value of o. As such, the compiler can optimise large switches by converting them to a hash map in IL. However, a pattern match statement must have each case examined sequentially as more than one may match and the spec requires that the first match be guaranteed to be selected. This change therefore forces the compiler to treat it as a match statement and thus not apply such optimisations.

* Problem 2: the break problem*

C# enforces the use of break, goto or return as the last statement in a case. This exists as matching ranges can be achieved through "fall through":

switch (o)
{
    case red:
    case orange:
        Reddish();
        break;
    case green:
        Green();
        break;
    default:
        OtherColor();
}

"Fall through" makes no sense with pattern matching though, as each case should be a pattern to match against. Thus the following ought to be treated as an error:

switch (o)
{
    case red:
    case orange:
        Reddish();
        break;
    case green:
        Green();
        break;
    case *:
        OtherColor();
}

As such, it's been proposed that break be removed as a requirement for patterns. However, with the following:

switch (o)
{
    case red: Red();
    case green: Green();
    case *: OtherColor();
}

The compiler must check the whole switch statement to determine if it is a pattern, before reporting missing breaks if it isn't. Or the proposal to remove break simply gets declined.

Both of these problems could be addressed by simply leaving switch as is and using match instead ... except that it can't as previously stated. Therefore a solution could be to work around that breaking change. A couple of possible solutions are:

Use a contextual keyword pair

match on o
{
    case ...
}

If match is used with another keyword, in this case on, though that's only an example, the two together becomes a lexical sequence that cannot currently exist, and so will not cause a break change.

Re-use an existing keyword to make match context-safe

using o match
{
    case ...
}

By using a reserved word in a new way, the code can be guaranteed to not conflict with existing code (eg, using currently must be followed by ([var|<type>] <identifier> = <expression>) and the like and thus using <identifier> match would be easily distinguished by the compiler in a safe fashion.

There are no doubt other options that could be used too. The point being that just because match by itself would cause breaking-change problems, there's no need to cause other non-breaking-change problems by re-using switch instead.

@alrz
Copy link
Contributor

alrz commented Feb 10, 2016

(1) The proposed syntax wasn't match(o), it was something like o match(...) which then changed to o switch(...). (2) According to the spec, case * is not permitted in a switch statement and break remains untouched.

@HaloFour
Copy link

I don't think that there is anything in this proposal that hasn't already been discussed (and discarded) in #206 and #5154.

@DavidArno
Copy link
Author

@alrz,

Which spec is that? The only one I know of is the Pattern matching for C# one, which conflicts with the current implementation on the feature/patterns branch and doesn't really make much sense. The very first definition is recursively nonsensical for example:

relational-expression
    : relational-expression 'is' complex-pattern
    | relational-expression 'is' type
    ;

It's worth checking out Bill Wagner's presentation to NDC last month. If I've remembered it correctly, case * is definitely valid and dropping break is definitely being considered. Unfortunately as the last design meeting notes are from November, and there's no actively maintained public spec description, it's hard to know what the current state of the spec is

@alrz
Copy link
Contributor

alrz commented Feb 10, 2016

The very first definition is recursively nonsensical

It was recursively nonsensical from version 1.0.

@DavidArno
Copy link
Author

@alrz,

If it's made no sense from v1 and still isn't fixed, I'm guessing I'm being naive in thinking we might get a spec that makes sense at some stage, then?

@alrz
Copy link
Contributor

alrz commented Feb 10, 2016

I wonder how this thing even works with incredibly recursively nonsensical grammar. Sure, I'm no expert, but you should definitely contribute to this hugely messed up design.

#MakeC#GreatAgain

@DavidArno
Copy link
Author

@HaloFour,

Again, I can only assume that Bill Wagner's presentation was an accurate representation of the current thinking of the design team. As he explicitly talked about dropping break and how switch isn't definitely agreed on, it's still up for discussion. As both the issues you mentioned cover many topics, it seemed sensible to create a new, focused proposal for this particular topic.

@DavidArno
Copy link
Author

@alrz,

The problem is a "chicken and egg" one. As the spec makes no sense, I can't work out what the full syntax for pattern matching is. As the current code apparently doesn't match the current thinking on the topic with the design team, I'm stuck not knowing where to start in trying to fix the spec... :(

@HaloFour
Copy link

@DavidArno That presentation was not made by Microsoft nor any member of the C# team. I would not consider it as authoritative.

I agree that the spec seriously needs some love.

@HaloFour
Copy link

I'll note that there is a whole tangent on pattern evaluation and fallthrough that appears to happen that @gafter touched in #7703 . break still exists at that time, too.

@gafter
Copy link
Member

gafter commented Feb 10, 2016

Regarding "problem" 1: we intend to generate the same good code in both cases.

Regarding "problem" 2: we do not intend to introduce this problem by adding the restriction you suggest. Whether or not break is a good idea for the switch statement has nothing to do with patterns.

@gafter gafter closed this as completed Feb 10, 2016
@gafter gafter added Area-Language Design Resolution-Answered The question has been answered labels Feb 10, 2016
@DavidArno
Copy link
Author

@HaloFour,

I take it back: you were right and matter is not up for discussion! :)

David

On 10 Feb 2016, at 17:28, Neal Gafter notifications@github.com wrote:

Regarding "problem" 1: we intend to generate the same good code in both cases.

Regarding "problem" 2: we do not intend to introduce this problem by adding the restriction you suggest. Whether or not break is a good idea for the switch statement has nothing to do with patterns.


Reply to this email directly or view it on GitHub.

@HaloFour
Copy link

@DavidArno

I take it back: you were right and matter is not up for discussion! :)

Oh man, I didn't mean that at all! I only meant that I think that these discussions had already bounced around a couple of times. I think revisiting conclusions is a good thing, particularly in light of new ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Language Design Resolution-Answered The question has been answered
Projects
None yet
Development

No branches or pull requests

4 participants