Bug 3449 – const and immutable struct members do not behave according to spec

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2009-10-29T04:52:00Z
Last change time
2015-02-18T03:38:01Z
Keywords
pull, rejects-valid
Assigned to
nobody
Creator
bugzilla

Comments

Comment #0 by bugzilla — 2009-10-29T04:52:34Z
When struct members are declared const or invariant, they seem to become manifest constants. Example: struct Foo { const int bar = 123; } writeln(Foo.sizeof); // Prints "1", not "4" Foo foo; auto p = &foo.bar; // Error: constant 123 is not an lvalue The same happens if const is replaced with immutable. According to the spec it should be possible to take the address of const/immutable variables, and in the case of const it is even considered well-defined behaviour to change their value after casting them to non-const. I see no reason why this shouldn't apply to structs as well.
Comment #1 by smjg — 2009-10-29T15:08:09Z
(In reply to comment #0) > and in the case of const it is even considered well-defined > behaviour to change their value after casting them to non-const. What bit of the spec is this? Are you sure you aren't confusing D with C(++)? But the rest of what you say is probably right.
Comment #2 by bugzilla — 2009-10-29T23:35:28Z
(In reply to comment #1) > (In reply to comment #0) > > and in the case of const it is even considered well-defined > > behaviour to change their value after casting them to non-const. > > What bit of the spec is this? Are you sure you aren't confusing D with C(++)? You're right, I got things mixed up there. :) On the "const and immutable" page there is a section named "Removing Immutable With A Cast", where it says that "The immutable type can be removed with a cast [...] This does not mean, however, that one can change the data". It says nothing about const, which was what led me to believe that changing consts is not illegal, at least. But I see now that the D/C++ comparison table at the bottom of the page has a similar statement for consts. But the rest still stands: It should be possible to take the address of both consts and immutables.
Comment #3 by smjg — 2009-10-30T06:39:03Z
(In reply to comment #2) > (In reply to comment #1) >> (In reply to comment #0) >>> and in the case of const it is even considered well-defined >>> behaviour to change their value after casting them to non-const. >> >> What bit of the spec is this? Are you sure you aren't confusing D >> with C(++)? > > You're right, I got things mixed up there. :) > > On the "const and immutable" page there is a section named > "Removing Immutable With A Cast", where it says that "The immutable > type can be removed with a cast [...] This does not mean, however, > that one can change the data". It says nothing about const, which > was what led me to believe that changing consts is not illegal, at > least. Since immutable is implicitly convertible to const, it's reasonable that the same rule should apply to const. My guess is that the reason for being able to cast away const/immutable is to interface APIs that take pointers to mutable because they _may_ change the data, but which can be controlled not to. The Windows API function DrawTextEx is an example of this. But I do wish the means of casting away const/immutable were explicit - see http://tinyurl.com/yzzbgdn > But I see now that the D/C++ comparison table at the bottom of the > page has a similar statement for consts. > > But the rest still stands: It should be possible to take the > address of both consts and immutables. Agreed.
Comment #4 by bugzilla — 2010-09-21T04:43:38Z
More strangeness: If you don't explicitly provide an initial value for const/immutable members, they do contribute to the size of the struct. struct Foo { const int i; } writeln(Foo.sizeof); // Prints 4 struct Bar { const int i = 123; } writeln(Bar.sizeof); // Prints 1 I suspect that this bug could cause unexpected memory corruption when such structs are, for instance, passed to C functions -- especially when the behaviour depends on such a small detail.
Comment #5 by bearophile_hugs — 2010-11-03T04:33:09Z
Comment #6 by yebblies — 2011-06-15T20:33:41Z
Comment #7 by bugzilla — 2011-11-24T00:50:02Z
I believe the correct solution is to make const/immutable fields with initializers into static members. Without initializers, they are per-instance fields, and must be initialized by the constructor. If the user wants manifest constants in a struct/class, use enum.
Comment #8 by smjg — 2011-11-24T04:14:00Z
(In reply to comment #7) > I believe the correct solution is to make const/immutable fields with > initializers into static members. Changing this would alter the memory layout of the struct, thereby breaking code that interfaces (for example) a C API or a binary file format.
Comment #9 by bugzilla — 2012-01-23T01:37:27Z
(In reply to comment #4) > More strangeness: If you don't explicitly provide an initial value for > const/immutable members, they do contribute to the size of the struct. > struct Foo { const int i; } > writeln(Foo.sizeof); // Prints 4 > struct Bar { const int i = 123; } > writeln(Bar.sizeof); // Prints 1 This is as designed. A const field without an initializer can be initialized by a constructor. A const field with an initializer does not need any per-instance storage, and becomes a static member. > I suspect that this bug could cause unexpected memory corruption when such > structs are, for instance, passed to C functions -- especially when the > behaviour depends on such a small detail. It is not a bug, it is as designed. (const in D and C are different, and conflating the two will cause problems anyway)
Comment #10 by bugzilla — 2012-01-23T01:39:58Z
(In reply to comment #0) > When struct members are declared const or invariant, they seem to become > manifest constants. Example: > struct Foo { const int bar = 123; } > writeln(Foo.sizeof); // Prints "1", not "4" > Foo foo; > auto p = &foo.bar; // Error: constant 123 is not an lvalue Taking the address should work. Compiler bug, not a spec issue.
Comment #11 by k.hara.pg — 2012-01-23T04:28:52Z
(In reply to comment #9) > This is as designed. A const field without an initializer can be initialized by > a constructor. A const field with an initializer does not need any per-instance > storage, and becomes a static member. > It is not a bug, it is as designed. (const in D and C are different, and > conflating the two will cause problems anyway) (In reply to comment #10) > Taking the address should work. Compiler bug, not a spec issue. I think that the *implicit static* variable is the worst specification in D. 'const/immutable(not modifiable)' and 'static(not per-instance)' is definitely orthogonal concepts, but in your argument, they are scary mixed. So, if we want to need static variable, language *must* require 'static' storage class for the purpose. Otherwise, it will force us a big (and meaningless) leap of imaging.
Comment #12 by smjg — 2012-01-23T05:16:02Z
(In reply to comment #11) > I think that the *implicit static* variable is the worst > specification in D. 'const/immutable(not modifiable)' and > 'static(not per-instance)' is definitely orthogonal concepts, but > in your argument, they are scary mixed. Agreed. Half the point of structs is that the layout in memory can be guaranteed. Being able to include immutable values within this memory layout (such as struct size in the case of some Windows API structs, or file format signatures) should be part of this. In classes, where there is no guarantee of memory layout, it makes sense to optimise immutable members to be static. In structs, OTOH, const/immutable should do what it says and nothing more.
Comment #13 by repeatedly — 2012-05-11T18:30:07Z
I hit this issue in my new library. I have a following struct. struct Option { string name; string[] fields; immutable type = "hoge"; } My library automatically converts such struct to JSON. But "type" field does not exist (tupleof does not return immutable field). expect: {"fields": ["a'], "unique": false, "type": "hoge"} actual: {"fields": ["a'], "unique": false} Currently, I remove immutable keyword temporally but 'type' should be immutable. Is there a better solution?
Comment #14 by qwertie256 — 2012-07-25T13:42:36Z
+1 from me. Implicit static and (worse) implicit enum are bad ideas, and the worst part is that whether it's static or not depends on whether there is an initializer or not. (admittedly I am left wondering what the difference is between "const int" and "immutable int", is it relevant?) However, as a compromise, perhaps if the user writes "const int x = 7;" the compiler could warn: "warning: since x is a constant, it should be declared with static or enum to avoid wasting memory."
Comment #15 by smjg — 2012-07-25T14:55:34Z
(In reply to comment #14) > +1 from me. Implicit static and (worse) implicit enum are bad ideas, and the > worst part is that whether it's static or not depends on whether there is an > initializer or not. I entirely agree. > (admittedly I am left wondering what the difference is > between "const int" and "immutable int", is it relevant?) There isn't any real difference on the surface. But when you take the address of one, you get quite different types. > However, as a compromise, perhaps if the user writes "const int x = 7;" the > compiler could warn: "warning: since x is a constant, it should be declared > with static or enum to avoid wasting memory." How would the programmer suppress this warning because it's deliberate? Maybe we need a new attribute for this. This would also enable an immutable value to be part of a struct's layout without breaking existing code.
Comment #16 by qwertie256 — 2012-07-25T18:55:01Z
> How would the programmer suppress this warning because it's deliberate? When I wrote that, I was thinking that the developer can initialize in the constructor if he really wants it to be non-static. But now I remember that structs can't have a default constructor. Personally I think coders should be able to suppress warnings inside a pragma. But I think I heard Walter doesn't like that. Nevertheless it could easily be a mistake to use "const int Foo = 7;" instead of "enum" or "static const int", and the developer usually won't find out about the mistake without a warning.
Comment #17 by smjg — 2012-07-26T06:14:15Z
(In reply to comment #16) > > How would the programmer suppress this warning because it's deliberate? > > When I wrote that, I was thinking that the developer can initialize in the > constructor if he really wants it to be non-static. But now I remember that > structs can't have a default constructor. Indeed. Moreover, being able to set the value just once in the code, in the member declaration, could be used to address the ongoing problem with structs and const-safety by deciding that a struct with a const/immutable member can be reassigned as long as said member's value is a compile-time constant (in the absence of other constraints preventing it). > Personally I think coders should be able to suppress warnings inside a pragma. > But I think I heard Walter doesn't like that. The tradition in C++ has been that typical compiler warnings can be suppressed by making changes at the code level, for example: - code has no effect - comment it out or cast it to void - unreachable code - comment it out - unused parameter - don't name it - implicit narrowing conversions - use an explicit cast - counter-intuitive operator precedence - use brackets It might be that Walter wants to follow this tradition. Or down to problems with the design of D pragmas. > Nevertheless it could easily be a > mistake to use "const int Foo = 7;" instead of "enum" or "static const int", > and the developer usually won't find out about the mistake without a warning. It could just as easily, if not more, be an attempt to use that declaration thinking it'll actually put that member in the structure's memory layout. An example would be to take a Windows API struct and modify the definition to hard-code that first member that is just the struct's size.
Comment #18 by github-bugzilla — 2012-11-22T03:30:39Z
Commit pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/245274b4083cfc26e45111673b6264151f29abbe Merge pull request #965 from 9rnsr/fix3449 Supplemental fix for Issue 3449 - Stop fwdref by the immutable field
Comment #19 by andrej.mitrovich — 2012-12-26T14:27:45Z
*** Issue 4203 has been marked as a duplicate of this issue. ***
Comment #20 by andrej.mitrovich — 2012-12-26T16:03:42Z
*** Issue 8192 has been marked as a duplicate of this issue. ***
Comment #21 by andrej.mitrovich — 2012-12-27T17:23:46Z
*** Issue 8193 has been marked as a duplicate of this issue. ***
Comment #22 by github-bugzilla — 2013-03-10T00:57:23Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/78e4dad96beccf795de36302feeb02d733553dc2 fix Issue 3449 - const and invariant struct members do not behave according to spec Non manifest-constant aggregate field now have runtime spaces. https://github.com/D-Programming-Language/dmd/commit/9808f85a876e8cb3435daeb3c97765bbf75ac66a Merge pull request #93 from 9rnsr/fix3449 Issue 3449 - const and invariant struct members do not behave according to spec
Comment #23 by andrej.mitrovich — 2013-04-06T07:32:19Z
Can this be marked as fixed?
Comment #24 by github-bugzilla — 2013-05-17T13:07:52Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/5442ea40a311bf9f76ffdd014bb10e2224c72ee1 Add `-vfield` switch for user code migration by fixing bug 3449 https://github.com/D-Programming-Language/dmd/commit/101120eb4f2fb4c4c83922286fc6057a8e645dca Merge pull request #2039 from 9rnsr/vfield Add `-vfield` switch for user code migration by fixing bug 3449
Comment #25 by github-bugzilla — 2013-05-17T13:09:49Z
Commit pushed to 2.063 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/e2ae16b6aa0154d6b19b8a1c3ca7662608f4a3d4 Merge pull request #2039 from 9rnsr/vfield Add `-vfield` switch for user code migration by fixing bug 3449
Comment #26 by bugzilla — 2013-05-25T14:39:10Z
Turning this into a warning instead: https://github.com/D-Programming-Language/dmd/pull/2076
Comment #27 by k.hara.pg — 2013-12-14T11:16:04Z
(In reply to comment #26) > Turning this into a warning instead: > > https://github.com/D-Programming-Language/dmd/pull/2076 Reopen until this is really fixed.
Comment #28 by k.hara.pg — 2013-12-27T05:20:49Z
Pull request to change current warning to deprecation message. https://github.com/D-Programming-Language/dmd/pull/3038
Comment #29 by github-bugzilla — 2013-12-27T09:06:56Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/031c1e3676a70d6be4b1614befcaaa728c3ad80c Issue 3449 transition - Change the warning to deprecation message https://github.com/D-Programming-Language/dmd/commit/65c6248a0920f3d4bfac616046834072c0dd9c82 Merge pull request #3038 from 9rnsr/fix3449 Issue 3449 transition - Change the warning to deprecation message
Comment #30 by k.hara.pg — 2014-10-03T12:21:24Z
Comment #31 by github-bugzilla — 2014-11-10T10:20:14Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/cc2e825d85b0acab1630f709557cf982c8236a86 fix Issue 3449 - const and invariant struct members do not behave according to spec https://github.com/D-Programming-Language/dmd/commit/6e715742066e08176adebec57868975aefbe2355 Merge pull request #4042 from 9rnsr/fix3449 Issue 3449 - const and invariant struct members do not behave according to spec
Comment #32 by schveiguy — 2014-11-10T14:27:47Z
Changing title, this bug was really old, and the changlog will be confusing at this point if not fixed :)
Comment #33 by github-bugzilla — 2015-02-18T03:38:01Z
Commits pushed to 2.067 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/cc2e825d85b0acab1630f709557cf982c8236a86 fix Issue 3449 - const and invariant struct members do not behave according to spec https://github.com/D-Programming-Language/dmd/commit/6e715742066e08176adebec57868975aefbe2355 Merge pull request #4042 from 9rnsr/fix3449