Skip to content

Fix Issue 24318 - Nullable should support non-copyable objects #8874

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

Merged
thewilsonator merged 4 commits into from
Jan 4, 2024
Merged

Fix Issue 24318 - Nullable should support non-copyable objects #8874

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35f3a0d
std.typecons: Refactor a unit test for clarity
CyberShadow Jan 4, 2024
aa39fa8
std.typecons: Allow calling destructor on default value in unittest
CyberShadow Jan 4, 2024
70bef69
std.typecons: Avoid a copy in Nullable.opAssign
CyberShadow Jan 4, 2024
7033b5c
std.typecons: Support non-copyable types
CyberShadow Jan 4, 2024
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
56 changes: 38 additions & 18 deletions std/typecons.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3256,11 +3256,19 @@ struct Nullable(T)
* Params:
* value = The value to initialize this `Nullable` with.
*/
this(inout T value) inout
{
_value.payload = value;
_isNull = false;
}
static if (isCopyable!T)
this(inout T value) inout
{
_value.payload = value;
_isNull = false;
}
else
this(T value) inout
{
import std.algorithm.mutation : move;
_value.payload = move(value);
_isNull = false;
}

static if (hasElaborateDestructor!T)
{
Expand All @@ -3273,6 +3281,9 @@ struct Nullable(T)
}
}

static if (!isCopyable!T)
@disable this(this);
else
static if (__traits(hasPostblit, T))
{
this(this)
Expand Down Expand Up @@ -3511,22 +3522,18 @@ struct Nullable(T)
* Params:
* value = A value of type `T` to assign to this `Nullable`.
*/
Nullable opAssign()(T value)
ref Nullable opAssign()(T value) return
{
import std.algorithm.mutation : moveEmplace, move;

// the lifetime of the value in copy shall be managed by
// this Nullable, so we must avoid calling its destructor.
auto copy = DontCallDestructorT(value);

if (_isNull)
{
// trusted since payload is known to be uninitialized.
() @trusted { moveEmplace(copy.payload, _value.payload); }();
() @trusted { moveEmplace(value, _value.payload); }();
}
else
{
move(copy.payload, _value.payload);
move(value, _value.payload);
}
_isNull = false;
return this;
Expand Down Expand Up @@ -3604,12 +3611,14 @@ struct Nullable(T)
alias back = front;

/// ditto
static if (isCopyable!T)
@property inout(typeof(this)) save() inout
{
return this;
}

/// ditto
static if (isCopyable!T)
inout(typeof(this)) opIndex(size_t[2] dim) inout
in (dim[0] <= length && dim[1] <= length && dim[1] >= dim[0])
{
Expand Down Expand Up @@ -4088,16 +4097,12 @@ auto nullable(T)(T t)

struct Test
{
bool b;

nothrow invariant { assert(b == true); }

SysTime _st;

static bool destroyed;

@disable this();
this(bool b) { this.b = b; }
this(int _dummy) {}
~this() @safe { destroyed = true; }

// mustn't call opAssign on Test.init in Nullable!Test, because the invariant
Expand All @@ -4109,7 +4114,7 @@ auto nullable(T)(T t)
{
Nullable!Test nt;

nt = Test(true);
nt = Test(1);

// destroy value
Test.destroyed = false;
Expand Down Expand Up @@ -10676,6 +10681,21 @@ unittest
assert(s2.get().b == 3);
}

// https://issues.dlang.org/show_bug.cgi?id=24318
@system unittest
{
static struct S
{
@disable this(this);
int i;
}

Nullable!S s = S(1);
assert(s.get().i == 1);
s = S(2);
assert(s.get().i == 2);
}

/// The old version of $(LREF SafeRefCounted), before $(LREF borrow) existed.
/// Old code may be relying on `@safe`ty of some of the member functions which
/// cannot be safe in the new scheme, and
Expand Down