On 25 April 2017 at 00:00, Steven Schveighoffer via Digitalmars-d < digitalmars-d@puremagic.com> wrote:

> On 4/24/17 12:21 AM, Manu via Digitalmars-d wrote:
>
> I wonder if this overload set could be made to work such that it is
>> certain that the non-ref overload is only called with rvalues; ie, given this ambiguous call, ref is preferred for lvalues. rval can not call ref, therefore must resolve to byval.
>>
>
> AFAIK, if you have an overload that varies solely on ref, then rvalues go to the non-ref and lvalues go to the ref. If this is not the case, it's *intended* to be the case, and should be filed as a bug.
>
> auto ref just templates that. And in your case, it's actually clearer and cleaner not to use auto ref.
>
> Not sure if this answers your question, or if it turns out to be a viable solution.
>

If you're going to pinch the guts of rvalue arguments, then this needs to
be 100% reliable.
This needs to be aggressively unit-tested, and probably documented that
this is the official pattern for rvalue construction/assignment operations.

On Monday, 24 April 2017 at 15:19:29 UTC, Manu wrote:

> If you're going to pinch the guts of rvalue arguments, then this needs to
> be 100% reliable.
> This needs to be aggressively unit-tested, and probably documented that
> this is the official pattern for rvalue construction/assignment operations.

Looking over your OP again, and the report made by Petar Kirov, I realized this is not quite right. We can't, generally speaking, treat this(const ref X x) as a copy ctor.

Consider:

struct X
{
    int* ptr;
}

struct Y
{
    int* ptr;

    this(ref const X x)
    {
        // ptr == ???;
        // typeof(x.ptr) == const(int*)
        // seems like the only way to implement this is:
        // ptr = new int(*x.ptr);
    }

    this(X x)
    {
        import std.algorithm.mutation : swap;
        swap(ptr, x.ptr);
    }
}

X x;
Y y = x; // this(ref const X) is called

Suddenly, we can't copy the pointer, or at least make a shallow copy of it, without violating const, and the latter is UB.
The ref const overload should only be called with const lvalues, and it seems to be the case.
OTOH, copying with this(ref X x) will work, as will this(X), so I guess the proper set of overloads should be:

struct Y
{
    this(ref X x) { /*copy...*/ }
    this(X x) { /*move...*/ }
}

with possible addition of this(ref const X x) for a deep copy.

Speaking of const violation and UB, std.algorithm.mutation.move seems to violate const without remorse:

struct Z
{
    const int i;

    ~this() {}
}

auto z = Z(10);
auto zz = move(z);
// z.i is now 0

So, move does set z to a default-constructed state, but in doing so changes the value of a const variable. And there seems to be no other way to implement destructive move.
In case of such Z, of course the only thing you can safely do with z now is destruct it (not that the language can enforce it), so we might say it's all good...

On 4/24/17 2:48 PM, Stanislav Blinov wrote:
> On Monday, 24 April 2017 at 15:19:29 UTC, Manu wrote:
>
>> If you're going to pinch the guts of rvalue arguments, then this needs to
>> be 100% reliable.
>> This needs to be aggressively unit-tested, and probably documented that
>> this is the official pattern for rvalue construction/assignment
>> operations.
>
> Looking over your OP again, and the report made by Petar Kirov, I
> realized this is not quite right. We can't, generally speaking, treat
> this(const ref X x) as a copy ctor.
>
> Consider:
>
> struct X
> {
>     int* ptr;
> }
>
> struct Y
> {
>     int* ptr;
>
>     this(ref const X x)
>     {
>         // ptr == ???;
>         // typeof(x.ptr) == const(int*)
>         // seems like the only way to implement this is:
>         // ptr = new int(*x.ptr);
>     }
>
>     this(X x)
>     {
>         import std.algorithm.mutation : swap;
>         swap(ptr, x.ptr);
>     }
> }
>
> X x;
> Y y = x; // this(ref const X) is called
>
> Suddenly, we can't copy the pointer, or at least make a shallow copy of
> it, without violating const, and the latter is UB.

