Bug 15848 – Identity opAssign not called on out parameters

Status
REOPENED
Severity
normal
Priority
P3
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-03-29T14:52:29Z
Last change time
2024-12-13T18:47:16Z
Keywords
wrong-code
Assigned to
Mathias Lang
Creator
Marc Schütz
Moved to GitHub: dmd#19109 →

Comments

Comment #0 by schuetzm — 2016-03-29T14:52:29Z
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 by lasssafin — 2016-03-29T18:14:26Z
I'm not sure I follow: Why should opAssign be called?
Comment #2 by ag0aep6g — 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
Comment #3 by initrd.gz — 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.
Comment #4 by schuetzm — 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".
Comment #5 by mathias.lang — 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.
Comment #6 by mathias.lang — 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.
Comment #7 by razvan.nitu1305 — 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.
Comment #8 by ag0aep6g — 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.
Comment #9 by robert.schadek — 2024-12-13T18:47:16Z
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