D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 5713 - Broken final switch on ints
Summary: Broken final switch on ints
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 major
Assignee: No Owner
URL:
Keywords: accepts-invalid, pull, spec
Depends on:
Blocks:
 
Reported: 2011-03-07 04:27 UTC by bearophile_hugs
Modified: 2021-02-05 12:22 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description bearophile_hugs 2011-03-07 04:27:15 UTC
The purpose of "final switch" is to increase code safety compared to normal switches on enums. This D2 program compiles and runs with no errors (dmd 2.052):


void main() {
    int x = 100;
    final switch (x % 3) {
        case 0: break;
        case 1: break;
    }
}


Two possible ways for the D compiler to manage this code:
- Disallow it, not allowing final switches on int values;
- Manage it correctly and require the case "case 2:" too.
- (A third possibility: ignore the limited range of the switching value and requiring coverage of the whole integer range, but this is not a good solution).
Comment 1 Stewart Gordon 2011-03-07 05:39:56 UTC
At first I thought maybe it was ignoring the error out of knowledge that x % 3 == 1 in this instance.  But no - it still accepts (and runs without even a SwitchError) if I change x to 101.

But you'd need to cover -1 and -2 as well for this to make sense.

The spec doesn't actually disallow it:
http://www.digitalmars.com/d/2.0/statement.html#FinalSwitchStatement

"A final switch statement is just like a switch statement, except that:

    * No DefaultStatement is allowed.
    * No CaseRangeStatements are allowed.
    * If the switch Expression is of enum type, all the enum members must appear in the CaseStatements.
    * The case expressions cannot evaluate to a run time initialized value."

But this seems to be a mistake, and that no SwitchError is thrown strikes me as a bug.
Comment 2 bearophile_hugs 2011-03-07 16:17:51 UTC
See also bug 5714
Comment 3 bearophile_hugs 2011-05-26 15:57:36 UTC
See also bug 6060
Comment 4 bearophile_hugs 2011-08-20 14:05:58 UTC
This code is a reduction of real code, with small changes. It shows why this final switch brokeness makes final switch not as safe as advertised:


void main() {
    enum Foo { A, B }
    enum Bar { C = 5, D = 6 }
    int fe; // first mistake, fe is not Foo
    bool someCondition = true;
    if (someCondition)
        fe = Bar.C; // second mistake, fe is not assigned to a Foo
    final switch (fe) {
        case Foo.A: break;
        case Foo.B: break;
    }
}


The code contains two mistakes, the first is giving fe int type instead of Foo. The second mistake is assigning  to fe an invalid enum value. The final switch doesn't catch the wrong enum value of fe, and it asks for no default case because it's supposed to be complete. The final switch spec need to be improved.
Comment 5 bearophile_hugs 2011-09-07 10:15:54 UTC
An example from Timon Gehr, this gives no compilation errors, and prints nothing:


import std.stdio;
enum Mode { nothing, read, write }
void main() {
    final switch (Mode.read | Mode.write) {
        case Mode.nothing: writeln(0); break;
        case Mode.read:    writeln(1); break;
        case Mode.write:   writeln(2); break;
    }
}
Comment 7 Denis Shelomovskii 2012-01-24 04:01:58 UTC
(In reply to comment #5)
> An example from Timon Gehr, this gives no compilation errors, and prints
> nothing:
> 
> 
> import std.stdio;
> enum Mode { nothing, read, write }
> void main() {
>     final switch (Mode.read | Mode.write) {
>         case Mode.nothing: writeln(0); break;
>         case Mode.read:    writeln(1); break;
>         case Mode.write:   writeln(2); break;
>     }
> }

Created issue 7358 inspired by this (inspired but different because this code can be statically rejected).
Comment 8 Denis Shelomovskii 2012-01-24 04:09:27 UTC
As bearophile wrote in issue 6060 description:
>in 5713 I don't like an error message (and I'd like the compiler to enforce the presence of the cases for 0,1, and 2)

