https://issues.dlang.org/show_bug.cgi?id=18417

Cauterite <cauterite@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cauterite@gmail.com

--- Comment #1 from Cauterite <cauterite@gmail.com> ---
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.

--

https://issues.dlang.org/show_bug.cgi?id=18417

anonymous4 <dfj1esp02@sneakemail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |spec

--- Comment #2 from anonymous4 <dfj1esp02@sneakemail.com> ---
Dunno, they can still refer global data?
---
int called;
struct A {
        this(this) immutable { called++; }
}
---

--

https://issues.dlang.org/show_bug.cgi?id=18417

--- Comment #3 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
(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.

--

https://issues.dlang.org/show_bug.cgi?id=18417

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--- Comment #4 from Steven Schveighoffer <schveiguy@yahoo.com> ---
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.

--

https://issues.dlang.org/show_bug.cgi?id=18417

RazvanN <razvan.nitu1305@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |razvan.nitu1305@gmail.com

--- Comment #5 from RazvanN <razvan.nitu1305@gmail.com> ---
(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

--

https://issues.dlang.org/show_bug.cgi?id=18417

--- Comment #6 from RazvanN <razvan.nitu1305@gmail.com> ---
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.

--

https://issues.dlang.org/show_bug.cgi?id=18417

--- Comment #7 from Radu Racariu <radu.racariu@gmail.com> ---
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?

--

https://issues.dlang.org/show_bug.cgi?id=18417

--- Comment #8 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
(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.

--

https://issues.dlang.org/show_bug.cgi?id=18417

--- Comment #9 from github-bugzilla@puremagic.com ---
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

--

https://issues.dlang.org/show_bug.cgi?id=18417

github-bugzilla@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--