So i've upgraded one of my structs to use the more flexible delegates instead of function pointers but when the member function tries to access the struct's members, the contents are random and the program fails.

i've isolated the problem by adding writefln calls before calling the delegate and inside the delegate(the functions are defined in the struct as member functions, the delegate itself is set in the constructor) :

In the code that uses the delegate :
>writefln!"test %s"(a, &a);
>T b = a.d();//the delegate
While in the most used delegate :
>writefln!"test2 %s %s"(this, &this);
The contents and pointers don't match(they're random, full of 0, -nan, -inf and other invalid values), are they(delegates) supposed to be used like this?
Big thanks

On 2/17/21 10:38 AM, z wrote:
> So i've upgraded one of my structs to use the more flexible delegates instead of function pointers but when the member function tries to access the struct's members, the contents are random and the program fails.
> 
> i've isolated the problem by adding writefln calls before calling the delegate and inside the delegate(the functions are defined in the struct as member functions, the delegate itself is set in the constructor) :
> 
> In the code that uses the delegate :
>> writefln!"test %s"(a, &a);
>> T b = a.d();//the delegate
> While in the most used delegate :
>> writefln!"test2 %s %s"(this, &this);
> The contents and pointers don't match(they're random, full of 0, -nan, -inf and other invalid values), are they(delegates) supposed to be used like this?

With structs and delegates, you have to be careful because structs are copied easily, and using a delegate on a struct that is no longer in scope is going to lead to memory corruption.

In order to properly ensure delegates are pointing to valid data, make sure the struct is still not moved or overwritten, or it is allocated on the heap.

-Steve

On Wed, Feb 17, 2021 at 03:38:00PM +0000, z via Digitalmars-d-learn wrote:
> So i've upgraded one of my structs to use the more flexible delegates instead of function pointers but when the member function tries to access the struct's members, the contents are random and the program fails.

You're likely running into the struct self-reference problem.  Keep in mind that in D, structs are what Andrei calls "glorified ints", i.e., they are value types that get freely copied around and passed around in registers.  Meaning that the address of a struct is likely to change (and change a lot as it gets passed around), unless you explicitly allocated it on the heap.  So if you have any pointers to the struct (including implicit pointers like delegate context pointers) stored inside itself, they are highly likely to become dangling pointers as the struct gets copied to another place and the old copy goes out of scope.

I.e., the following is NOT a good idea:

	struct S {
		void delegate() member;
		this() {
			member = &this.impl;
		}
		private void impl();
	}

because a pointer to S is stored inside S itself, so as soon as S gets copied or moved, the delegate context pointer is no longer valid (or else points to a different copy of the struct than the one it will be called from).

If you need to store references to an aggregate inside itself, you should be using a class instead.  Or be absolutely sure you allocate the struct on the heap with `new`, AND never pass it around except by reference (using pointers).


T

-- 
Almost all proofs have bugs, but almost all theorems are true. -- Paul Pedersen

On Wednesday, 17 February 2021 at 17:45:01 UTC, H. S. Teoh wrote:
> I.e., the following is NOT a good idea:
>
> 	struct S {
> 		void delegate() member;
> 		this() {
> 			member = &this.impl;
> 		}
> 		private void impl();
> 	}
>
> because a pointer to S is stored inside S itself, so as soon as S gets copied or moved, the delegate context pointer is no longer valid (or else points to a different copy of the struct than the one it will be called from).

A copy constructor and opAssign can be used to update pointers that are relative to &this:
    https://dlang.org/spec/struct.html#struct-copy-constructor

// The following program prints 9 with the copy constructor, or 7 if it is commented out:
module app;

import std.stdio;

struct Foo {
    int[2] things;
    private int* ptr;

    this(const(int[2]) things...) inout pure @trusted nothrow @nogc {
        this.things = things;
        ptr = &(this.things[0]);
    }

    // Copy constructor:
    this(scope ref const(typeof(this)) that) inout pure @trusted nothrow @nogc {
        this.things = that.things;
        this.ptr = this.things.ptr + (that.ptr - that.things.ptr);
    }
    // Defining a matching opAssign is a good idea, as well:
    ref typeof(this) opAssign(scope ref const(typeof(this)) that) return pure @trusted nothrow @nogc {
        __ctor(that);
        return this;
    }

    void choose(const(int) x) pure @trusted nothrow @nogc {
        ptr = &(things[x]); }
    @property ref inout(int) chosen() return inout pure @safe nothrow @nogc {
        return *ptr; }
}

void main() {
    auto foo = Foo(3, 7);
    foo.choose(1);

    Foo bar = foo;
    bar.things[1] = 9;
    writeln(bar.chosen);
}

On Wednesday, 17 February 2021 at 19:42:00 UTC, tsbockman wrote:
> On Wednesday, 17 February 2021 at 17:45:01 UTC, H. S. Teoh wrote:
>> I.e., the following is NOT a good idea:
>>
>> 	struct S {
>> 		void delegate() member;
>> 		this() {
>> 			member = &this.impl;
>> 		}
>> 		private void impl();
>> 	}
>>
>> because a pointer to S is stored inside S itself, so as soon as S gets copied or moved, the delegate context pointer is no longer valid (or else points to a different copy of the struct than the one it will be called from).
>
> A copy constructor and opAssign can be used to update pointers that are relative to &this:
>     https://dlang.org/spec/struct.html#struct-copy-constructor

Unfortunately this is not enough, because the compiler is free to implicitly move struct instances in memory any time it wants to. See the bug report below for more details:

https://issues.dlang.org/show_bug.cgi?id=17448

Until D gets move constructors, structs with interior pointers should be avoided.

