Bug 21175 – opAssign should be allowed to return void and let the compiler take care of chained assignments

Status
NEW
Severity
enhancement
Priority
P4
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2020-08-18T21:39:28Z
Last change time
2024-12-13T19:11:09Z
Assigned to
No Owner
Creator
Andrei Alexandrescu
Moved to GitHub: dmd#19775 →

Comments

Comment #0 by andrei — 2020-08-18T21:39:28Z
The return value of opAssign should always be `this` of type `ref T`. Literally anything else is a bug, and that makes for a poor convention. The compiler should take care of all that. Means user code is free of the convention and can return void. Chained assignments should be lowered as follows. Currently e1 = e2 = e3 is lowered as: e1.opAssign(e2.opAssign(e3)) It should be lowered as: (ref T x) { x.opAssign(e3); e1.opAssign(x); }(e2) meaning e2 is evaluated first, then e3 is evaluated and assigned to (the result of evaluating) e2, then then x is assigned to (the result of evaluating) e1.
Comment #1 by snarwin+bugzilla — 2020-08-19T00:45:15Z
I think a lowering that works for all cases is e1 = e2 to auto ref typeof(e1) (auto ref typeof(e1) v1) { v1.opAssign(e2); return v1; }(e1) This can be expanded recursively to handle any number of chained assignments (e.g., replace e2 with `e2a = e2b`). The only catch is that D doesn't currently allow function literals that return `auto ref`.
Comment #2 by dkorpel — 2021-11-18T15:23:35Z
N.b. this is relevant to dip1000, since this works: ``` @safe: struct S { int* x; void opAssign(return scope S other) scope { this.x = other.x; } } void main() { scope S b; scope S a; a = b; } ``` But this doesn't: ``` @safe: struct S { int* x; ref S opAssign(return scope S other) return scope { this.x = other.x; return this; } } void main() { scope S b; scope S a; a = b; } ``` source/apps/retscope.d(7): Error: scope variable `other` assigned to `this` with longer lifetime The return destination of `return scope S other` was `this` when `opAssign` returned `void`, but now the return destination is the return value. Example in Phobos: https://github.com/dlang/phobos/blob/62780daf85c4c57bbc805e1e6499a33a852a2802/std/datetime/systime.d#L696 The only reason this compiles currently is because of issue 20149
Comment #3 by qs.il.paperinik — 2023-02-22T14:27:34Z
(In reply to Andrei Alexandrescu from comment #0) > The return value of opAssign should always be `this` of type `ref T`. > Literally anything else is a bug, and that makes for a poor convention. The > compiler should take care of all that. Means user code is free of the > convention and can return void. > > Chained assignments should be lowered as follows. Currently e1 = e2 = e3 is > lowered as: > > e1.opAssign(e2.opAssign(e3)) > > It should be lowered as: > > (ref T x) { x.opAssign(e3); e1.opAssign(x); }(e2) > > meaning e2 is evaluated first, then e3 is evaluated and assigned to (the > result of evaluating) e2, then then x is assigned to (the result of > evaluating) e1. The idea is right in principle, but what comes to my mind is C++ valarray. A lot of operator= in there return void. I guess that’s because an assignment operator should not return something else than a reference to the assigned object. If that’s not possible, return `void`. D has better indexing operators than C++: `obj[index] = rhs` can lower to one function call of `opIndexAssign`, meaning that the example is invalid in D; but that is due to index+assignment being a special case in D. There are similar scenarios imaginable in which the issue is relevant.
Comment #4 by robert.schadek — 2024-12-13T19:11:09Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19775 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB