D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 15848 - Identity opAssign not called on out parameters
Summary: Identity opAssign not called on out parameters
Status: REOPENED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P3 normal
Assignee: Mathias Lang
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2016-03-29 14:52 UTC by Marc Schütz
Modified: 2024-12-13 18:47 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Marc Schütz 2016-03-29 14:52:29 UTC
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.
Comment 1 Lass Safin 2016-03-29 18:14:26 UTC
I'm not sure I follow: Why should opAssign be called?
Comment 2 ag0aep6g 2016-03-29 18:56:56 UTC
(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
Comment 3 Alex Parrill 2016-03-31 03:01:10 UTC
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.
Comment 4 Marc Schütz 2016-04-03 13:57:51 UTC
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".
Comment 5 Mathias Lang 2016-06-15 00:52:02 UTC
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.
Comment 6 Mathias Lang 2016-06-15 00:52:34 UTC
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.
Comment 7 RazvanN 2022-11-07 09:05:32 UTC
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.
Comment 8 ag0aep6g 2022-11-07 11:41:02 UTC
(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.
Comment 9 dlangBugzillaToGithub 2024-12-13 18:47:16 UTC
THIS ISSUE HAS BEEN MOVED TO GITHUB

https://github.com/dlang/dmd/issues/19109

DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB