Fix Issue 18578 - First enum value assigned 0 instead of EnumBaseType.init#8090
Fix Issue 18578 - First enum value assigned 0 instead of EnumBaseType.init#8090dlang-bot merged 1 commit intodlang:masterfrom
Conversation
|
Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + dmd#8090" |
|
Also this: #7996 |
| } | ||
| Expression e = new IntegerExp(em.loc, 0, Type.tint32); | ||
| e = e.implicitCastTo(sc, t); | ||
| Expression e = new IntegerExp(em.loc, 0, t); |
There was a problem hiding this comment.
Not too familiar with the AST, but @LemonBoy's change "looks" more correct:
Expression e = t.defaultInitLiteral(em.loc);There was a problem hiding this comment.
That was my first option also, but for some reason it led to segfaults when running the tests. From my point of view, enums are an enumeration of constants that are represented by integers behind the scenes, so this solution looks correct (notice that the cast is not needed anymore)
There was a problem hiding this comment.
The trouble is there is significant code depending on it being 0. A deprecation cycle will be needed. @LemonBoy 's solution is more correct, but your test case is better. Both of you fixing the same bug is a pity, as now I have to close one. :-(
There was a problem hiding this comment.
Yeah merging this one first seems reasonable.
@RazvanN7 The language spec doesn't say anything about enums being "integers behind the scenes" (https://dlang.org/spec/enum.html). In fact, if enums had this restriction you couldn't do things like this:
enum message = "MyMessage"; // not an integer behind the scenes
|
Better yet, I'll just approve this one, and regard @LemonBoy 's fix as an enhancement. |
Let's deprecate this behaviour that, according to the specification, is incorrect. Builds up on dlang#8090.
Let's deprecate this behaviour that, according to the specification, is incorrect. Builds up on dlang#8090.
Let's deprecate this behaviour that, according to the specification, is incorrect. Builds up on dlang#8090.
Let's deprecate this behaviour that, according to the specification, is incorrect. Builds up on dlang#8090.
Let's deprecate this behaviour that, according to the specification, is incorrect. Builds up on dlang#8090.
4 participants
No description provided.