http://d.puremagic.com/issues/show_bug.cgi?id=6194

           Summary: [GSoC] Destructor gets called on object before it is
                    copied when calling writeln()
           Product: D
           Version: D2
          Platform: All
        OS/Version: Windows
            Status: NEW
          Severity: major
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: cristi.cobzarenco@gmail.com


--- Comment #0 from Cristi Cobzarenco <cristi.cobzarenco@gmail.com> 2011-06-22 08:46:01 PDT ---
import std.stdio;
import std.conv;

struct Test {
    int x;

    this( int m )     { x = m; }
    this( this )      { writeln( "Postblit: ", x ); }

    ~this()           { x = 42; }

    string toString() { return to!string( x ); }
}

int main(string[] argv) {
    auto a = Test(3);
    writeln( a );
    return 0;
}

When the running the code above, 42 gets printed instead of 3. Looking at the "Postblit: " writes, one can see that the postblit ctor gets called 6 times and only the last copy sets x to 42. It seems that at the last copy the destructor gets called before the object gets blitted.

Using a debugger, I think I found the offending copy at line 1599 in file format.d. Strangely, as much as I tried to reproduce the same kind of copying, I could never get the bug to appear without using writeln.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6194


kennytm@gmail.com changed:

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


--- Comment #1 from kennytm@gmail.com 2011-06-22 09:51:21 PDT ---
Reduced:

---------------------------------------
struct Test {
    int x;
    ~this() { x = 42; }
}
void formattedWrite(A...)(A args) {
    Test* argsAddresses;
    foreach (a; args) {
        argsAddresses = &a;
        assert(argsAddresses.x != 42, "line 9: unexpected: x == 42");
    }
    assert(argsAddresses.x != 42, "line 11: unexpected: x == 42");
}
int main(string[] argv) {
    auto a = Test(3);
    formattedWrite(a);
    return 0;
}
---------------------------------------
core.exception.AssertError@y.d(11): line 11: unexpected: x == 42
----------------
5   y                                   0x000087e9 onAssertErrorMsg + 73
6   y                                   0x00011be6 _d_assert_msg + 26
7   y                                   0x00001ab6 void
y.formattedWrite!(y.Test).formattedWrite(y.Test) + 138
8   y                                   0x00001944 _Dmain + 24
9   y                                   0x000122e3 extern (C) int
rt.dmain2.main(int, char**).void runMain() + 23
10  y                                   0x00011e8d extern (C) int
rt.dmain2.main(int, char**).void tryExec(scope void delegate()) + 29
11  y                                   0x0001232b extern (C) int
rt.dmain2.main(int, char**).void runAll() + 59
12  y                                   0x00011e8d extern (C) int
rt.dmain2.main(int, char**).void tryExec(scope void delegate()) + 29
13  y                                   0x00011e27 main + 179
14  y                                   0x00001921 start + 53
----------------
---------------------------------------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6194



--- Comment #2 from kennytm@gmail.com 2011-06-22 10:33:34 PDT ---
I'm not sure if it's a bug in DMD or Phobos. I'm inclined to it's Phobos bug. The loop variable 'a' in the reduced 'formattedWrite' is obviously escaping its scope, so calling the destructor is reasonable. However, this is essentially the implementation of std.format.formattedWrite:

1. the arguments are taken address

    foreach (i, arg; args)
    {
        funs[i] = &formatGeneric!(Writer, typeof(arg), Char);
        // We can safely cast away shared because all data is either
        // immutable or completely owned by this function.
        argsAddresses[i] = cast(const(void*)) &arg;
    }

2. and then they are referred later for actual formatting.

        {
            funs[currentArg](w, argsAddresses[currentArg], spec);
            ++currentArg;
        }

https://github.com/D-Programming-Language/phobos/blob/master/std/format.d#L302

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6194



--- Comment #3 from Cristi Cobzarenco <cristi.cobzarenco@gmail.com> 2011-06-24 04:23:28 PDT ---
(In reply to comment #2)
> I'm not sure if it's a bug in DMD or Phobos. I'm inclined to it's Phobos bug. The loop variable 'a' in the reduced 'formattedWrite' is obviously escaping its scope, so calling the destructor is reasonable. However, this is essentially the implementation of std.format.formattedWrite:
> 
> 1. the arguments are taken address
> 
>     foreach (i, arg; args)
>     {
>         funs[i] = &formatGeneric!(Writer, typeof(arg), Char);
>         // We can safely cast away shared because all data is either
>         // immutable or completely owned by this function.
>         argsAddresses[i] = cast(const(void*)) &arg;
>     }
> 
> 2. and then they are referred later for actual formatting.
> 
>         {
>             funs[currentArg](w, argsAddresses[currentArg], spec);
>             ++currentArg;
>         }
> 
> https://github.com/D-Programming-Language/phobos/blob/master/std/format.d#L302


This works for me:
 foreach (i, arg; args)
    {
        funs[i] = &formatGeneric!(Writer, typeof(arg), Char);

        static if(hasAliasing!(typeof(arg)))
        {
-            argsAddresses[i] = &arg;
+            argsAddresses[i] = &args[i];

        }
        else
        {
            // We can safely cast away shared because all data is either
            // immutable or completely owned by this function.
-            argsAddresses[i] = cast(const(void*)) &arg;
+            argsAddresses[i] = cast(const(void*)) &args[i];
        }
    }

Unfortunately we can't replace the foreach with a for loop because we can't do typeof( args[ i ] ). Does any one have any idea how we could avoid copying the arguments needlessly?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6194



--- Comment #4 from Cristi Cobzarenco <cristi.cobzarenco@gmail.com> 2011-07-19 06:24:20 PDT ---
Here's a pull request with my fix. https://github.com/D-Programming-Language/phobos/pull/151

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6194


Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |k.hara.pg@gmail.com
         Resolution|                            |FIXED


--- Comment #5 from Kenji Hara <k.hara.pg@gmail.com> 2011-07-31 13:44:37 PDT ---
https://github.com/D-Programming-Language/phobos/commit/d2541785e79fa67212cb8281122b14e38ea12ecf

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------