Bug 11934 – Allow `ref` in `foreach` over range iff `front` returns by `ref`

Status
REOPENED
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-01-15T23:45:17Z
Last change time
2024-12-13T18:16:05Z
Keywords
accepts-invalid, pull
Assigned to
No Owner
Creator
Denis Shelomovskii
See also
https://issues.dlang.org/show_bug.cgi?id=11935, https://issues.dlang.org/show_bug.cgi?id=24679
Moved to GitHub: dmd#18759 →

Comments

Comment #0 by verylonglogin.reg — 2014-01-15T23:45:17Z
Test code: --- struct S1 { @property bool empty(); @property int front(); void popFront(); } struct S2 { @property bool empty(); @property ref int front(); void popFront(); } void main() { static assert( __traits(compiles, { foreach( n; S1()) { } })); static assert(!__traits(compiles, { foreach(ref n; S1()) { } })); // fails static assert( __traits(compiles, { foreach(ref n; S2()) { } })); } --- When `front` doesn't return by `ref` it means returned value is a temporary and its modification is nonsense. In such case `ref` in `foreach` should be rejected.
Comment #1 by alphaglosined — 2018-06-14T14:15:13Z
*** Issue 15351 has been marked as a duplicate of this issue. ***
Comment #2 by alphaglosined — 2018-06-14T14:22:46Z
A more substantial example of why this is a problem: --- import std.stdio; import std.algorithm; struct A { this(int i) { id=counter++; counterLive[id] = 1; writeln("CTOR ", id); } ~this() { counterLive[id]--; writeln("DTOR ", id); } this(this) { counterLive[id]++;writeln("POSTBLIT ", id); } static size_t counter=1; static size_t[size_t] counterLive; size_t id; } void main() { foreach(ref r; [A(10), A(2), A(3)].map!(x => x)) { } foreach(k, v; A.counterLive) if (v > 0) writeln("A (", k, ") count ", v); } --- Output: --- CTOR 1 CTOR 2 CTOR 3 POSTBLIT 1 POSTBLIT 1 DTOR 1 POSTBLIT 2 POSTBLIT 2 DTOR 2 POSTBLIT 3 POSTBLIT 3 DTOR 3 A (2) count 2 A (3) count 2 A (1) count 2 DTOR 3 DTOR 2 DTOR 1 --- The destructors for 3 copies never get called.
Comment #3 by simen.kjaras — 2019-02-21T08:08:06Z
Simplified example: import std.stdio; import std.algorithm; struct A { this(int i) { writeln("ctor ", &this); } ~this() { writeln("dtor ", &this); } this(this) { writeln("postblit ", &this); } } unittest { writeln("Without ref:"); foreach(r; [1].map!(x => A(x))) { } writeln("With ref:"); foreach(ref r; [1].map!(x => A(x))) { } } As we can see from the output, the dtor is correctly called in the non-ref case, but not in the ref case. Memory addresses are the same in both cases, and are stack addresses, so it's not a case of 'allocate on heap and let GC sort it out'. As Denis points out though, it may be better for this not to compile than to compile and silently do something other than expected (even though the ref'ed struct is mutated, this is not reflected anywhere since it's a temporary).
Comment #4 by dlang-bot — 2019-02-25T09:17:04Z
@trikko updated dlang/dmd pull request #8437 "Fixed issue 11934. Range with a non-ref front + foreach(ref...)" fixing this issue: - Fixed issue 11934. Range with a non-ref front + foreach(ref...) https://github.com/dlang/dmd/pull/8437
Comment #5 by dlang-bot — 2019-03-01T15:20:02Z
dlang/dmd pull request #8437 "Fixed issue 11934. Range with a non-ref front + foreach(ref...)" was merged into master: - 9a9bcb0cd78dda7ef0c6da0444e10af5b67cfa65 by Andrea Fontana: Fixed issue 11934. Range with a non-ref front + foreach(ref...) https://github.com/dlang/dmd/pull/8437
Comment #6 by nick — 2024-08-19T15:56:15Z
(In reply to Denis Shelomovskii from comment #0) > static assert(!__traits(compiles, { foreach(ref n; S1()) { } })); // fails This line still fails, so reopening. This could be fixed in the next edition.
Comment #7 by robert.schadek — 2024-12-13T18:16:05Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/18759 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB