www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Enum alias members: yay or nay?

reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
I've recently ran into a bug that was very hard to track down for me.
I've had a good set of unittests, but I kept getting the wrong results
out of my functions, which was very bizarre.

To boil it down, when you introduce a member in an enum which
initializes itself to another member of that enum, the next member
after it will be initialized to + 1 of the previous member. So:

enum E
{
    a,
    b = a,
    c
}

Here, a equals 0, b equals 0 as well, and c equals b + 1, so it's 1.
There's nothing problematic here.

The real problem appears when you need to make sure each member is
unique, except for any members which are either convenience members or
are deprecated members (there's no "deprecated" member feature
yet[2]).

Here's an example:

enum MouseAction
{
    ///
    press,

    ///
    release,

    /** Convenience - equal to $(D press). */
    click = press,

    ///
    double_click,
}

Notice how introducing the convenience member has re-set the enum
initializer counter, meaning double_click will be equal to (click +
1), which is really (press + 1), which becomes (1). But (1) is also
the intializer for "release", so by mistake I made "double_click"
equal "release", and hence my bug.

So to work around this, I thought about introducing an alias feature to enums:

enum MouseAction
{
    press,

    release,

    alias click = press,  // does not reset the counter!

    double_click,  // equals release + 1
}

The alias member would not re-set the counter and instead the next
non-alias member would initialize itself to the previous non-alias
member + 1.

This would also lend itself well with the "deprecated" keyword, where
we could add deprecated aliases for old enum members when we want to
rename the members. For example, if you want to rename a Phobos enum
you can currently do :

enum SpanMode
{
    shallow,
    deep,
    depth = deep;  /// $(RED Deprecated, please use .deep)
    breadth,
}

With the enhancements Issue 9395[2] and Issue 10965[1] we could write
the above as:

enum SpanMode
{
    shallow,
    deep,
    deprecated("Please use .deep") alias depth = deep;
    breadth,
}

Anyway, before I knew it and before any discussion took place, Henning
Pohl already made a pull request[3] implementing the feature! So I had
to go to the newsgroups.

Does the alias member feature pull its weight? Or is it overkill and
we should drop it?

[1] : http://d.puremagic.com/issues/show_bug.cgi?id=10965
[2] : http://d.puremagic.com/issues/show_bug.cgi?id=9395
[3] : https://github.com/D-Programming-Language/dmd/pull/2529
Sep 07 2013
next sibling parent Manfred Nowak <svv1999 hotmail.com> writes:
Andrej Mitrovic wrote:

 Or is it overkill and we should drop it?


This source of errors is the exact counterpart of a missing `break' in a 
`switch'-statement, which may result in an unintended fallthrough, thereby 
resetting some counter.

Therefore syntactical assurance of correctness in `enum's should be 
accompanied by syntactical assurance in `switch'es.

Until then and in those cases, where the implicit counter is indead 
decremented, an error might be exclaimed, because the subtraction indicates 
that the coder has lost the conciousness to code a _linear ordered_ enum.

-manfred
Sep 07 2013
prev sibling next sibling parent reply "Daniel Murphy" <yebblies nospamgmail.com> writes:
"Andrej Mitrovic" <andrej.mitrovich gmail.com> wrote in message 
news:mailman.980.1378598947.1719.digitalmars-d puremagic.com...


 enum MouseAction
 {
    press,

    release,

    alias click = press,  // does not reset the counter!

    double_click,  // equals release + 1
 }

 Does the alias member feature pull its weight? Or is it overkill and
 we should drop it?


Honestly, it seems like overkill to me.

I can understand it would be an annoying bug to hit, but I doubt it would be 
that common.

Two strategies that will prevent this bug are:

1) Put the 'alias' members directly after the member they reference
2) Put the 'alias' members at the end

A possible alternative would be to warn/error on having two implicitly 
initialized enum members that get the same value.

eg

enum E
{
  a,
  b,
  c = a,
  d,  // Warning: Enum member E.d has been automatically assigned the same 
value as E.a
}

