Bug 9985 – Postblit isn't called on local struct return

For this program: http://dpaste.dzfl.pl/d73575a1 I get made S at 18FC64, s.b == 18FC68 got back S at 18FCF4, s.b == 18FC68 as output. Note that no "postblit" message was printed. Patching s.b to point into the newly allocated struct in postblit is crucial here, but it seems like the postblit constructor isn't called, nor is there any attempt to optimize away the temporary in `makeS()` even with -O. Am I doing something wrong? http://dpaste.dzfl.pl/cc460feb copies the struct and ivokes postblit just fine. Maybe this is a bug in RVO?

This is not a bug, but a feature. D has very efficient move semantics, in particular [N]RVO, because D stipulates that stack objects may be *moved* without calling postblits. One of the corollaries to this is that internal pointers are pointers to self are not accepted as legal code in D, and (as you have noticed) breaks the program. Note that your second program exhibits the same behavior: you merely added an extra copy wich triggers the postblit. You have an intermediary temporary. Rule of thumb is that you usually don't need internal pointers anyways. If you absolutely can't do without it, construct your object into a specific place, and *then* connect the pointers. I'll close this if the explanation is okay with you.

I stumbled over this when using a C library and it got me plenty time to trace it back. Seems like after all I missed one of those tiny spec details. Anyhow, I even had a look at the generated assembly on windows, with/without optimization. There seems to be no NRVO applied, since makeS() does 2 copies (rep movsd), one to copy S.init into s and one to copy s into __retval at exit. I may be mistaken, but isn't the point of NRVO to make &s == &__retval, thus only needing to copy S.init into &__retval?

(In reply to comment #2) > I stumbled over this when using a C library and it got me plenty time to trace > it back. Seems like after all I missed one of those tiny spec details. > > Anyhow, I even had a look at the generated assembly on windows, with/without > optimization. > There seems to be no NRVO applied, since makeS() does 2 copies (rep movsd), one > to copy S.init into s and one to copy s into __retval at exit. I may be > mistaken, but isn't the point of NRVO to make &s == &__retval, thus only > needing to copy S.init into &__retval? I could be mistaken, but the "&s == &__retval" would be the "C++ NRVO". Since D is allowed to move things, it just detects that, and does a postblit-less memcopy, which is relatively simple and cheap.

(In reply to comment #3) > > I could be mistaken, but the "&s == &__retval" would be the "C++ NRVO". Since D > is allowed to move things, it just detects that, and does a postblit-less > memcopy, which is relatively simple and cheap. Yeah, I meant NRVO in a C++ sense. So D doesn't attempt that and instead relies on copying the struct without side effects, thus eliding destructors and postblit. Still seems awkward to me, why not just leave out copying all along? Or is memcpy really that fast? If not, is this a potential optimization for the future? Sorry to hijack this into a learning thread. Feel free to close it any time.

(In reply to comment #0) > For this program: http://dpaste.dzfl.pl/d73575a1 Don't link to external web site. Instead please directly paste the code in comment, or attach code file. // Code: import std.stdio; struct S { ubyte* b; ubyte buf[128]; this(this) { writeln("postblit"); } } auto ref makeS() { S s; s.b = s.buf; writeln("made S at ", cast(void*)&s, ", s.b == ", s.b); return s; } void main() { S s = makeS(); writeln("got back S at ", cast(void*)&s, ", s.b == ", s.b); } > I get > > made S at 18FC64, s.b == 18FC68 > got back S at 18FCF4, s.b == 18FC68 > > as output. Note that no "postblit" message was printed. > Patching s.b to point into the newly allocated struct in postblit is crucial > here, but it seems like the postblit constructor isn't called, nor is there any > attempt > to optimize away the temporary in `makeS()` even with -O. > Am I doing something wrong? > > http://dpaste.dzfl.pl/cc460feb // Code: import std.stdio; struct S { ubyte* b; ubyte buf[128]; this(this) { writeln("postblit"); } } auto ref makeS() { S s; s.b = s.buf; writeln("made S at ", cast(void*)&s, ", s.b == ", s.b); return s; } void main() { S s = makeS(); writeln("got back S at ", cast(void*)&s, ", s.b == ", s.b); } > copies the struct and ivokes postblit just fine.

(In reply to comment #0) > Maybe this is a bug in RVO? This is a compiler bug at the intersection of the deduction for `auto ref` and NRVO. If change the code auto ref makeS() to: auto makeS() The code would print the result as follows. made S at 12FDA0, s.b == 12FDA4 got back S at 12FDA0, s.b == 12FDA4 <-- NRVO applied!

(In reply to comment #6) > (In reply to comment #0) > > Maybe this is a bug in RVO? > > This is a compiler bug at the intersection of the deduction for `auto ref` and > NRVO. Could you clarify the "This is a compiler bug"? Are you saying this is an actual bug according to spec, or just that a "missed optimization opportunity" ? In particular, if I compile using "S" instead of "auto ref", then NRVO only triggers in release. However, in non release, postblit still doesn't get called. This is the correct behavior, correct? In non-release, there is no NRVO, but no postblit either, so the code is wrong according to spec? ================ I also want to note that the "NRVO fix" does not actually fix the original code, as passing a temp by value doesn't postblit. This will still fail, even in release, even with NRVO: //-------- void doIt(S9985 s) { assert(S9985.ptr == &s); //Passed without postblit, fails here } void main() { doIt(makeS9985()); } //--------

(In reply to comment #8) > Could you clarify the "This is a compiler bug"? Are you saying this is an > actual bug according to spec, or just that a "missed optimization opportunity" > ? I say "missed optimization opportunity". > In particular, if I compile using "S" instead of "auto ref", then NRVO only > triggers in release. However, in non release, postblit still doesn't get > called. > > This is the correct behavior, correct? In non-release, there is no NRVO, but no > postblit either, so the code is wrong according to spec? With git head dmd, `S makeS()` and `auto makeS()` do NRVO. This is expected. But `auto ref makeS()` doesn't NRVO. This is unexpected and a bug. > ================ > > I also want to note that the "NRVO fix" does not actually fix the original > code, as passing a temp by value doesn't postblit. This will still fail, even > in release, even with NRVO: > > //-------- > void doIt(S9985 s) > { > assert(S9985.ptr == &s); //Passed without postblit, fails here > > } > void main() > { > doIt(makeS9985()); > } > //-------- makeS9985 returns an rvalue, and doIt receives the rvalue with 'move'. There is no copy, so postblit is not called. And, compiler does not apply NRVO in this case. Non-ref parameter `s` in doIt function always means that "the given argument is moved in the function". So &s is always different from S9985.ptr.

(In reply to comment #9) > [SNIP] Thanks.

Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/15912c73192ef85d8cfaceb457c1af19dd575640 fix Issue 9985 - Postblit isn't called on local struct return Until 'ref' deduction finished, auto ref function is not an actual ref function. So compiler should keep NRVO ability. https://github.com/D-Programming-Language/dmd/commit/66f3122cd17c1584db723370de27cd47f025e373 Merge pull request #1933 from 9rnsr/fix9985 Issue 9985 - Postblit isn't called on local struct return