This is the review thread for the first Community Review round for DIP 1014, "Hooking D's struct move semantics".

All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on May 31, or when I make a post declaring it complete.

At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the formal review and evaluation by the language maintainers.

Please familiarize yourself with the documentation for the Community Review before participating.

https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review

Thanks in advance to all who participate.

Destroy!

On Thursday, 17 May 2018 at 08:12:50 UTC, Mike Parker wrote:
> This is the review thread for the first Community Review round for DIP 1014, "Hooking D's struct move semantics".

And the link to the DIP:

https://github.com/dlang/DIPs/blob/38cec74a7471735559e3b8a7553f55102d289d28/DIPs/DIP1014.md

On 17/05/2018 8:12 PM, Mike Parker wrote:
> This is the review thread for the first Community Review round for DIP 1014, "Hooking D's struct move semantics".
> 
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on May 31, or when I make a post declaring it complete.
> 
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the formal review and evaluation by the language maintainers.
> 
> Please familiarize yourself with the documentation for the Community Review before participating.
> 
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
> 
> Thanks in advance to all who participate.
> 
> Destroy!

What is the benefit of opPostMove over copy constructors (not postblit)?

On 17/05/18 11:22, rikki cattermole wrote:
> 
> What is the benefit of opPostMove over copy constructors (not postblit)?

The two are unrelated. A copy is a very different operation from a move. With a copy, you have to figure out how to duplicate the resources used by the object. With a move, no such duplication is needed.

People are somewhat conditioned to treat a move as a "copy+destroy". One certainly may implement it that way. There is a lot of power in not having to do it, though. Think a struct with "@disable this(this)".

The D compiler does moves of structs, whether they are copyable or not. This DIP is about being able to hook this move and fix external and internal references.

Shachar

> 3. When deciding to move a struct instance, the compiler MUST emit a call to the struct's __move_post_blt after blitting the instance and before releasing the memory containing the old instance. __move_post_blt MUST receive references to both the pre- and post-move instances.

This implies that such structs must not be considered PODs, i.e., cannot be passed in registers and must be passed on the stack. It also means that the compiler will have to insert a __move_post_blt call right before the call (as the callee has no idea about the old address), after blitting the arg to the callee params stack; this may be tricky to implement for LDC, as that last blit is implicit in LLVM IR (LLVM byval attribute).

As a side note, when passing a postblit-struct lvalue arg by value, the compiler first copies the lvalue to a temporary on the caller's stack, incl. postblit call, and then moves that copy to the callee. So this requires either a postblit+postmove combo on the caller side before the actual call, or a single postblit call for the final address (callee's param).

I'm not sure I follow all of your comments.

For the rest my comments, let's assume that the compiler may assume that __move_post_blt is a no-op if hasElaborateMove returns false.

On 17/05/18 14:33, kinke wrote:
>> 3. When deciding to move a struct instance, the compiler MUST emit a call to the struct's __move_post_blt after blitting the instance and before releasing the memory containing the old instance. __move_post_blt MUST receive references to both the pre- and post-move instances.
> 
> This implies that such structs must not be considered PODs, i.e., cannot be passed in registers and must be passed on the stack.

I'm not familiar with passing structs in registers. I am familiar with passing pointer to the structs in registers, which should not be affected by this.

If actual (I'm assuming short) structs can be passed in registers, then, yes, structs with elaborate move are not PoDs.

> It also means that the compiler will have to insert a __move_post_blt call right before the call (as the callee has no idea about the old address),

Again, as far as I know, structs are not copied when passed as arguments. They are allocated on the caller's stack and a reference is passed to the callee. If that's the case, no move (of any kind) is done.

I might be missing something. Can you write a code snippet to which you are referring?

> after blitting the arg to the callee params stack; this may be tricky to implement for LDC, as that last blit is implicit in LLVM IR (LLVM byval attribute).

And yet, C++ clang managed to do it in classes with && constructors.

> As a side note, when passing a postblit-struct lvalue arg by value,

Just to be clear, are we talking about passing by reference an instance of a struct that has postblit?

> the compiler first copies the lvalue


> to a temporary on the caller's stack, incl. postblit call, and then moves that copy to the callee. So this requires either a postblit+postmove combo on the caller side before the actual call, or a single postblit call for the final address (callee's param).