tl;dr I don't think this justifies a new feature.  A lint rule, absolutely. 
A warning, possibly.  But not a new feature.
Sep 07 2013
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, September 08, 2013 14:45:21 Daniel Murphy wrote:

 "Andrej Mitrovic" <andrej.mitrovich gmail.com> wrote in message
 news:mailman.980.1378598947.1719.digitalmars-d puremagic.com...
 

 enum MouseAction
 {
 
    press,
    
    release,
    
    alias click = press,  // does not reset the counter!
    
    double_click,  // equals release + 1
 
 }
 
 Does the alias member feature pull its weight? Or is it overkill and
 we should drop it?


 
 Honestly, it seems like overkill to me.
 
 I can understand it would be an annoying bug to hit, but I doubt it would be
 that common.
 
 Two strategies that will prevent this bug are:
 
 1) Put the 'alias' members directly after the member they reference
 2) Put the 'alias' members at the end


Agreed. Both of those easily avoid this problem, and I would have argued that 
the examples which ran into this problem were bad practice precisely because 
they didn't do either of these two. Personally, I would argue that an enum 
that is intended to be have the same value as another should pretty much 
always be listed right after the one it shares a value with.

- Jonathan M Davis
Sep 08 2013
prev sibling next sibling parent reply Kenji Hara <k.hara.pg gmail.com> writes:
2013/9/8 Jonathan M Davis <jmdavisProg gmx.com>


 On Sunday, September 08, 2013 14:45:21 Daniel Murphy wrote:

 Honestly, it seems like overkill to me.

 I can understand it would be an annoying bug to hit, but I doubt it


 would be

 that common.


 Agreed.



I also agree that the compiler enhancement is overkill.

Kenji Hara
Sep 08 2013
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Sunday, 8 September 2013 at 09:43:02 UTC, Kenji Hara wrote:

 I also agree that the compiler enhancement is overkill.

 Kenji Hara


Let's not throw this away quite yet: There is *another* 
fundamental difference:

enum S
{
     a,
     b = a,
}

This creates an enum with *two* entries.

enum S
{
     a,
     alias b = a,
}

This would create an enum with a *single* entry, which can be 
accessed via two different names.

*This*, in itself, I think is a good idea. It helps distinguish 
between "an enum that has multiple entries, some of which have 
the same values" and "an enum whose entry 'a' can also be refered 
to as 'b', fo rconvenience".

For starters, the distinction would be self documenting.

Second, once you involve things like `EnumMembers`, it becomes a 
pretty interesting distinction to make.

I for one support this proposal (but not necessarilly for the 
"counter" proposal: I think it is "a nice plus", but not an 
essential feature).
Sep 08 2013
next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 9/8/13, monarch_dodra <monarchdodra gmail.com> wrote:

 enum S
 {
      a,
      alias b = a,
 }

 This would create an enum with a *single* entry, which can be
 accessed via two different names.


Yeah I've thought about this separately from that enhancement, I think
this feature *alone* would help to avoid issues with duplicate enum
members, e.g. where you're generating a switch case table via
EnumMembers. You can use NoDuplicates to for this, but it still
doesn't change that the following fails:

enum E
{
    a,
    b = a
}

void main()
{
    static assert(is(E.a == E.b));  // fails
}

This may or may not be an issue.. Also, currently we don't have a way
to get the length of enum members except via EnumMembers, but if we
ever implemented "E.length" then maybe the alias behavior could be
useful.
Sep 08 2013
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, September 08, 2013 12:08:07 monarch_dodra wrote:

 On Sunday, 8 September 2013 at 09:43:02 UTC, Kenji Hara wrote:

 I also agree that the compiler enhancement is overkill.
 
 Kenji Hara


 
 Let's not throw this away quite yet: There is *another*
 fundamental difference:
 
 enum S
 {
      a,
      b = a,
 }
 
 This creates an enum with *two* entries.
 
 enum S
 {
      a,
      alias b = a,
 }
 
 This would create an enum with a *single* entry, which can be
 accessed via two different names.
 
 *This*, in itself, I think is a good idea. It helps distinguish
 between "an enum that has multiple entries, some of which have
 the same values" and "an enum whose entry 'a' can also be refered
 to as 'b', fo rconvenience".
 
 For starters, the distinction would be self documenting.
 
 Second, once you involve things like `EnumMembers`, it becomes a
 pretty interesting distinction to make.


But then things get weird, because EnumMembers wouldn't return everything, and 
presumably final switch wouldn't have every member. It's not necessarily a bad 
idea, but I think that it would have to be thought through very thoroughly, 
and ultimately, I'm not sure that it's all that valuable.