On Wednesday, 17 February 2021 at 20:18:53 UTC, Paul Backus wrote:
> On Wednesday, 17 February 2021 at 19:42:00 UTC, tsbockman wrote:
>> A copy constructor and opAssign can be used to update pointers that are relative to &this:
>>     https://dlang.org/spec/struct.html#struct-copy-constructor
>
> Unfortunately this is not enough, because the compiler is free to implicitly move struct instances in memory any time it wants to. See the bug report below for more details:
>
> https://issues.dlang.org/show_bug.cgi?id=17448
>
> Until D gets move constructors, structs with interior pointers should be avoided.

That bug is about postblits, this(this), not copy constructors: this(ref typeof(this)). Copy constructors were added to the language specifically to fix those sort of problems. From the spec:

https://dlang.org/spec/struct.html#struct-postblit
> WARNING: The postblit is considered legacy and is not recommended for new code. Code should use copy constructors defined in the previous section. For backward compatibility reasons, a struct that explicitly defines both a copy constructor and a postblit will only use the postblit for implicit copying. However, if the postblit is disabled, the copy constructor will be used. If a struct defines a copy constructor (user-defined or generated) and has fields that define postblits, a deprecation will be issued, informing that the postblit will have priority over the copy constructor.

However, since issue #17448 is still open I have posted a question to the issue report asking for clarification:
    https://issues.dlang.org/show_bug.cgi?id=17448#c39

On Wednesday, 17 February 2021 at 20:44:46 UTC, tsbockman wrote:
> On Wednesday, 17 February 2021 at 20:18:53 UTC, Paul Backus wrote:
>> On Wednesday, 17 February 2021 at 19:42:00 UTC, tsbockman wrote:
>>> A copy constructor and opAssign can be used to update pointers that are relative to &this:
>>>     https://dlang.org/spec/struct.html#struct-copy-constructor
>>
>> Unfortunately this is not enough, because the compiler is free to implicitly move struct instances in memory any time it wants to. See the bug report below for more details:
>>
>> https://issues.dlang.org/show_bug.cgi?id=17448
>>
>> Until D gets move constructors, structs with interior pointers should be avoided.
>
> That bug is about postblits, this(this), not copy constructors: this(ref typeof(this)). Copy constructors were added to the language specifically to fix those sort of problems.

Nope, Paul is right, the copy ctors don't solve anything in this regard. Simplest example I could come up with: https://run.dlang.io/is/TgxyU3

On Thursday, 18 February 2021 at 08:29:48 UTC, kinke wrote:
> On Wednesday, 17 February 2021 at 20:44:46 UTC, tsbockman wrote:
>> On Wednesday, 17 February 2021 at 20:18:53 UTC, Paul Backus wrote:
>>> [...]
>>
>> That bug is about postblits, this(this), not copy constructors: this(ref typeof(this)). Copy constructors were added to the language specifically to fix those sort of problems.
>
> Nope, Paul is right, the copy ctors don't solve anything in this regard. Simplest example I could come up with: https://run.dlang.io/is/TgxyU3

opPostMove https://github.com/dlang/DIPs/blob/master/DIPs/accepted/DIP1014.md can solve this problem but it isn't implemented;

On Thursday, 18 February 2021 at 11:00:50 UTC, vitamin wrote:
>
> opPostMove https://github.com/dlang/DIPs/blob/master/DIPs/accepted/DIP1014.md can solve this problem but it isn't implemented;

IIRC opPostMove has been abandoned for the same reason postblits were abandoned (issues with type-checking and const/immutable).

Walter has a draft DIP that introduces move constructors [1], but it is currently on hold because of a new rule requiring that DIPs authored by a Language Maintainer (i.e., Walter or Atila) have a third-party "champion" to take them through the DIP process. So, if anyone wants to "adopt" this DIP, it would be a great service to the community.

[1] https://github.com/dlang/DIPs/pull/182

On Thursday, 18 February 2021 at 08:29:48 UTC, kinke wrote:
> Nope, Paul is right, the copy ctors don't solve anything in this regard. Simplest example I could come up with: https://run.dlang.io/is/TgxyU3

I found that example very confusing, as it does not contain an explicit copy constructor, violate memory safety, or output incorrect results.

However, I experimented with it and I think figured out what you're getting at? Copy constructors (and postblits) may not be called for moves. I guess that makes sense...

///////////////////////////
import std.stdio : write, writeln;

struct S {
    private const(S*) self, old = null;

    this(bool refSelf) inout pure @trusted {
        if(refSelf)
        	self = &this;
    }
    @disable this(this) inout pure @safe;
    this(scope ref inout(typeof(this)) that) inout pure @trusted {
        self = &this;
        old = &that;
    }

    void print() const @safe {
        write(&this);
        if(old is null)
            write(" (never copied)");
        else
            write(" (last copied from ", old, " to ", self, ")");
        writeln(" in sync: ", self is &this);
    }
}

S makeS() @safe {
    S s = true;
    s.print();
    return s; // NRVO
}

void takeS(S s) @safe {
    s.print();
}

void main() @safe {
    S s = true;
    s.print();

    takeS(s);
    takeS(makeS());
}
///////////////////////////

This works on LDC, but fails on DMD with output:

7FFF765249A0 (never copied) in sync: true
7FFF765249C0 (last copied from 7FFF765249A0 to 7FFF765249D0) in sync: false
7FFF76524A00 (never copied) in sync: true
7FFF765249F0 (never copied) in sync: false

(It's weird that DMD runs the copy constructor with a destination address that isn't even the real destination.)

Anyway, thanks for helping me understand, everyone.