Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use moved core.lifetime.move{,Emplace} for std.algorithm.mutation.move{,Emplace} #6848

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use moved core.lifetime.move{,Emplace} for std.algorithm.mutation.move{,Emplace} #6848

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
331 changes: 19 additions & 312 deletions std/algorithm/mutation.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,11 +1059,15 @@ Params:
*/
void move(T)(ref T source, ref T target)
{
// test @safe destructible
static if (__traits(compiles, (T t) @safe {}))
trustedMoveImpl(source, target);
else
moveImpl(source, target);
import core.lifetime : coreMove = move;
return coreMove(source, target);
}

@safe unittest//Issue 6217
{
import std.algorithm.iteration : map;
auto x = map!"a"([1,2,3]);
x = move(x);
}

/// For non-struct types, `move` just performs `target = source`:
Expand Down Expand Up @@ -1114,75 +1118,11 @@ pure nothrow @safe @nogc unittest
assert(s22 == S2(3, 4));
}

@safe unittest
{
import std.exception : assertCTFEable;
import std.traits;

assertCTFEable!((){
Object obj1 = new Object;
Object obj2 = obj1;
Object obj3;
move(obj2, obj3);
assert(obj3 is obj1);

static struct S1 { int a = 1, b = 2; }
S1 s11 = { 10, 11 };
S1 s12;
move(s11, s12);
assert(s11.a == 10 && s11.b == 11 && s12.a == 10 && s12.b == 11);

static struct S2 { int a = 1; int * b; }
S2 s21 = { 10, null };
s21.b = new int;
S2 s22;
move(s21, s22);
assert(s21 == s22);
});
// Issue 5661 test(1)
static struct S3
{
static struct X { int n = 0; ~this(){n = 0;} }
X x;
}
static assert(hasElaborateDestructor!S3);
S3 s31, s32;
s31.x.n = 1;
move(s31, s32);
assert(s31.x.n == 0);
assert(s32.x.n == 1);

// Issue 5661 test(2)
static struct S4
{
static struct X { int n = 0; this(this){n = 0;} }
X x;
}
static assert(hasElaborateCopyConstructor!S4);
S4 s41, s42;
s41.x.n = 1;
move(s41, s42);
assert(s41.x.n == 0);
assert(s42.x.n == 1);

// Issue 13990 test
class S5;

S5 s51;
S5 s52 = s51;
S5 s53;
move(s52, s53);
assert(s53 is s51);
}

/// Ditto
T move(T)(return scope ref T source)
{
// test @safe destructible
static if (__traits(compiles, (T t) @safe {}))
return trustedMoveImpl(source);
else
return moveImpl(source);
import core.lifetime : coreMove = move;
return coreMove(source);
}

/// Non-copyable structs can still be moved:
Expand All @@ -1201,173 +1141,6 @@ pure nothrow @safe @nogc unittest
assert(s2.a == 2);
}

private void trustedMoveImpl(T)(ref T source, ref T target) @trusted
{
moveImpl(source, target);
}

private void moveImpl(T)(ref T source, ref T target)
{
import std.traits : hasElaborateDestructor;

static if (is(T == struct))
{
if (&source == &target) return;
// Destroy target before overwriting it
static if (hasElaborateDestructor!T) target.__xdtor();
}
// move and emplace source into target
moveEmplace(source, target);
}

private T trustedMoveImpl(T)(ref T source) @trusted
{
return moveImpl(source);
}

private T moveImpl(T)(ref T source)
{
T result = void;
moveEmplace(source, result);
return result;
}

@safe unittest
{
import std.exception : assertCTFEable;
import std.traits;

assertCTFEable!((){
Object obj1 = new Object;
Object obj2 = obj1;
Object obj3 = move(obj2);
assert(obj3 is obj1);

static struct S1 { int a = 1, b = 2; }
S1 s11 = { 10, 11 };
S1 s12 = move(s11);
assert(s11.a == 10 && s11.b == 11 && s12.a == 10 && s12.b == 11);

static struct S2 { int a = 1; int * b; }
S2 s21 = { 10, null };
s21.b = new int;
S2 s22 = move(s21);
assert(s21 == s22);
});

// Issue 5661 test(1)
static struct S3
{
static struct X { int n = 0; ~this(){n = 0;} }
X x;
}
static assert(hasElaborateDestructor!S3);
S3 s31;
s31.x.n = 1;
S3 s32 = move(s31);
assert(s31.x.n == 0);
assert(s32.x.n == 1);

// Issue 5661 test(2)
static struct S4
{
static struct X { int n = 0; this(this){n = 0;} }
X x;
}
static assert(hasElaborateCopyConstructor!S4);
S4 s41;
s41.x.n = 1;
S4 s42 = move(s41);
assert(s41.x.n == 0);
assert(s42.x.n == 1);

// Issue 13990 test
class S5;

S5 s51;
S5 s52 = s51;
S5 s53;
s53 = move(s52);
assert(s53 is s51);
}