The main feature that enums lack that would be nice would be the ability to 
deprecate their members (presumably with the intention of replacing them with 
new names). Aliases of some kind might be beneficial there, but again, I think 
that it all would have to be thought through quite thoroughly. And ultimately, 
it might be that it's just better to deprecate the entire enum at once and 
come up with a new name, much as that's often not what you want to do, because 
you just want to rename some of the values rather than replace the whole 
thing.

So, all in all, I think that any changes to enums along these lines really 
need to be thought through carefully before we consider doing anything, and 
whatever changes we make would have to pull their weight (which Andrej's 
suggestion doesn't do - and he seems to now agree with us on that).

- Jonathan M Davis
Sep 08 2013
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 9/8/13, Daniel Murphy <yebblies nospamgmail.com> wrote:

 Two strategies that will prevent this bug are:

 1) Put the 'alias' members directly after the member they reference
 2) Put the 'alias' members at the end


There is another strategy, which I currently use, is to explicitly
initialize all enums.


be in a location that makes sense in the documentation, this is why
"click" is right above "double_click", "triple_click", etc.
Sep 08 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 9/7/2013 9:45 PM, Daniel Murphy wrote:

 tl;dr I don't think this justifies a new feature.  A lint rule, absolutely.
 A warning, possibly.  But not a new feature.


I agree with the reasoning of the others here - not worth it.
Sep 08 2013
parent "Andrej Mitrovic" <andrej.mitrovich gmail.com> writes:
On Sunday, 8 September 2013 at 23:24:32 UTC, Walter Bright wrote:

 On 9/7/2013 9:45 PM, Daniel Murphy wrote:

 tl;dr I don't think this justifies a new feature.  A lint 
 rule, absolutely.
 A warning, possibly.  But not a new feature.


 I agree with the reasoning of the others here - not worth it.


It seems my last message got lost, but I reached the same 
conclusion:

Anyway after some more thought I think it's overkill, since not
resetting the counter could be just as confusing as resetting it.
Sep 08 2013
prev sibling parent Lionello Lunesu <lionello lunesu.remove.com> writes:
On 9/8/13 8:08, Andrej Mitrovic wrote:

 I've recently ran into a bug that was very hard to track down for me.
 I've had a good set of unittests, but I kept getting the wrong results
 out of my functions, which was very bizarre.

 To boil it down, when you introduce a member in an enum which
 initializes itself to another member of that enum, the next member
 after it will be initialized to + 1 of the previous member. So:

 enum E
 {
      a,
      b = a,
      c
 }

 Here, a equals 0, b equals 0 as well, and c equals b + 1, so it's 1.
 There's nothing problematic here.

 The real problem appears when you need to make sure each member is
 unique, except for any members which are either convenience members or
 are deprecated members (there's no "deprecated" member feature
 yet[2]).

 Here's an example:

 enum MouseAction
 {
      ///
      press,

      ///
      release,

      /** Convenience - equal to $(D press). */
      click = press,

      ///
      double_click,
 }

 Notice how introducing the convenience member has re-set the enum
 initializer counter, meaning double_click will be equal to (click +
 1), which is really (press + 1), which becomes (1). But (1) is also
 the intializer for "release", so by mistake I made "double_click"
 equal "release", and hence my bug.

 So to work around this, I thought about introducing an alias feature to enums:

 enum MouseAction
 {
      press,

      release,

      alias click = press,  // does not reset the counter!

      double_click,  // equals release + 1
 }

 The alias member would not re-set the counter and instead the next
 non-alias member would initialize itself to the previous non-alias
 member + 1.

 This would also lend itself well with the "deprecated" keyword, where
 we could add deprecated aliases for old enum members when we want to
 rename the members. For example, if you want to rename a Phobos enum
 you can currently do :

 enum SpanMode
 {
      shallow,
      deep,
      depth = deep;  /// $(RED Deprecated, please use .deep)
      breadth,
 }


I won't deny the possibility of human error here, but I don't think it's 
worth complicating the language for this case. It's easily fixed by 
putting the 'alias' after the 'good' enum value.

The case for 'deprecate' is much stronger, though. It's impossible to 
deprecate enum values now.
Sep 08 2013