This is what inout is for. I'll update the bug report.

-Steve

On 04/24/2017 08:48 PM, Stanislav Blinov wrote:
> Speaking of const violation and UB, std.algorithm.mutation.move seems to
> violate const without remorse:

Yup. Even in @safe code, which is a bug. https://issues.dlang.org/show_bug.cgi?id=15315

On 04/24/2017 04:23 PM, ag0aep6g wrote:
> On 04/24/2017 08:48 PM, Stanislav Blinov wrote:
>> Speaking of const violation and UB, std.algorithm.mutation.move seems to
>> violate const without remorse:
>
> Yup. Even in @safe code, which is a bug.
> https://issues.dlang.org/show_bug.cgi?id=15315

Should fail in all situations, thanks for reporting.

The original code should work like this:

struct X {}

struct Y {
  private X _x;
  this()(auto ref X x)
  {
    static if (__traits(isRef, x))
    {
        _x = x;
    }
    else
    {
        import std.algorithm.mutation : move;
        _x = move(x);
    }
  }
}

Note how I made the ctor templated so it accepts auto ref. The less sophisticated version is simply:

struct X {}

struct Y {
  private X _x;
  this(ref X x)
  {
     _x = x;
  }
  this(X x)
  {
     import std.algorithm.mutation : move;
     _x = move(x);
  }
}

There should be ways to do this easier, i.e. add a forward() function to std.algorithm.mutation. Contributions are welcome!


Andrei

On Monday, 24 April 2017 at 22:46:18 UTC, Andrei Alexandrescu wrote:
> On 04/24/2017 04:23 PM, ag0aep6g wrote:
>> On 04/24/2017 08:48 PM, Stanislav Blinov wrote:
>>> Speaking of const violation and UB, std.algorithm.mutation.move seems to
>>> violate const without remorse:
>>
>> Yup. Even in @safe code, which is a bug.
>> https://issues.dlang.org/show_bug.cgi?id=15315
>
> Should fail in all situations, thanks for reporting.

So, basically any type with const/immutable members effectively becomes immovable?

> There should be ways to do this easier, i.e. add a forward() function to std.algorithm.mutation. Contributions are welcome!

Ah, thanks for the hint. There actually is one in std.functional, but it's not designed to handle one-argument situations well (returns a tuple instead of forwarding the argument). This works:

import std.algorithm.mutation : move;

template forward(args...)
{
    import std.meta;

    static if (args.length)
    {
        static if (__traits(isRef, args[0]))
            alias fwd = args[0];
        else
            @property fwd() { return move(args[0]); }

        static if (args.length == 1)
            alias forward = fwd;
        else
            alias forward = AliasSeq!(fwd, forward!(args[1..$]));
    }
    else alias forward = AliasSeq!();
}

struct Y
{
    private X _x;
    this()(auto ref X x)
    {
        _x = forward!x;
    }
}

struct X {
    this(this) { writeln("copy"); }
}

void main()
{
    X x;
    Y y = x;        // outputs "copy"
    Y y2 = move(x); // moves, postblit not called
}

I'll make the PR.

On 04/24/2017 07:13 PM, Stanislav Blinov wrote:
> On Monday, 24 April 2017 at 22:46:18 UTC, Andrei Alexandrescu wrote:
>> On 04/24/2017 04:23 PM, ag0aep6g wrote:
>>> On 04/24/2017 08:48 PM, Stanislav Blinov wrote:
>>>> Speaking of const violation and UB, std.algorithm.mutation.move
>>>> seems to
>>>> violate const without remorse:
>>>
>>> Yup. Even in @safe code, which is a bug.
>>> https://issues.dlang.org/show_bug.cgi?id=15315
>>
>> Should fail in all situations, thanks for reporting.
>
> So, basically any type with const/immutable members effectively becomes
> immovable?

If the type moved from has no elaborate postblit, a move is the same as a copy so those should work.

> I'll make the PR.

Thanks!


Andrei

On 25 April 2017 at 08:46, Andrei Alexandrescu via Digitalmars-d < digitalmars-d@puremagic.com> wrote:

