Identity opAssign not called on out parameters

[dlangBugzillaToGithub]

Marc Schütz (@schuetzm) reported this on 2016-03-29T14:52:29Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=15848

CC List

• ag0aep6g
• Alex Parrill
• Lass Safin
• Mathias Lang
• RazvanN

Description

import std.stdio;

void foo(out Test x) {
    writeln("x.n = ", x.n);
}

struct Test {
    int n;
    ~this() {
        writeln("~this()");
    }
    int opAssign(int val) {
        writefln("opAssign(%s)", val);
        return n = val + 1;
    }
}

void main() {
    Test t;
    foo(t);
    writeln("done");
}

// output:
x.n = 0
done
~this()

Conclusion:
Upon entering foo(), Test.opAssign() is not called. An argument could be made that `out` shouldn't construct a new struct, not assign an existing one, but in that case, it would have to call Test.~this(), which doesn't happen either.

[dlangBugzillaToGithub]

lasssafin commented on 2016-03-29T18:14:26Z

I'm not sure I follow: Why should opAssign be called?

[dlangBugzillaToGithub]

ag0aep6g commented on 2016-03-29T18:56:56Z

(In reply to lasssafin from comment #1)
> I'm not sure I follow: Why should opAssign be called?

Either a new Test is constructed by foo, but then the destructor should be called on the old Test. Or the existing Test is used, but then opAssign should be called instead of just writing Test.init over the old value.

I think the spec [1] is rather clear about which one should happen:

> out	parameter is initialized upon function entry with the default value for its type

So I think the destructor should be called.


1 http://dlang.org/spec/function.html#parameters

[dlangBugzillaToGithub]

initrd.gz commented on 2016-03-31T03:01:10Z

I don't think opAssign should be called here. Default initialization is not assignment; declaring a variable does not call opAssign with T.init, it just copies over T.init.

So the real issue is that `t`'s destructor is not being called when `foo(t)` is ran.

[dlangBugzillaToGithub]

schuetzm commented on 2016-04-03T13:57:51Z

I don't feel strongly either way. But the specification is not clear IMO, "initialized" could be understood as construction as well as a "first assignment".

[dlangBugzillaToGithub]

mathias.lang commented on 2016-06-15T00:52:02Z

Note: Your `opAssign` is not an identity `opAssign` (http://dlang.org/spec/struct.html#assign-overload), so it wouldn't be called in any copying situation.

Correct test code:

```
import std.stdio;

void foo(out Test x) {
    writeln("x.n = ", x.n);
}

struct Test {
    int n;
    ~this() {
        writeln("~this()");
    }
    ref Test opAssign(Test val) {
        writefln("opAssign(%s)", val);
        return this;
    }
}

void main() {
    Test t;
    foo(t);
    writeln("done");
}
```

And this doesn't call `opAssign` either (same output).

What should happen here:
- The destructor should be called if no `opAssign` is defined, because the compiler provides an elaborate `opAssign` when it sees a struct with a dtor or postblit being copied.
- If an identity `opAssign` is defined, it should be called.

I'll change the title, because `out` can be contract as well.

[dlangBugzillaToGithub]

mathias.lang commented on 2016-06-15T00:52:34Z

Note: Your `opAssign` is not an identity `opAssign` (http://dlang.org/spec/struct.html#assign-overload), so it wouldn't be called in any copying situation.

Correct test code:

```
import std.stdio;

void foo(out Test x) {
    writeln("x.n = ", x.n);
}

struct Test {
    int n;
    ~this() {
        writeln("~this()");
    }
    ref Test opAssign(Test val) {
        writefln("opAssign(%s)", val);
        return this;
    }
}

void main() {
    Test t;
    foo(t);
    writeln("done");
}
```

And this doesn't call `opAssign` either (same output).

What should happen here:
- The destructor should be called if no `opAssign` is defined, because the compiler provides an elaborate `opAssign` when it sees a struct with a dtor or postblit being copied.
- If an identity `opAssign` is defined, it should be called.

I'll change the title, because `out` can be contract as well.

[dlangBugzillaToGithub]

razvan.nitu1305 commented on 2022-11-07T09:05:32Z

I think that the behavior exhibited here is correct.

The spec for out parameters says: "The argument must be an lvalue, which will be passed by reference and initialized upon function entry with the default value (T.init) of its type."

Although it is not clearly stated, the way I read it is that the compiler simply blits T.init over the argument and then it passes a reference to it. No copy constructor and no assignment operator are called. Therefore, the destructor is called only for `t` to destroy the object in the main function.

This is correct behavior and much more efficient as it avoids a copy constructor/assignment call and a destructor call.

I'm going to tentatively close this as WORKSFORME. But please reopen if I am missing something.

[dlangBugzillaToGithub]

ag0aep6g commented on 2022-11-07T11:41:02Z

(In reply to RazvanN from comment #7)
> Although it is not clearly stated, the way I read it is that the compiler
> simply blits T.init over the argument and then it passes a reference to it.
> No copy constructor and no assignment operator are called. Therefore, the
> destructor is called only for `t` to destroy the object in the main function.
> 
> This is correct behavior and much more efficient as it avoids a copy
> constructor/assignment call and a destructor call.
> 
> I'm going to tentatively close this as WORKSFORME. But please reopen if I am
> missing something.

Reopening. It's *because* no copy constructor and no assignment operator are being called that the destructor must  be called.

You can't just blit T.init over an existing value without calling its destructor first.

By the way, if the observed behavior were correct, the proper status to close this would be INVALID. WORKSFORME is when you cannot reproduce the described behavior.