Issue 11613 - Remove case variables

[yebblies]

Add warning.

https://d.puremagic.com/issues/show_bug.cgi?id=11613

[ghost]

This is actually a dupe report (I'm going to try and find it), and there was some controversial discussion in it AFAIK. IIRC it was also in TDPL.

[ghost]

I think it's this one: http://d.puremagic.com/issues/show_bug.cgi?id=5862

But it wasn't the place where it was discussed, I think it might have been in the NG. I'd have a hard time finding it again though. But if it's listed in TDPL as a feature it probably warrants some new NG discussion so we're clear on whether we support it or not (I'm ok either way).

[yebblies]

Ah ok, I couldn't find the issue so I just opened a new one. I don't remember if any official decision was made on this, but I'm hoping this pull request will force the issue. We've had several NG discussions on this in the past, and I'm hoping we can just get a decision on it from Walter or Andrei.

[ghost]

Is there any specific reason why a case variable must be known at compile-time? I mean other than optimization opportunities. Maybe some people like using switch statements regardless of the optimizations. I have a feeling that if we disable this feature people will inevitably start asking for it as an enhancement in the future.

[yebblies]

The big reason for me is that it is the only case in the language where the choice to interpret or not is not based entirely on context.

It also really bugs me that you can have a switch where the cases aren't distinct. This seems to go against the whole point of a switch. At least with if-else the ordering is explicit and expected.

I absolutely think we should have this feature - as a part of a much more complete pattern matching system.

[ghost]

It also really bugs me that you can have a switch where the cases aren't distinct.

Ah right I haven't thought of that.

[MartinNowak]

I absolutely think we should have this feature - as a part of a much more complete pattern matching system.

👍, but fade out the crippled if-else.

[MartinNowak]

The current implementation is too incomplete to be useful, so deprecating and rethinking it is good.
I would go ahead and merge this, it's only a warning, but I'd like to hear why it was added in the beginning @WalterBright.

[yebblies]

I would go ahead and merge this, it's only a warning, but I'd like to hear why it was added in the beginning

I'm not sure an answer is coming. It would be nice if you would merge it!

[yebblies]

Ping? Time to move on this!

[MartinNowak]

We're waiting for either @andralex or @WalterBright to chime in and explain what this feature was originally intended for.

[ghost]

To quote TDPL:

Usually the case expressions are compile-time constants, 
but D allows variables, too, and guarantees lexical-order
evaluation up to the first match.

...

For [case] labels evaluated during compilation,
it is enforced that no overlap exists. 

[yebblies]

I think it was put in so runtime-initialized immutables would work.

immutable int awful;
static this()
{
    awful = 99999;
}
void main(string[] args)
{
    switch(args.length)
    {
    case 0:
    case awful:
        assert(0);
    default:
        assert(0);
    }
}

[andralex]

I'm not sure what the win is here, and the fact that we're explicitly breaking TDPL biases me :o). Yes, duplicate case labels are possible but only if variables are involved. It's unlikely people would type a variable where they mean a constant, and the nice properties are held if using constant. So there's no loss here as far as I can tell.

We want expressiveness, and allowing variables in switch is a good thing even though they don't get converted into jump tables etc.

I'm leaning toward letting things be. Thoughts?

[yebblies]

I'm leaning toward letting things be. Thoughts?

Have you ever actually used this feature?

It's a departure from normal switch semantics, a hidden performance problem, and has weird semantics where the interpreter is not always invoked.

The fact that TDPL references it is unfortunate, but far from cancels out the problems.

switch(0)
{
case x:
    break;
case 0:
    writeln("You'd think this code would always run, right?");
    break;
default:
}

[bearophile]

@andralex: in this case breaking TDPL is good, because in my opinion the current state of switch is not acceptable.

[andralex]

It's irrelevant whether I used it. In all likelihood others have used it and will read about it in TDPL and attempt to use it. So we need to make a strong case for removal, not a strong case for keeping it for there already is a very strong case.

It's a departure from normal switch semantics,

It's an extension, not a departure. All current semantics is preserved 100%.

a hidden performance problem,

If someone needs a variable in there, they would otherwise use cascading if/else anyway. I don't see how that's a performance problem.

and has weird semantics where the interpreter is not always invoked.

I don't understand this. What are the weird semantics?

Overall I'd say we should let it be.

[andralex]

@bearophile could you please elaborate.

[yebblies]

It's irrelevant whether I used it.

I'll take that as a no. It's relevant because I'd like to know why you're so attached to a feature that it seems nobody uses intentionally.

In all likelihood others have used it and will read about it in TDPL and attempt to use it. So we need to make a strong case for removal, not a strong case for keeping it for there already is a very strong case.

All examples of this being used were either from new users surprised it worked, or bug reports where someone thought it was a compiler bug. Whether or not it's being commonly used is certainly relevant, as lack of use indicates that code breakage is going to be small.

Also note we're talking about a warning here, not a sudden break. If it turns out this will break lots (or any) intentional uses of this feature, we can re-evaluate continuing with deprecation.

It's an extension, not a departure. All current semantics is preserved 100%.

Tomayto, tomahto.

If someone needs a variable in there, they would otherwise use cascading if/else anyway.

They would, and they should!

I don't see how that's a performance problem.

It looks exactly like normal switch code (which is fast) but it isn't. I don't see how that's not a performance problem?

I don't understand this. What are the weird semantics?

• If x is a variable known at compile time, it is treated as a constant.
• If x is a variable not known at compile time, magic runtime switch!
• If any other expression is used, it is interpreted

So now we have:

int x;
int func() { return x; }
switch(...)
{
case x: // work
case func(): // Error: cannot read x at compile time
case x + 1: // Error: cannot read x at compile time
case x * 1: // Error: cannot read x at compile time
}

This is the only place in the language where the choice to invoke ctfe is chosen based on the expression, not just the context.

To make it worse, the dynamic switch only partially works - only variables can be used, not any expression.

[MartinNowak]

We might as well try to improve the current instead of deprecating it. I once implemented this for any kind of variables, not just integral values, to allow basic pattern matching (using opEquals), but nobody liked it.

[yebblies]

We might as well try to improve the current instead of deprecating it. I once implemented this for any kind of variables, not just integral values, to allow basic pattern matching (using opEquals), but nobody liked it.

That sounds nice, but not in switch.

[andralex]

@yebblies: @WalterBright and I discussed more about this and we oppose the change. The arguments stand: the behavior is an extension, it's in TDPL, it works today, and would cause breakage no matter how we go about it. There is nothing this hurts.

I am sorry about your wasted work. Please let me know how we can let this go without starting yet another holy war here or in the newsgroup. Thanks.

[bearophile]

@andralex:

and would cause breakage no matter how we go about it.

I suggest to measure experimentally how much breakage this fix causes (in Phobos and other D projects like vibe.d, etc), and only later take a decision.