Bug 16455 – Wrong code when calling a struct delegate

Status
RESOLVED
Resolution
INVALID
Severity
critical
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Windows
Creation time
2016-08-31T20:50:00Z
Last change time
2016-09-01T18:54:57Z
Assigned to
nobody
Creator
apz28

Attachments

IDFilenameSummaryContent-TypeSize
1612Test.dTest caseapplication/x-dsrc1842

Comments

Comment #0 by apz28 — 2016-08-31T20:50:42Z
Created attachment 1612 Test case module Test; import std.stdio : writeln; class Node(S) { Node!S parent; Node!S first; Node!S next; S text; this() { } this(S aText) { text = aText; } NodeRange!S children() { return NodeRange!S(this); } Node!S push(Node!S aNode) { assert(aNode.next is null); aNode.parent = this; if (first is null) first = aNode; else { aNode.next = first; first = aNode; } return aNode; } } struct NodeRange(S) { private: Node!S parent; Node!S current; void delegate() doPopFront; void doMove() { writeln("doMove()"); assert(current !is null); current = current.next; } public: this(Node!S aParent) { writeln("aParent.text: ", aParent.text); parent = aParent; current = aParent.first; doPopFront = &doMove; } void popFront() { writeln("popFront()"); assert(current !is null); // work if straight call //doMove(); // access violation when calling delegate // or current is not set correctly doPopFront(); } @property { bool empty() { return (current is null); } Node!S front() { return current; } } } void main() { Node!string tree = new Node!string(); Node!string first = tree.push(new Node!string("first")); first.push(new Node!string("firstChild")); NodeRange!string r = tree.first.children(); assert(!r.empty); writeln("r.front.text: ", r.front.text); assert(r.front.text == "firstChild"); r.popFront(); writeln("r.empty: ", r.empty); assert(r.empty); }
Comment #1 by ag0aep6g — 2016-08-31T21:51:42Z
(In reply to apham from comment #0) > struct NodeRange(S) > { [...] > void delegate() doPopFront; > > void doMove() > { [...] > } [...] > this(Node!S aParent) > { [...] > doPopFront = &doMove; > } [...] > } As far as I see, that code is invalid. &doMove refers to the current location of the struct. But structs can be copied and moved around (the compiler is even allowed to do it on its own). Whenever that happens, doPopFront is invalidated. Closing as invalid. Please reopen if you disagree.
Comment #2 by apz28 — 2016-08-31T22:50:10Z
Based on this https://dlang.org/spec/function.html#closures and the code, the struct var is not moving anywhere and not out of scope, so it must work
Comment #3 by apz28 — 2016-09-01T02:52:20Z
need to move the setting "doPopFront = &doMove;" out of constructor to make it work. Can move it to empty() to complete the initialization
Comment #4 by ag0aep6g — 2016-09-01T18:54:57Z
(In reply to apham from comment #2) > Based on this https://dlang.org/spec/function.html#closures and the code, > the struct var is not moving anywhere and not out of scope, so it must work The NodeRange struct may be moved during construction (constructed at one location, then moved to the target location). If this happens, it invalidates the internal pointer you set up in the constructor. Returning from `children` may mean another copy/move. There are no hard rules here. The compiler has some leeway. From <https://dlang.org/spec/struct.html> (second paragraph): > A struct is defined to not have an identity; that is, the implementation is free to make bit copies of the struct as convenient. (In reply to apham from comment #3) > need to move the setting "doPopFront = &doMove;" out of constructor to make > it work. Can move it to empty() to complete the initialization Note that any copy/move of the struct will still invalidate the pointer. And as far as I understand, the compiler may assume that the struct does not point to itself. So you may run into trouble, even when you have no explicit copy/move in your code.