Bug 11051 – Unmatched case in a final switch should throw in both release and non-release mode

----- module test; import std.stdio; enum E { a } string get() { auto e = cast(E)123; final switch(e) { case E.a: return "foobar"; } } void main() { writeln(get()); } ----- $ dmd -run test.d > core.exception.SwitchError@test(10): No appropriate switch clause found $ dmd -release -run test.d > x ↑ The release switch ends up making get() return garbage.

Letting it throw an error in non-release mode kills some of optimization you are supposed to have using a final switch, so I am not sure it's a good idea.

(In reply to comment #1) > Letting it throw an error in non-release mode kills some of optimization you > are supposed to have using a final switch, so I am not sure it's a good idea. You mean *in release mode*, right? It currently throws in non-release mode, it doesn't throw in release mode. But what is the above code supposed to do if not throw in both cases?

(In reply to comment #2) > You mean *in release mode*, right? Right, sorry. > It currently throws in non-release mode, it doesn't throw in release mode. This seems good to me. If you want to convert an untrusted value to an enum safely you should use std.conv.to: import std.stdio, std.conv; enum E { a } string get() { //immutable e = cast(E)123; immutable e = to!E(123); final switch(e) { case E.a: return "foobar"; } } void main() { get.writeln; }

(In reply to comment #3) > This seems good to me. No, returning garbage is not good. At the very least I expect a HALT instruction after the final switch for code which should not reach that point.

(In reply to comment #3) > (In reply to comment #2) > > You mean *in release mode*, right? > > Right, sorry. > > > > It currently throws in non-release mode, it doesn't throw in release mode. > > This seems good to me. If you want to convert an untrusted value to an enum > safely you should use std.conv.to: > > > import std.stdio, std.conv; > > enum E { a } > > string get() { > //immutable e = cast(E)123; > immutable e = to!E(123); > final switch(e) { > case E.a: return "foobar"; > } > } > void main() { > get.writeln; > } No, you will run into memory errors. Either dmd should still throw SwitchError or insert halt instruction.

(In reply to comment #1) > Letting it throw an error in non-release mode kills some of optimization you > are supposed to have using a final switch, so I am not sure it's a good idea. But an error isn't covered by the nothrow contract, so you wouldn't be able to optimize that anyways.

This is an error which should be caught by the type system at compile time, but the type system is broken for enums. The problem is that the 'enum' keyword can mean either 'genuine enumeration' or 'collection of named constants with some unspecified relationship between them'. A 'genuine enumeration' has an implicit contract that it only contains valid values. And 'final switch' relies on that contract. This is broken, because the type system doesn't make that promise. The run-time assert that it's a valid value is really just a hack. Only a 'genuine enumeration' makes sense in a final switch. And arithmetic and logical operations don't make sense on genuine enumerations. Interestingly, a whole-program lint tool could identify all enums which appear inside a 'final switch', and disallow all arithmetic on them. <g> Of course that's completely backwards. The semantics of the enum _should_ be deducable from the declaration of the enum. I see no reason to regard violations of that implicit contract as somehow more important than other contracts. *Any* contract violation will result in incorrect runtime behaviour. The bottom line is that -release is a very dangerous flag. I don't think we should promote a false sense of security about it.

(In reply to Don from comment #7) > Only a 'genuine enumeration' makes sense in a final switch. I'd like "final switch" to support (safely) code like this too: void main(in string[] args) { // Today immutable keeps the value range. immutable n = args % 3; final switch(n) { case 0: break; case 1: break; case 2: break; } }

This seems like another case where a check should stay present in release mode, but only if it's in @safe code.

(In reply to Orvid King from comment #9) > This seems like another case where a check should stay present in release > mode, > but only if it's in @safe code. I like this idea!

@ntrel created dlang/dmd pull request #14841 "Fix issue 11051" fixing this issue: - Fix issue 11051 Keep a HALT instruction in a final switch statement if the function is @safe and -release mode is enabled. https://github.com/dlang/dmd/pull/14841

dlang/dmd pull request #14841 "Fix issue 11051 - Unmatched case in a final switch should throw in both release and non-release mode" was merged into master: - c30a823cdecd206070665950fa1bf0f8231b46d5 by Andrej Mitrovic: Fix issue 11051 Keep a HALT instruction in a final switch statement if the function is @safe and -release mode is enabled. https://github.com/dlang/dmd/pull/14841

*** Issue 14643 has been marked as a duplicate of this issue. ***

This is fixed, but just to note: Issue 14643 shows without this, @safe can be broken due to flow analysis assuming at least one of the case statements is matched.