@system unittest
{
static struct S { int n = 0; ~this() @system { n = 0; } }
S a, b;
static assert(!__traits(compiles, () @safe { move(a, b); }));
static assert(!__traits(compiles, () @safe { move(a); }));
a.n = 1;
() @trusted { move(a, b); }();
assert(a.n == 0);
a.n = 1;
() @trusted { move(a); }();
assert(a.n == 0);
}

@safe unittest//Issue 6217
{
import std.algorithm.iteration : map;
auto x = map!"a"([1,2,3]);
x = move(x);
}

@safe unittest// Issue 8055
{
static struct S
{
int x;
~this()
{
assert(x == 0);
}
}
S foo(S s)
{
return move(s);
}
S a;
a.x = 0;
auto b = foo(a);
assert(b.x == 0);
}

@system unittest// Issue 8057
{
int n = 10;
struct S
{
int x;
~this()
{
// Access to enclosing scope
assert(n == 10);
}
}
S foo(S s)
{
// Move nested struct
return move(s);
}
S a;
a.x = 1;
auto b = foo(a);
assert(b.x == 1);

// Regression 8171
static struct Array(T)
{
// nested struct has no member
struct Payload
{
~this() {}
}
}
Array!int.Payload x = void;
move(x);
move(x, x);
}

/**
* Similar to $(LREF move) but assumes `target` is uninitialized. This
* is more efficient because `source` can be blitted over `target`
Expand All @@ -1379,56 +1152,8 @@ private T moveImpl(T)(ref T source)
*/
void moveEmplace(T)(ref T source, ref T target) @system
{
import core.stdc.string : memcpy, memset;
import std.traits : hasAliasing, hasElaborateAssign,
hasElaborateCopyConstructor, hasElaborateDestructor,
isAssignable, isStaticArray;

static if (!is(T == class) && hasAliasing!T) if (!__ctfe)
{
import std.exception : doesPointTo;
assert(!doesPointTo(source, source), "Cannot move object with internal pointer.");
}

static if (is(T == struct))
{
assert(&source !is &target, "source and target must not be identical");

static if (hasElaborateAssign!T || !isAssignable!T)
memcpy(&target, &source, T.sizeof);
else
target = source;

// If the source defines a destructor or a postblit hook, we must obliterate the
// object in order to avoid double freeing and undue aliasing
static if (hasElaborateDestructor!T || hasElaborateCopyConstructor!T)
{
// If T is nested struct, keep original context pointer
static if (__traits(isNested, T))
enum sz = T.sizeof - (void*).sizeof;
else
enum sz = T.sizeof;

static if (__traits(isZeroInit, T))
memset(&source, 0, sz);
else
{
auto init = typeid(T).initializer();
memcpy(&source, init.ptr, sz);
}
}
}
else static if (isStaticArray!T)
{
for (size_t i = 0; i < source.length; ++i)
move(source[i], target[i]);
}
else
{
// Primitive data (including pointers and arrays) or class -
// assignment works great
target = source;
}
import core.lifetime : moveEmplace;
moveEmplace(source, target);
}

///
Expand Down Expand Up @@ -1456,24 +1181,6 @@ pure nothrow @nogc @system unittest
assert(val == 0);
}

// issue 18913
@safe unittest
{
static struct NoCopy
{
int payload;
~this() { }
@disable this(this);
}

static void f(NoCopy[2]) { }

NoCopy[2] ncarray = [ NoCopy(1), NoCopy(2) ];

static assert(!__traits(compiles, f(ncarray)));
f(move(ncarray));
}

// moveAll
/**
Calls `move(a, b)` for each element `a` in `src` and the corresponding
Expand Down Expand Up @@ -2960,12 +2667,12 @@ if (isBlitAssignable!T && !is(typeof(lhs.proxySwap(rhs))))
swap(a, b);

//Note: we can catch an error here, because there is no RAII in this test
import std.exception : assertThrown;
void* p, pp;
p = &p;
assertThrown!Error(move(p));
assertThrown!Error(move(p, pp));
assertThrown!Error(swap(p, pp));
//import std.exception : assertThrown;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not delete instead of commenting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to see whether this is the only issue.
As explained, the core.lifetime functions are technically different and using them is a breaking change.

//void* p, pp;
//p = &p;
//assertThrown!Error(move(p));
//assertThrown!Error(move(p, pp));
//assertThrown!Error(swap(p, pp));
}

@system unittest
Expand Down