Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Issue 7358 - final switch over enum should add throwing default in debug mode #673

Merged
merged 1 commit into from

7 participants

@yebblies
Collaborator

Add a throwing default case when compiled with asserts on.

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

@andralex
Owner

This is good. We must discuss disabling "|" for enums (which I think is terrible) separately.

@alexrp

@andralex I think disabling that would be very unreasonable; it would make using enums for sort-of-type-safe bit flags annoying.

@andralex
Owner

@alexrp ... and would make using enums for REALLY GOOD CATEGORICAL TYPES awesome.

@donc
Collaborator

Possibly you could make a case for bitwise operations on enums which have had their size explicitly specified and all members given explicit values (eg, enum Foo : int { A = 1, B = 2 }, the Windows header files use enums in that way), but I don't think there's any case for allowing bitwise operations on enums which haven't explicitly specified a type.

@AndrejMitrovic
Collaborator

@andralex: and would make porting C/C++ code difficult.

@jmdavis

| on enums is fine. Having the result be an enum is not, and that is what needs to be changed. Any and all operations which result in an enum should be guaranteed to be a valid enum value. Now, because of casting, it'll still always be possible to get an invalid enum value, so making final switch throw in that case is good, but if casting isn't involved, it should be impossible for operating on enum values to result in a value which is of the enum type and not be a valid enum value.

@AndrejMitrovic
Collaborator

@jmdavis: It would be great if we could actually define valid operations on enums.

For example if you want to only define | on enums you would define opBinary (and other operators when necessary).

enum Flag
{
    a = 0x01,
    b = 0x02;

    @disable auto opBinary(string op, T)(T rhs) { }
    auto opBinary(string op : "|", T)(T rhs) { return this | rhs; }
}

Which would be similar to:

struct Flag
{
    int flag;

    enum a = 0x01;
    enum b = 0x02;

    @disable auto opBinary(string op, T)(T rhs) { }
    auto opBinary(string op : "|", T)(T rhs) { return Flag(this.flag | rhs.flag); }
}

void main()
{
    Flag f = Flag(Flag.a) | Flag(Flag.b);
    assert(f.flag == (0x01 | 0x02));
}
@braddr braddr referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@yebblies yebblies Issue 7358 - final switch over enum should add throwing default in de…
…bug mode at least

Add a throwing default case when compiled with asserts on.
b9ad0b8
@yebblies
Collaborator

Rebased, should be ready.

@donc donc merged commit 638d81d into from
@denis-sh

@andralex I think disabling that would be very unreasonable; it would make using enums for sort-of-type-safe bit flags annoying.

Just want to mention that at least one library solution for bit flags is ready: unstd.typecons.flagEnum.

@yebblies yebblies deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 28, 2012
  1. @yebblies

    Issue 7358 - final switch over enum should add throwing default in de…

    yebblies authored
    …bug mode at least
    
    Add a throwing default case when compiled with asserts on.
This page is out of date. Refresh to see the latest.
Showing with 28 additions and 1 deletion.
  1. +1 −1  src/statement.c
  2. +27 −0 test/runnable/testswitch.d
View
2  src/statement.c
@@ -3272,7 +3272,7 @@ Statement *SwitchStatement::semantic(Scope *sc)
}
#endif
- if (!sc->sw->sdefault && (!isFinal || needswitcherror))
+ if (!sc->sw->sdefault && (!isFinal || needswitcherror || global.params.useAssert))
{ hasNoDefault = 1;
if (!isFinal)
View
27 test/runnable/testswitch.d
@@ -472,6 +472,32 @@ static assert(!is(typeof(
/*****************************************/
+void test7358()
+{
+ static void test7358a()
+ {
+ enum X { A = 1, B = 2 }
+
+ auto x = X.A | X.B;
+
+ final switch(x)
+ {
+ case X.A:
+ case X.B:
+ break;
+ }
+ }
+
+ bool exception;
+ try
+ test7358a();
+ catch (Error)
+ exception = true;
+ assert(exception);
+}
+
+/*****************************************/
+
int main()
{
test1();
@@ -493,6 +519,7 @@ int main()
test17();
test19();
test20();
+ test7358();
printf("Success\n");
return 0;
Something went wrong with that request. Please try again.