diff --git a/std/algorithm/searching.d b/std/algorithm/searching.d index 42a9df518..b7119d24b 100644 --- a/std/algorithm/searching.d +++ b/std/algorithm/searching.d @@ -3735,6 +3735,47 @@ if (isInputRange!Range && !isInfinite!Range && assert(arr.minElement!"a.val".val == 0); } +// https://issues.dlang.org/show_bug.cgi?id=24827 +@safe unittest +{ + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + bool opEquals()(auto ref S rhs) + { + return this.i == rhs.i; + } + + int opCmp()(auto ref S rhs) + { + if (this.i < rhs.i) + return -1; + + return this.i == rhs.i ? 0 : 1; + } + + @safe invariant + { + assert(!destroyed); + } + } + + auto arr = [S(19), S(2), S(145), S(7)]; + assert(minElement(arr) == S(2)); +} + /** Iterates the passed range and returns the maximal element. A custom mapping function can be passed to `map`. @@ -3888,6 +3929,47 @@ if (isInputRange!Range && !isInfinite!Range && assert(arr[0].getI == 2); } +// https://issues.dlang.org/show_bug.cgi?id=24827 +@safe unittest +{ + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + bool opEquals()(auto ref S rhs) + { + return this.i == rhs.i; + } + + int opCmp()(auto ref S rhs) + { + if (this.i < rhs.i) + return -1; + + return this.i == rhs.i ? 0 : 1; + } + + @safe invariant + { + assert(!destroyed); + } + } + + auto arr = [S(19), S(2), S(145), S(7)]; + assert(maxElement(arr) == S(145)); +} + // minPos /** Computes a subrange of `range` starting at the first occurrence of `range`'s diff --git a/std/typecons.d b/std/typecons.d index 3c425c7d7..368a5e5f5 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -3102,17 +3102,18 @@ private: { static if (useQualifierCast) { - this.data = cast() value; + static if (hasElaborateAssign!T) + { + import core.lifetime : copyEmplace; + copyEmplace(cast() value, this.data); + } + else + this.data = cast() value; } else { - // As we're escaping a copy of `value`, deliberately leak a copy: - static union DontCallDestructor - { - T value; - } - DontCallDestructor copy = DontCallDestructor(value); - this.data = *cast(Payload*) © + import core.lifetime : copyEmplace; + copyEmplace(cast() value, cast() *cast(T*) &this.data); } } @@ -3137,6 +3138,334 @@ package(std) Rebindable2!T rebindable2(T)(T value) return Rebindable2!T(value); } +// Verify that the destructor is called properly if there is one. +@system unittest +{ + { + bool destroyed; + + struct S + { + int i; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + } + + { + auto foo = rebindable2(S(42)); + + // Whether destruction has occurred here depends on whether the + // temporary gets moved or not, so we won't assume that it has or + // hasn't happened. What we care about here is that foo gets destroyed + // properly when it leaves the scope. + destroyed = false; + } + assert(destroyed); + + { + auto foo = rebindable2(const S(42)); + destroyed = false; + } + assert(destroyed); + } + + // Test for double destruction with qualifer cast being used + { + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + @safe invariant + { + assert(!destroyed); + } + } + + { + auto foo = rebindable2(S(42)); + assert(typeof(foo).useQualifierCast); + assert(foo.data.i == 42); + assert(!foo.data.destroyed); + } + { + auto foo = rebindable2(S(42)); + destroy(foo); + } + { + auto foo = rebindable2(const S(42)); + assert(typeof(foo).useQualifierCast); + assert(foo.data.i == 42); + assert(!foo.data.destroyed); + } + { + auto foo = rebindable2(const S(42)); + destroy(foo); + } + } + + // Test for double destruction without qualifer cast being used + { + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + @disable ref S opAssign()(auto ref S rhs); + + @safe invariant + { + assert(!destroyed); + } + } + + { + auto foo = rebindable2(S(42)); + assert(!typeof(foo).useQualifierCast); + assert((cast(S*)&(foo.data)).i == 42); + assert(!(cast(S*)&(foo.data)).destroyed); + } + { + auto foo = rebindable2(S(42)); + destroy(foo); + } + } +} + +// Verify that if there is an overloaded assignment operator, it's not assigned +// to garbage. +@safe unittest +{ + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + ref opAssign()(auto ref S rhs) + { + assert(!this.destroyed); + this.i = rhs.i; + return this; + } + } + + { + auto foo = rebindable2(S(42)); + foo = S(99); + assert(foo.data.i == 99); + } + { + auto foo = rebindable2(S(42)); + foo = const S(99); + assert(foo.data.i == 99); + } +} + +// Verify that postblit or copy constructor is called properly if there is one. +@system unittest +{ + // postblit with type qualifier cast + { + static struct S + { + int i; + static bool copied; + + this(this) @safe + { + copied = true; + } + } + + { + auto foo = rebindable2(S(42)); + + // Whether a copy has occurred here depends on whether the + // temporary gets moved or not, so we won't assume that it has or + // hasn't happened. What we care about here is that foo gets copied + // properly when we copy it below. + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + { + auto foo = rebindable2(const S(42)); + assert(typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + } + + // copy constructor with type qualifier cast + { + static struct S + { + int i; + static bool copied; + + this(ref inout S rhs) @safe inout + { + this.i = i; + copied = true; + } + } + + { + auto foo = rebindable2(S(42)); + assert(typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + { + auto foo = rebindable2(const S(42)); + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + } + + // FIXME https://issues.dlang.org/show_bug.cgi?id=24829 + + // Making this work requires either reworking how the !useQualiferCast + // version works so that the compiler can correctly generate postblit + // constructors and copy constructors as appropriate, or an explicit + // postblit or copy constructor needs to be added for such cases, which + // gets pretty complicated if we want to correctly add the same attributes + // that T's postblit or copy constructor has. + + /+ + // postblit without type qualifier cast + { + static struct S + { + int* ptr; + static bool copied; + + this(int i) + { + ptr = new int(i); + } + + this(this) @safe + { + if (ptr !is null) + ptr = new int(*ptr); + copied = true; + } + + @disable ref S opAssign()(auto ref S rhs); + } + + { + auto foo = rebindable2(S(42)); + assert(!typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + { + auto foo = rebindable2(const S(42)); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + } + + // copy constructor without type qualifier cast + { + static struct S + { + int* ptr; + static bool copied; + + this(int i) + { + ptr = new int(i); + } + + this(ref inout S rhs) @safe inout + { + if (rhs.ptr !is null) + ptr = new inout int(*rhs.ptr); + copied = true; + } + + @disable ref S opAssign()(auto ref S rhs); + } + + { + auto foo = rebindable2(S(42)); + assert(!typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + { + auto foo = rebindable2(const S(42)); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + } + +/ +} + /** Similar to `Rebindable!(T)` but strips all qualifiers from the reference as opposed to just constness / immutability. Primary intended use case is with