Again, that does not align with my familiarity of the ABI. If I do:

struct S {
  ...

  this(this) {
    // some code
  }

  void opPostMove(ref S new, const ref S old) {
    // some code
  }
}

void func1(ref S arg) {
}

void func2(S arg) {
}


As far as I know the ABI, in func1, a pointer to S is passed. In func2, a pointer to caller stack allocate instance is passed, and the original is copied in. It sounds to me like you are talking about the case of:

S s;

func2(s);

in which case you need to copy s to a temporary, and then pass a pointer to that temporary to func2. I don't see where a move enters here.

However, if a move does enter here (and one is necessary if, for example, func2's frame needs to be dynamically allocated because an escaping delegate references it), then, yes, an opPostMove will also need to be called.

Again, if hasElaborateMove returns false, then no change from current behavior is needed.

Shachar

Looks like requirement for @nogc @safe has no consequence as the DIP suggests to infer them anyway. On ideological side safety can't be a requirement because it would contradict its purpose of providing guarantee. Especially if the suggested use case is handling of dangling pointers.

On 17/05/18 16:42, Kagamin wrote:
> Looks like requirement for @nogc @safe has no consequence as the DIP suggests to infer them anyway. On ideological side safety can't be a requirement because it would contradict its purpose of providing guarantee.

I think you are confusing __move_post_blt's implementation (by druntime) with opPostMove's implementation by the user.

For the former, these attributes are deducted by the compiler. For the later, the user may choose to include them for all of the usual reasons for including them, not least of which is that if she does not include @nogc, then trying to move a struct's instance from @nogc code will cause compilation failure.

> Especially if the suggested use case is handling of dangling
> pointers.

There is no such use case. Please remember that at the time opPostMove is called, both new and old memory are still allocated.

Shachar

On Thursday, 17 May 2018 at 12:36:29 UTC, Shachar Shemesh wrote:
> Again, as far as I know, structs are not copied when passed as arguments. They are allocated on the caller's stack and a reference is passed to the callee. If that's the case, no move (of any kind) is done.

That's the exception to the rule (LDC's `ExplicitByvalRewrite`), and true for structs > 64 bit on Win64 (and some more structs) and something similar for AArch64. No other ABIs supported by LDC pass a low-level pointer to a caller-allocated copy for high-level pass-argument-by-value semantics; the argument is normally moved to the function parameter (in the callEE parameters stack).

```
struct S
{
    size_t a, b;
    this(this) {} // no POD anymore
}

void foo(S param);

void bar()
{
    // allocate a temporary on the caller's stack and move it to the callee
    foo(S(1, 2));

    S lvalue;
    // copy lvalue to a temporary on the caller's stack (incl. postblit call)
    // and then move that temporary to the callee
    foo(lvalue);

    import std.algorithm.mutation : move;
    // move move()-rvalue-result to the callee
    foo(move(lvalue));
}
```

'Move to callee' for most ABIs meaning a bitcopy/blit to the callee's memory parameters stack, for LDC via LLVM `byval` attribute.

See IR for https://run.dlang.io/is/1JIsk7.

On Thursday, 17 May 2018 at 15:23:50 UTC, kinke wrote:
> See IR for https://run.dlang.io/is/1JIsk7.

I should probably emphasize that the LLVM `byval` attribute is strange at first sight. Pseudo-IR `void foo(S* byval param); ... foo(S* byarg arg);` doesn't mean that the IR callee gets the S* pointer from the IR callsite; it means 'memcpy(param, arg, S.sizeof)', with `param` being an *implicit* address in foo's parameters stack (calculated by LLVM and so exposed to the callee only). That's the difficulty for LDC I mentioned earlier.