Bug 23868 – Compiler-generated opAssign has very high stack frame usage

Status
RESOLVED
Resolution
WONTFIX
Severity
major
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2023-04-29T20:48:35Z
Last change time
2024-01-26T23:06:30Z
Keywords
industry
Assigned to
No Owner
Creator
johanengelen

Comments

Comment #0 by johanengelen — 2023-04-29T20:48:35Z
Testcase: ``` struct S { int[100] arr; ~this() {} } S s; ``` This gives AST (https://d.godbolt.org/z/KKzYnPjah): ``` { int[100] arr; ~this() { } alias __xdtor = ~this() { } ; ref @system S opAssign(S p) return { (S __swap2 = void;) , __swap2 = this , (this = p , __swap2.~this()); return this; } } ``` The local `__swap2` variable in the compiler-generated opAssign() results in very high stack frame usage for large structs. I believe this implemention of opAssign would solve the problem, as I am 99% sure that the semantic difference (dtor call before the assignment) is allowed by the spec: ``` ref @system S opAssign(S p) return { this.S.~this(); this = p; return this; } ``` Workaround is to disable the generation of opAssign: `@disable ref typeof(this) opAssign(typeof(this) p);`.
Comment #1 by dlang-bot — 2023-04-29T22:44:59Z
@JohanEngelen created dlang/dmd pull request #15148 "Attempt to fix issue 23868" mentioning this issue: - Attempt to fix issue 23868 https://issues.dlang.org/show_bug.cgi?id=23868 Not sure if this is correct. https://github.com/dlang/dmd/pull/15148
Comment #2 by bugzilla — 2024-01-26T23:06:30Z
(In reply to johanengelen from comment #0) > I believe this implemention of opAssign would solve the problem, as I am 99% > sure that the semantic difference (dtor call before the assignment) is > allowed by the spec: > ``` > ref @system S opAssign(ref S p) return // note I added `ref` > { > this.S.~this(); > this = p; > return this; > } > ``` This does not work in the general case. The canonical example is if S is a ref-counting wrapper around some other object. If the ref count is 1, and p wraps the same object as `this`, then the wrapped object will get deleted before it gets copied. Adding: @disable ref S opAssign(ref S); will disable the automatic generation of the assignment operator for S. Note use of `ref` for both the return and the parameter. I'm going to close this as WONTFIX because the existing algorithm is the pedantically correct one, writing a custom one will override the standard algorithm, and the generating the standard one can be disabled with @disable. Another solution would be to make `arr` a pointer to the data rather than the data itself. Which solution to pick depends, of course, on the application.