fix Issue 14432 - move construction for RefCounted

std/algorithm/mutation.d

@@ -833,7 +833,7 @@ Params:
 */
 void move(T)(ref T source, ref T target)
 {
-    import core.stdc.string : memcpy;
+    import core.stdc.string : memcpy, memset;
     import std.traits : hasAliasing, hasElaborateAssign,
                         hasElaborateCopyConstructor, hasElaborateDestructor,
                         isAssignable;
@@ -860,19 +860,17 @@ void move(T)(ref T source, ref T target)
         // object in order to avoid double freeing and undue aliasing
         static if (hasElaborateDestructor!T || hasElaborateCopyConstructor!T)
         {
-            import std.algorithm.searching : endsWith;
+            // 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 T empty;
-            static if (T.tupleof.length > 0 &&
-                       T.tupleof[$-1].stringof.endsWith("this"))
-            {
-                // If T is nested struct, keep original context pointer
-                memcpy(&source, &empty, T.sizeof - (void*).sizeof);
-            }
+            auto init = typeid(T).init();
+            if (init.ptr is null) // null ptr means initialize to 0s
+                memset(&source, 0, sz);
             else
-            {
-                memcpy(&source, &empty, T.sizeof);
-            }
+                memcpy(&source, init.ptr, sz);
         }
     }
     else
@@ -992,7 +990,7 @@ unittest
 /// Ditto
 T move(T)(ref T source)
 {
-    import core.stdc.string : memcpy;
+    import core.stdc.string : memcpy, memset;
     import std.traits : hasAliasing, hasElaborateAssign,
                         hasElaborateCopyConstructor, hasElaborateDestructor,
                         isAssignable;
@@ -1016,19 +1014,17 @@ T move(T)(ref T source)
         // object in order to avoid double freeing and undue aliasing
         static if (hasElaborateDestructor!T || hasElaborateCopyConstructor!T)
         {
-            import std.algorithm.searching : endsWith;
+            // 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 T empty;
-            static if (T.tupleof.length > 0 &&
-                       T.tupleof[$-1].stringof.endsWith("this"))
-            {
-                // If T is nested struct, keep original context pointer
-                memcpy(&source, &empty, T.sizeof - (void*).sizeof);
-            }
+            auto init = typeid(T).init();
+            if (init.ptr is null) // null ptr means initialize to 0s
+                memset(&source, 0, sz);
             else
-            {
-                memcpy(&source, &empty, T.sizeof);
-            }
+                memcpy(&source, init.ptr, sz);
         }
     }
     else
@@ -2255,4 +2251,3 @@ void uninitializedFill(Range, Value)(Range range, Value value)
         // Doesn't matter whether fill is initialized or not
         return fill(range, value);
 }
-

std/typecons.d

@@ -4244,18 +4244,63 @@ if (!is(T == class) && !(is(T == interface)))
 
         private void initialize(A...)(auto ref A args)
         {
+            import core.exception : onOutOfMemoryError;
             import core.memory : GC;
             import core.stdc.stdlib : malloc;
             import std.conv : emplace;
-            import std.exception : enforce;
 
-            _store = cast(Impl*) enforce(malloc(Impl.sizeof));
+            _store = cast(Impl*)malloc(Impl.sizeof);
+            if (_store is null)
+                onOutOfMemoryError();
             static if (hasIndirections!T)
                 GC.addRange(&_store._payload, T.sizeof);
             emplace(&_store._payload, args);
             _store._count = 1;
         }
 
+        private void move(ref T source)
+        {
+            import core.exception : onOutOfMemoryError;
+            import core.memory : GC;
+            import core.stdc.stdlib : malloc;
+            import core.stdc.string : memcpy, memset;
+
+            _store = cast(Impl*)malloc(Impl.sizeof);
+            if (_store is null)
+                onOutOfMemoryError();
+            static if (hasIndirections!T)
+                GC.addRange(&_store._payload, T.sizeof);
+
+            // Can't use std.algorithm.move(source, _store._payload)
+            // here because it requires the target to be initialized.
+            // Might be worth to add this as `moveEmplace`
+
+            // Can avoid destructing result.
+            static if (hasElaborateAssign!T || !isAssignable!T)
+                memcpy(&_store._payload, &source, T.sizeof);
+            else
+                _store._payload = 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;
+
+                auto init = typeid(T).init();
+                if (init.ptr is null) // null ptr means initialize to 0s
+                    memset(&source, 0, sz);
+                else
+                    memcpy(&source, init.ptr, sz);
+            }
+
+            _store._count = 1;
+        }
+
         /**
            Returns $(D true) if and only if the underlying store has been
            allocated and initialized.
@@ -4305,6 +4350,12 @@ Postcondition: $(D refCountedStore.isInitialized)
         _refCounted.initialize(args);
     }
 
+    /// Ditto
+    this(T val)
+    {
+        _refCounted.move(val);
+    }
+
 /**
 Constructor that tracks the reference count appropriately. If $(D
 !refCountedStore.isInitialized), does nothing.
@@ -4517,6 +4568,51 @@ unittest
     assert(b == 5);
 }
 
+/**
+ * Initializes a `RefCounted` with `val`. The template parameter
+ * `T` of `RefCounted` is inferred from `val`.
+ * This function can be used to move non-copyable values to the heap.
+ * It also disables the `autoInit` option of `RefCounted`.
+ *
+ * Params:
+ *   val = The value to be reference counted
+ * Returns:
+ *   An initialized $(D RefCounted) containing $(D val).
+ * See_Also:
+ *   $(WEB http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared, C++'s make_shared)
+ */
+RefCounted!(T, RefCountedAutoInitialize.no) refCounted(T)(T val)
+{
+    typeof(return) res;
+    res._refCounted.move(val);
+    return res;
+}
+
+///
+unittest
+{
+    static struct File
+    {
+        string name;
+        @disable this(this); // not copyable
+        ~this() { name = null; }
+    }
+
+    auto file = File("name");
+    assert(file.name == "name");
+    // file cannot be copied and has unique ownership
+    static assert(!__traits(compiles, {auto file2 = file;}));
+
+    // make the file refcounted to share ownership
+    import std.algorithm.mutation : move;
+    auto rcFile = refCounted(move(file));
+    assert(rcFile.name == "name");
+    assert(file.name == null);
+    auto rcFile2 = rcFile;
+    assert(rcFile.refCountedStore.refCount == 2);
+    // file gets properly closed when last reference is dropped
+}
+
 /**
     Creates a proxy for the value `a` that will forward all operations
     while disabling implicit conversions. The aliased item `a` must be