digitalmars.D.learn - Using a delegate stored as a member of a destroyed struct?
- Nicolas Sicard (35/35) Jan 26 2014 Running a piece of code that can be reduced to:
- Steven Schveighoffer (26/58) Jan 26 2014 No. Just don't do that. The runtime is permitted to move structs
- Nicolas Sicard (23/90) Jan 27 2014 Actually I used a struct because the code is more complex, and it
- Steven Schveighoffer (27/46) Jan 27 2014 Actually, the delegate there is fine! The makeFun function becomes a
- Nicolas Sicard (5/60) Jan 27 2014 This makes perfect sense. My real code works as expected now.
Running a piece of code that can be reduced to:
---
import std.stdio;
void main()
{
import std.range;
foreach(item; iota(0, 10).transform(2))
writeln(item);
}
auto transform(T)(T list, real x)
{
auto t = /* new */ Transformer(x); // line 12
return t.applyTo(list);
}
struct Transformer
{
real delegate(real) fun;
this(real x)
{
fun = (real r) => r * x;
}
auto applyTo(T)(T list)
{
import std.algorithm;
return list.map!(x => fun(x));
}
}
---
the program segfaults. I guess it's because fun is destroyed when
't' goes out of scope in 'transform'. I would have thought that
the MapResult struct returned by 'applyTo' still holds a valid
copy of fun, but I'm wrong... Is there a way to do it?
Of course, uncommenting 'new' on line 12 resolves the problem.
Thanks,
Nicolas
Jan 26 2014
On Sun, 26 Jan 2014 18:41:00 -0500, Nicolas Sicard <dransic gmail.com>
wrote:
Running a piece of code that can be reduced to:
---
import std.stdio;
void main()
{
import std.range;
foreach(item; iota(0, 10).transform(2))
writeln(item);
}
auto transform(T)(T list, real x)
{
auto t = /* new */ Transformer(x); // line 12
return t.applyTo(list);
}
struct Transformer
{
real delegate(real) fun;
this(real x)
{
fun = (real r) => r * x;
}
auto applyTo(T)(T list)
{
import std.algorithm;
return list.map!(x => fun(x));
}
}
---
the program segfaults. I guess it's because fun is destroyed when 't'
goes out of scope in 'transform'. I would have thought that the
MapResult struct returned by 'applyTo' still holds a valid copy of fun,
but I'm wrong... Is there a way to do it?
No. Just don't do that. The runtime is permitted to move structs
bit-for-bit, so you are not allowed to store a pointer that references
'this'. Unless you prevent copying via disable this(this), your struct
could be moved if someone, for instance, passed it as a parameter on the
stack, or returned it.
A delegate using 'this' as the context pointer is the same thing.
The only way to solve this is to put the struct on the heap. But why even
use a struct? You could just use a closure (which automatically goes on
the heap if needed):
auto Transformer(real x)
{
return (real r) => r * x;
}
auto applyTo(X, T)(X fun, T list)
{
import std.algorithm;
return list.map!(x => fun(x));
}
auto transform(T)(T list, real x)
{
auto t = Transformer(x);
return t.applyTo(list);
}
-Steve
Jan 26 2014
On Monday, 27 January 2014 at 02:27:08 UTC, Steven Schveighoffer wrote:On Sun, 26 Jan 2014 18:41:00 -0500, Nicolas Sicard <dransic gmail.com> wrote:Actually I used a struct because the code is more complex, and it builds an array of delegates, which are returned from global functions, like: --- struct Transformer { real delegate(real)[] funs; addFun(real x) { fun ~= makeFun(x); } // etc. } real delegate(real) makeFun(real x) { return (real r) => r * x; } --- This means my design was bad in the first place. Thanks for the explanation. NicolasRunning a piece of code that can be reduced to: --- import std.stdio; void main() { import std.range; foreach(item; iota(0, 10).transform(2)) writeln(item); } auto transform(T)(T list, real x) { auto t = /* new */ Transformer(x); // line 12 return t.applyTo(list); } struct Transformer { real delegate(real) fun; this(real x) { fun = (real r) => r * x; } auto applyTo(T)(T list) { import std.algorithm; return list.map!(x => fun(x)); } } --- the program segfaults. I guess it's because fun is destroyed when 't' goes out of scope in 'transform'. I would have thought that the MapResult struct returned by 'applyTo' still holds a valid copy of fun, but I'm wrong... Is there a way to do it?No. Just don't do that. The runtime is permitted to move structs bit-for-bit, so you are not allowed to store a pointer that references 'this'. Unless you prevent copying via disable this(this), your struct could be moved if someone, for instance, passed it as a parameter on the stack, or returned it. A delegate using 'this' as the context pointer is the same thing. The only way to solve this is to put the struct on the heap. But why even use a struct? You could just use a closure (which automatically goes on the heap if needed): auto Transformer(real x) { return (real r) => r * x; } auto applyTo(X, T)(X fun, T list) { import std.algorithm; return list.map!(x => fun(x)); } auto transform(T)(T list, real x) { auto t = Transformer(x); return t.applyTo(list); } -Steve
Jan 27 2014
On Mon, 27 Jan 2014 03:17:51 -0500, Nicolas Sicard <dransic gmail.com>
wrote:
Actually I used a struct because the code is more complex, and it builds
an array of delegates, which are returned from global functions, like:
---
struct Transformer
{
real delegate(real)[] funs;
addFun(real x)
{
fun ~= makeFun(x);
}
// etc.
}
real delegate(real) makeFun(real x)
{
return (real r) => r * x;
}
---
This means my design was bad in the first place.
Thanks for the explanation.
Actually, the delegate there is fine! The makeFun function becomes a
closure, and will be allocated on the heap.
Where you are running into trouble is simply that the struct goes out of
scope, and the array is therefore invalid.
In fact, I think you were already doing that before (creating a closure).
Here is a possible solution to your original example:
auto applyTo(T)(T list)
{
import std.algorithm;
auto funcopy = fun;
return list.map!(x => funcopy(x));
}
What's happening here is that funcopy is a stack local variable. However,
since you're creating a delegate that uses local variables, the compiler
creates a closure. In essence, it's like putting a new struct on the heap
with the single member funcopy, and using that as the context pointer.
Note that the original code also creates a closure, but 'fun' is a member
of the hidden 'this' reference. Because the 'this' reference refers to
destructed data, fun is garbage, hence the segfault.
I actually was wrong about my original diagnosis. The delegate stored in
your original code does NOT store a delegate with a context pointer that
points to 'this', it's pointing to a closure. Because 'x' doesn't exist
inside the struct, only inside the function. But my statements were still
good advice, don't store pointers to yourself inside a struct :)
-Steve
Jan 27 2014
"Nicolas Sicard" <dransic gmail.com> On Monday, 27 January 2014 at 14:47:21 UTC, Steven Schveighoffer wrote:On Mon, 27 Jan 2014 03:17:51 -0500, Nicolas Sicard <dransic gmail.com> wrote:This makes perfect sense. My real code works as expected now. Thanks for the clear explanation, and advice. NicolasActually I used a struct because the code is more complex, and it builds an array of delegates, which are returned from global functions, like: --- struct Transformer { real delegate(real)[] funs; addFun(real x) { fun ~= makeFun(x); } // etc. } real delegate(real) makeFun(real x) { return (real r) => r * x; } --- This means my design was bad in the first place. Thanks for the explanation.Actually, the delegate there is fine! The makeFun function becomes a closure, and will be allocated on the heap. Where you are running into trouble is simply that the struct goes out of scope, and the array is therefore invalid. In fact, I think you were already doing that before (creating a closure). Here is a possible solution to your original example: auto applyTo(T)(T list) { import std.algorithm; auto funcopy = fun; return list.map!(x => funcopy(x)); } What's happening here is that funcopy is a stack local variable. However, since you're creating a delegate that uses local variables, the compiler creates a closure. In essence, it's like putting a new struct on the heap with the single member funcopy, and using that as the context pointer. Note that the original code also creates a closure, but 'fun' is a member of the hidden 'this' reference. Because the 'this' reference refers to destructed data, fun is garbage, hence the segfault. I actually was wrong about my original diagnosis. The delegate stored in your original code does NOT store a delegate with a context pointer that points to 'this', it's pointing to a closure. Because 'x' doesn't exist inside the struct, only inside the function. But my statements were still good advice, don't store pointers to yourself inside a struct :) -Steve
Jan 27 2014