From 35f3a0de577b1c6988d45d659bc7557d514d48c6 Mon Sep 17 00:00:00 2001
From: Vladimir Panteleev <git@cy.md>
Date: Thu, 4 Jan 2024 16:36:55 +0000
Subject: [PATCH 1/4] std.typecons: Refactor a unit test for clarity

---
 std/typecons.d | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/std/typecons.d b/std/typecons.d
index 67a1ede8148..64053bfe3f1 100644
--- a/std/typecons.d
+++ b/std/typecons.d
@@ -4088,16 +4088,16 @@ auto nullable(T)(T t)
 
     struct Test
     {
-        bool b;
+        bool initialized;
 
-        nothrow invariant { assert(b == true); }
+        nothrow invariant { assert(initialized); }
 
         SysTime _st;
 
         static bool destroyed;
 
         @disable this();
-        this(bool b) { this.b = b; }
+        this(int _dummy) { this.initialized = true; }
         ~this() @safe { destroyed = true; }
 
         // mustn't call opAssign on Test.init in Nullable!Test, because the invariant
@@ -4109,7 +4109,7 @@ auto nullable(T)(T t)
     {
         Nullable!Test nt;
 
-        nt = Test(true);
+        nt = Test(1);
 
         // destroy value
         Test.destroyed = false;

From aa39fa8f5bdb4121069830afff0013f58c109888 Mon Sep 17 00:00:00 2001
From: Vladimir Panteleev <git@cy.md>
Date: Thu, 4 Jan 2024 16:37:50 +0000
Subject: [PATCH 2/4] std.typecons: Allow calling destructor on default value
 in unittest

Generally, if a type has a default value, it should be safe to assume
that it's OK to call the destructor on a default-initialized variable.

The converse is not compatible with `move`, and goes against
implementing non-copyable types.
---
 std/typecons.d | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/std/typecons.d b/std/typecons.d
index 64053bfe3f1..1f57870b322 100644
--- a/std/typecons.d
+++ b/std/typecons.d
@@ -4088,16 +4088,12 @@ auto nullable(T)(T t)
 
     struct Test
     {
-        bool initialized;
-
-        nothrow invariant { assert(initialized); }
-
         SysTime _st;
 
         static bool destroyed;
 
         @disable this();
-        this(int _dummy) { this.initialized = true; }
+        this(int _dummy) {}
         ~this() @safe { destroyed = true; }
 
         // mustn't call opAssign on Test.init in Nullable!Test, because the invariant

From 70bef69a560c282a87eb48600c6bf539a6ae2b64 Mon Sep 17 00:00:00 2001
From: Vladimir Panteleev <git@cy.md>
Date: Thu, 4 Jan 2024 16:43:05 +0000
Subject: [PATCH 3/4] std.typecons: Avoid a copy in Nullable.opAssign

Use `move` directly.
---
 std/typecons.d | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/std/typecons.d b/std/typecons.d
index 1f57870b322..2b29858812f 100644
--- a/std/typecons.d
+++ b/std/typecons.d
@@ -3515,18 +3515,14 @@ struct Nullable(T)
     {
         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;

From 7033b5c0f0d923f20ba1dae350d143bd8fd92451 Mon Sep 17 00:00:00 2001
From: Vladimir Panteleev <git@cy.md>
Date: Thu, 4 Jan 2024 16:53:49 +0000
Subject: [PATCH 4/4] std.typecons: Support non-copyable types

Fixes issue 24318.
---
 std/typecons.d | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/std/typecons.d b/std/typecons.d
index 2b29858812f..13ae2941296 100644
--- a/std/typecons.d
+++ b/std/typecons.d
@@ -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)
     {
@@ -3273,6 +3281,9 @@ struct Nullable(T)
         }
     }
 
+    static if (!isCopyable!T)
+        @disable this(this);
+    else
     static if (__traits(hasPostblit, T))
     {
         this(this)
@@ -3511,7 +3522,7 @@ 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;
 
@@ -3600,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])
     {
@@ -10668,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