Issue 3144 - Invalid break accepted

[yebblies]

Force curly braces on switch statement's body.

It also disables the technically allowed but absolutely useless version:

switch(exp)
    default:
       statement;

I don't see any reason to allow this.

http://d.puremagic.com/issues/show_bug.cgi?id=3144

[alexrp]

For what it's worth, looks good to me.

[andralex]

OK with me. I had mixed feelings about the language change but then I remembered each label introduces its own scope. So having a scope with the whole switch statement seems like a reasonable special case.

[yebblies]

@andralex This change is syntax only, switch statement semantics are unaffected.

[nazriel]

LGTM 👍

@9rnsr @dawgfoto what do you think folks?

[JakobOvrum]

Will this break useful idioms like the following?

enum Foo { a, b }

Foo foo;
final switch(foo) with(Foo)
{
    case a:
        ...
        break;
    case b:
        ...
        break;
}

I know we could switch the statements so that with() is first here, but, a) LOTS of code already does it this way, and b) I think this way reads better.

[yebblies]

@JakobOvrum Yes it will break that, and I agree it shouldn't.

[braddr]

Also, any spec updates to go with this pull request?

[yebblies]

@braddr No, I'm going to take that part out.

[9rnsr]

I also sometimes use switch(foo) with(Foo) idiom, and it is one of valid use I think. Therefore, the parser change is not correct to fix issue 3144.

The right way to fix it is: If a switch statement has zero or one cases (CaseStatement or DefaultStatement, excluding CaseRangeStatement), it will be an error in semantic analysis.

[yebblies]

I also sometimes use switch(foo) with(Foo) idiom, and it is one of valid use I think. Therefore, the parser change is not correct to fix issue 3144.

Yes, I've come to agree.

The right way to fix it is: If a switch statement has zero or one cases (CaseStatement or DefaultStatement, excluding CaseRangeStatement), it will be an error in semantic analysis.

The bug I found 9 months ago was that dmd keeps searching for a closing } even if the switch block didn't start with a {. It will be fairly easy to fix this while preserving with switch(foo) with(Foo) idiom.

I'm fairly sure 'zero cases' is already an error, but 'one case' is perfectly valid.

[9rnsr]

The bug I found 9 months ago was that dmd keeps searching for a closing } even if the switch block didn't start with a {.

OK, just now I have understand the root cause of the bug.
Will you update this pull request based on the discussion?

I'm fairly sure 'zero cases' is already an error, but 'one case' is perfectly valid.

That's correct. I'd like to withdraw the point in my argument.

[9rnsr]

I've opened another pull request #1182, which doesn't rejectswitch(cond) with(exp) { ... } idiom.

[yebblies]

Heh, turns out switch(cond) with(exp) { ... } is actually a bad idea, because when used with a variable, the 'with' initialization is skipped, resulting in crashes.