Thread overview
[Issue 6186] New: Struct destructor is not called on out parameter
Jun 21, 2011
Kenji Hara
Jun 27, 2011
Walter Bright
Jun 27, 2011
Walter Bright
Jul 09, 2012
Benjamin Thaut
Oct 03, 2013
Denis Shelomovskij
June 21, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6186

           Summary: Struct destructor is not called on out parameter
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: k.hara.pg@gmail.com


--- Comment #0 from Kenji Hara <k.hara.pg@gmail.com> 2011-06-20 18:26:25 PDT ---
An object typed T through out parameter is initialized by T.init (like memcpy), but its destructor is not called before initializing.

----
int dtor_cnt;
struct S
{
    int n;
    ~this(){ dtor_cnt++; }
}
void test1()
{
    void f1(out S s){}

    S s = S(1);
    dtor_cnt = 0;
    f1(s);
    assert(dtor_cnt == 1);  // NG
}
void test2()
{
    void f2(out S[2] sa){}

    S[2] sa = [S(1), S(2)];
    dtor_cnt = 0;
    f2(sa);
    assert(dtor_cnt == 2);  // NG
}
void main()
{
    test1();
    test2();
}
----

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 27, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6186


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com


--- Comment #1 from Walter Bright <bugzilla@digitalmars.com> 2011-06-26 17:28:44 PDT ---
I'm unsure what the right fix is for this.

Out variables are supposed to be initialized by the function, not assigned. Hence, I think uninitialized variables should be passed to them, and it should be an error to pass a non-default-initialized variable.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 27, 2011
http://d.puremagic.com/issues/show_bug.cgi?id=6186



--- Comment #2 from Walter Bright <bugzilla@digitalmars.com> 2011-06-26 17:29:25 PDT ---
See also:

https://github.com/D-Programming-Language/dmd/pull/155

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
July 09, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=6186


Benjamin Thaut <code@benjamin-thaut.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |code@benjamin-thaut.de


--- Comment #3 from Benjamin Thaut <code@benjamin-thaut.de> 2012-07-09 09:22:23 PDT ---
Why is the github merge request closed? This issue is not fixed yet and the current behaviour leads to really strange bugs...

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
September 20, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=6186


monarchdodra@gmail.com changed:

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


--- Comment #4 from monarchdodra@gmail.com 2013-09-20 06:22:43 PDT ---
I just hit this. This is my use case, and it's causing a leak:

//----
struct S
{
    this(int)  {writeln("constructor");}
    ~this()    {writeln("destructor");}
}
alias RefCounted!S RCS;

void foo(out RCS){}

void main()
{
    auto rcs = RCS(5); //Use my variable.
    foo(rcs); //Re-use my variable
}
//----
Output:
constructor

EG: It leaked.

Walter says:

> Out variables are supposed to be initialized by the function, not assigned. Hence, I think uninitialized variables should be passed to them, and it should
be an error to pass a non-default-initialized variable.

It was my understanding that the entire *point* of "out" was that the
*language* initialized the parameter, both for the function (no need to test
the variable is in an initial state), and the caller (no need to (re)-set to
initial state).

Overwriting a variable without first destroying it is always wrong behavior, and unsafe. It should not be out's default behavior.

If you insist that the correct behavior is that an out parameter must not be initialized beofre use, then "out"'s correct behavior should instead be to verify the passed argument already has the T.init value, and assert otherwise.

In his pull: https://github.com/D-Programming-Language/dmd/pull/155 Kenji says:

> Sure. It was out of consideration about void-initialized variable.
> But, calling dtor before passing to out parameter is still 'right semantic' for initialized variable. To separate cases, we should detect statically that the variable is initialized or not.

D specs says that "uninitialized" has the value of T.init, and T.init must *always* be destroyable, so that is not an issue, IMO. void-initialization is a user-explicit unsafe state we should not have to bother supporting.

Also, how hard is it to detect the two scenarios?
//----
void foo(out T);
void main()
{
    T t;
    foo(t); //t was *just* decalred, No need to re-initialize
}
//----

Or
//----
void bar(out T t)
{
    foo(t); //t is already out, so no need to re-initialize
}
//----

IMO, the first case represents the majority of "out" usage, and the second an interesting optimization.

In every other case, we can re-initialize.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 02, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=6186


hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh@quickfur.ath.cx


--- Comment #5 from hsteoh@quickfur.ath.cx 2013-10-02 11:27:20 PDT ---
Wow. This is a nasty one. I agree that out parameters should be destructed if they're already initialized.

Isn't it good enough for the compiler to just issue the equivalent of `x = typeof(x).init` at the beginning of the function when x is an out parameter? AFAIK, if you were to actually write this out in code it'd call the dtor before making the assignment (right?). At least, that's what I *thought* the compiler did based on what TDPL describes.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 03, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=6186


Denis Shelomovskij <verylonglogin.reg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |verylonglogin.reg@gmail.com


--- Comment #6 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-10-03 11:22:56 MSD ---
NG thread: http://forum.dlang.org/thread/l2hnbk$ptm$1@digitalmars.com

As Kenji wrote in the thread:

> If an out parameter typed T exists, and
>  1. T has elaborate destructor
>  2. T.init represents logically invalid object (T is nested struct, and/or
> T has @disable this();)
> Automatic blit initializing for the out parameter would be unsafe.

> So semantic analysis would reject such a out parameter declaration.

Also `out` parameters recently become rejected for `@disable this()` structs.

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