> On 04/24/2017 04:23 PM, ag0aep6g wrote:
>
>> On 04/24/2017 08:48 PM, Stanislav Blinov wrote:
>>
>>> Speaking of const violation and UB, std.algorithm.mutation.move seems to violate const without remorse:
>>>
>>
>> Yup. Even in @safe code, which is a bug. https://issues.dlang.org/show_bug.cgi?id=15315
>>
>
> Should fail in all situations, thanks for reporting.
>
> The original code should work like this:
>
> struct X {}
>
> struct Y {
>   private X _x;
>   this()(auto ref X x)
>   {
>     static if (__traits(isRef, x))
>     {
>         _x = x;
>     }
>     else
>     {
>         import std.algorithm.mutation : move;
>         _x = move(x);
>     }
>   }
> }
>
> Note how I made the ctor templated so it accepts auto ref. The less sophisticated version is simply:
>
> struct X {}
>
> struct Y {
>   private X _x;
>   this(ref X x)
>   {
>      _x = x;
>   }
>   this(X x)
>   {
>      import std.algorithm.mutation : move;
>      _x = move(x);
>   }
> }
>

Ah crap, I somehow missed the single-argument move() function.

Okay, so this pattern is definitely reliable? I haven't seen it clearly
documented anywhere that this is the prescribed pattern, and it's come up
on stack overflow a few times, but it hasn't been answered correctly.
I think this is poorly distributed knowledge.

As Schveighoffer pointed out, this pattern requires *another* overload for
`ref const X`? Surely if there is no `ref X` and only a `ref const X`
available, it should choose that one rather than the byval one when the
argument is not const (as demonstrated in the bug report?)
It's demonstrated that `ref const X` might inhibit an efficient copy
constructor implementation in some cases, but you can't have false-move's
occurring when the lvalue is not const.

On Tuesday, 25 April 2017 at 01:59:55 UTC, Manu wrote:

> Ah crap, I somehow missed the single-argument move() function.
>
> Okay, so this pattern is definitely reliable? I haven't seen it clearly documented anywhere that this is the prescribed pattern, and it's come up on stack overflow a few times, but it hasn't been answered correctly.
> I think this is poorly distributed knowledge.

It is definitely reliable.

> As Schveighoffer pointed out, this pattern requires *another* overload for `ref const X`? Surely if there is no `ref X` and only a `ref const X` available, it should choose that one rather than the byval one when the argument is not const (as demonstrated in the bug report?)

No, the bug report is incorrect. See http://dlang.org/spec/function.html#function-overloading. Remember that, unlike C++'s '&', "ref" is not a type, it's a parameter storage class. So, given the overloads

foo(const ref X);
foo(X x);

or, more precisely:

foo(ref const(X));
foo(X x);

if you call foo with a non-const X lvalue *or* rvalue, the second overload will be chosen (the exact match):

X x;
foo(x);   // typeof(x) == X, so foo(X) is chosen. It's passed by value, so x is copied first.
foo(X()); // also calls foo(X), this time no copy is needed

const X cx;
foo(cx); // calls foo(ref const(X)), since typeof(cx) == const(X)

In C++, those foos would be ambiguous. In D, they aren't, because it first checks whether it's const or not, and then whether it's an lvalue or an rvalue.

> It's demonstrated that `ref const X` might inhibit an efficient copy constructor implementation in some cases, but you can't have false-move's occurring when the lvalue is not const.

It's not about efficiency, it's about preserving type information. Once you slap "const" on, there's no getting rid of it without violating the type system.
And there won't be any "false" moves. You either have a reference to copy from, or a full-blown copy/rvalue to move from.

On Monday, 24 April 2017 at 18:48:00 UTC, Stanislav Blinov wrote:
> Suddenly, we can't copy the pointer, or at least make a shallow copy of it, without violating const, and the latter is UB.

Because transitive const/immutable is shit. And that shit had to introduce another shit - Rebindable. D const/immutable system is very big PITA. I hate it.