Bug 18417 – Make const and immutable postblit constructors illegal

Fundamentally, const and immutable postblit constructors don't work. A postblit constructor has to mutate the object after it's been copied, which breaks const and immutable. For const this is a serious problem, because it makes it so that objects with a postblit constructor don't work with const, but we'd need some kind of language change to fix that (e.g. introducing some sort of copy constructor). For immutable, the postblit constructor is pointless, because there's no reason to do a deep copy of an immutable object. Either way, having a const or immutable postblit constructor makes no sense, and the errors that we get when folks try aren't necessarily very clear - e.g. this forum post has an example of that: https://forum.dlang.org/post/[email protected] struct A { this(this) immutable {} } struct B { A a; } // Error: immutable method f18.A.__postblit is not callable using a mutable object If I understand correctly, we get the error that we get here becaus __xpostblit tries to call __postblit with a mutable object, but regardless, the resulting error message really doesn't tell the programmer what the real problem is. The real problem is that it doesn't work to declare the postblit constructor as immutable. As such, I propose that we simply make it illegal to mark postblit constructors with const or immutable. It fundamentally doesn't make sense to mark a postblit constructor with const or immutable, and if we simply make it illegal, then folks who try will get an error message about why it can't be done rather than getting unclear error messages related to the postblit constructor not working. The real error is that the postblit constructor has been marked improperly, not that the postblit constructor doesn't work once it's been marked improperly.

i'm wondering, after this is fixed, whether traits.hasElaborateCopyConstructor should return false for immutable types? currently it seems if a struct has a (mutable) postblit, there will be a mutable __xpostblit that does the work, and an immutable __xpostblit that does nothing. hasElaborateCopyConstructor finds the second one and returns true.

Dunno, they can still refer global data? --- int called; struct A { this(this) immutable { called++; } } ---

(In reply to anonymous4 from comment #2) > Dunno, they can still refer global data? > --- > int called; > struct A { > this(this) immutable { called++; } > } > --- And what would the point of that be? The entire point of a postblit constructor is to define how a struct is copied. That has nothing to do with global data. And it's impossible for an immutable or const postblit constructor to be called in the first place. If you try and declare a const or immutable postblit constructor, you just end up with compilation errors, because they don't work - and they can't work, not for what the purpose of a postblit constructor is. Making them illegal would just make it so that the error is clear as to what's going on, whereas right now, they tend to be confusing and cryptic to anyone who doesn't understand what's going on.

Posted it on the forums as well, but I'll do so here too: postblit is what is used to do things like reference counting. You may be altering data that isn't actually part of the struct, so we still do need a postblit capability for immutable structs. Basically the same thing that anonymous4 said, but with a real use case. (In reply to Jonathan M Davis from comment #3) > And it's impossible for an immutable or const postblit > constructor to be called in the first place. If you try and declare a const > or immutable postblit constructor, you just end up with compilation errors, > because they don't work - and they can't work, not for what the purpose of a > postblit constructor is. The purpose is to run some code after blitting. Which is perfectly reasonable for immutable and const structs as well. What we should do is make it possible for immutable/const postblit to work. i.e. immutable S s; immutable y = s; // Error: immutable method S.__postblit is not callable using a mutable object Why does it think y is mutable? This should work.

(In reply to Steven Schveighoffer from comment #4) > Posted it on the forums as well, but I'll do so here too: > > postblit is what is used to do things like reference counting. You may be > altering data that isn't actually part of the struct, so we still do need a > postblit capability for immutable structs. > > Basically the same thing that anonymous4 said, but with a real use case. > > (In reply to Jonathan M Davis from comment #3) > > And it's impossible for an immutable or const postblit > > constructor to be called in the first place. If you try and declare a const > > or immutable postblit constructor, you just end up with compilation errors, > > because they don't work - and they can't work, not for what the purpose of a > > postblit constructor is. > > The purpose is to run some code after blitting. Which is perfectly > reasonable for immutable and const structs as well. > > What we should do is make it possible for immutable/const postblit to work. > > i.e. > > immutable S s; > immutable y = s; // Error: immutable method S.__postblit is not callable > using a mutable object > > Why does it think y is mutable? This should work. That's a different problem: the compiler creates a function __postblit which represents the postblit; when it does that it checks for the struct declaration qualifiers (in this case, none) and forwards them to the postblit. Later, when the immutable instance is created, all members are considered immutable, except for the postblit. That is also the case for shared : [1] [1] https://issues.dlang.org/show_bug.cgi?id=18474

PR : https://github.com/dlang/dmd/pull/8032 The PR makes it illegal to declare a postblit const/immutable/shared, however, it is still possible to make a postblit const/immutable/shared by appending a qualifier to the struct: immutable struct B { this(this) {} } There might be code in the wild which does this but never actually uses the postblit.

As Steven Schveighoffer asked, how do you see an immutable refcounted struct working after this change? Consider this simplified example: ============================================================== struct RC { this (int i) immutable { } this(this) { import core.stdc.stdio; // prints `postblit called on RC` printf("postblit called on %s", typeof(this).stringof.ptr); } } void main() { auto rc1 = immutable RC(1); auto rc2 = rc1; static assert(is(typeof(rc2) == immutable(RC))); } ============================================================== fixing this bug will imply that the postblit will not compile anymore for the above example?

(In reply to RazvanN from comment #6) > PR : https://github.com/dlang/dmd/pull/8032 > > The PR makes it illegal to declare a postblit const/immutable/shared, > however, > it is still possible to make a postblit const/immutable/shared by appending > a qualifier to the struct: > > immutable struct B > { > this(this) {} > } > > There might be code in the wild which does this but never actually uses the > postblit. IMHO, it makes no sense whatsoever to allow for postblit constructor to be const or immutable by marking the struct as const or immutable while making it illegal to mark the postblit constructor const or immutable directly. Either, it needs to be an error just like it is when marking the postblit constructor directly, or it needs to not affect the postblit constructor. I believe that it is currently the case that if immutable is used on a struct, that modifier is ignored on constructors. If that is indeed the case, then the same rule should apply to postblit constructors if marking a postblit constructor as const or immutable is illegal.

Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/86f2428bf3676a849f3b9cbaab2f1441e9776c71 Fix Issue 18417 - Make const and immutable postblit constructors illegal https://github.com/dlang/dmd/commit/6ef4e36f176b905bfd11e3d34ff826d9ebafec86 Merge pull request #8032 from RazvanN7/Issue_18417 Fix Issue 18417 - Make const and immutable postblit constructors illegal