So this issue requires the following function be compilable _iff_ every `case` is present:
---
void f(int x) {
    final switch (x % 3) {
        case -2:
        case -1:
        case 0:
        case 1:
        case 2:
    }
}
---
Comment 9 bearophile_hugs 2012-02-05 06:59:27 UTC
Turning bugs into enhancement requests is a good way to reduce bug count, but it doesn't address the problems. 

"enhancement" sounds like something that someone wants to add, like switching on structs. But this is not the case. Given a sane definition of final switch, asking the compiler to refuse code like this at compile-time is not an enhancement:


void main() {
    int x = 100;
    final switch (x % 3) {
        case 0: break;
        case 1: break;
    }
}


Then maybe we need a "wrong_specs" tag in Bugzilla, for the situations where the compiler is working as the spec say, but where the spec themselves look wrong.
Comment 10 yebblies 2012-02-05 07:38:35 UTC
(In reply to comment #9)
> Then maybe we need a "wrong_specs" tag in Bugzilla, for the situations where
> the compiler is working as the spec say, but where the spec themselves look
> wrong.

The problem with this is that it is completely subjective.  The line between 'I wish D had this' and 'It is a design error that D doesn't have this' varies from person to person, and without hard rules having a keyword to distinguish between them is useless.

I don't have a solution for this, but the people fixing bugs and implementing features are well aware that enhancement means 'not a priority' not 'won't happen'.
Comment 11 Rene 2012-08-31 09:48:27 UTC
Ok, this change broke my code that I wrote *following the spec*. And it only breaks on runtime! The fix was simple (adding a case 0: break;), but still the spec needs to be updated if you guys are changing it. 

And breaking changes that don't give compiler errors on now-wrong-code are quite nasty...
Comment 12 bearophile_hugs 2012-12-23 22:57:35 UTC
This issue was definitively mislabelled, this is clearly a bug, and even significant. Bumped to major.



void main() {
    bool b;
    final switch (b) {
        case true:
            break;
    }
}

It compiles without errors.

At runtime gives:


core.exception.SwitchError@test(3): No appropriate switch clause found
Comment 13 bearophile_hugs 2012-12-26 04:13:35 UTC
See also this thread:
http://forum.dlang.org/thread/hoczugrnzfbtvpnwjevd@forum.dlang.org


I think this code should be supported, because here the compiler is able to statically enforce that every possible ushort value is covered by exactly one of the final switch cases:


void main () {
    ushort x;
    final switch (x) {
        case 0: .. case 1000:
            break;
        case 1001: .. case ushort.max:
            break;
    }
}


See also issue 5714
Comment 14 bearophile_hugs 2014-10-11 22:37:34 UTC
Another simple case worth supporting:


void main() {
    foreach (immutable i; 0 .. 3) {
        final switch (i) {
            case 0: break;
            case 1: break;
            case 2: break;
        }
    }
}
Comment 15 Dlang Bot 2020-08-09 08:29:55 UTC
@WalterBright created dlang/dlang.org pull request #2841 "fix Issue 5713 - Broken final switch on ints" fixing this issue:

- fix Issue 5713 - Broken final switch on ints

https://github.com/dlang/dlang.org/pull/2841
Comment 16 Walter Bright 2020-08-09 08:30:56 UTC
What happens is the compiler inserts a default that throws an exception. This is compatible with what the spec says. I added a clarification to the spec.
Comment 17 Walter Bright 2020-08-09 08:31:46 UTC
It is not an issue of wrong-code.
Comment 18 Dlang Bot 2021-02-05 12:22:43 UTC
dlang/dlang.org pull request #2841 "fix Issue 5713 - Broken final switch on ints" was merged into master:

- 9455b02ee18d49a05af695ed91cfbed84366de64 by Walter Bright:
  fix Issue 5713 - Broken final switch on ints

https://github.com/dlang/dlang.org/pull/2841