From a6a6870beb69537f2ccd3d50289594ebae30ad15 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Fri, 10 Apr 2015 05:16:54 +0200 Subject: [PATCH 1/3] fix Issue 14432 - move construction for RefCounted - add RefCounted!T.this(T) which takes an RValue or a copy and use move to initialized the refcounted store - add refCounted to infer T from the argument and disable RefCounted's autoInit, also move initializes the store --- std/typecons.d | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/std/typecons.d b/std/typecons.d index d7a490d527d..2083109f626 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -4256,6 +4256,49 @@ if (!is(T == class) && !(is(T == interface))) _store._count = 1; } + private void move(ref T source) + { + import core.memory : GC; + import core.stdc.stdlib : malloc; + import core.stdc.string : memcpy; + import std.exception : enforce; + + _store = cast(Impl*) enforce(malloc(Impl.sizeof)); + 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) + { + import std.algorithm.searching : endsWith; + + 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); + } + else + { + memcpy(&source, &empty, T.sizeof); + } + } + + _store._count = 1; + } + /** Returns $(D true) if and only if the underlying store has been allocated and initialized. @@ -4305,6 +4348,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 +4566,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 From 74a219976016a9c7090488dc4a2c2b33bbf08fc4 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Fri, 10 Apr 2015 05:20:42 +0200 Subject: [PATCH 2/3] use onOutOfMemoryError instead of enforce --- std/typecons.d | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 2083109f626..ceb1bfe050a 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -4244,12 +4244,14 @@ 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); @@ -4258,12 +4260,14 @@ if (!is(T == class) && !(is(T == interface))) private void move(ref T source) { + import core.exception : onOutOfMemoryError; import core.memory : GC; import core.stdc.stdlib : malloc; import core.stdc.string : memcpy; - 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); From 1e19a4f7135e518cb6a445d93057ee03ca57e80f Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Mon, 20 Apr 2015 00:53:04 +0200 Subject: [PATCH 3/3] use typeid(T).init() to reset moved value --- std/algorithm/mutation.d | 45 ++++++++++++++++++---------------------- std/typecons.d | 22 +++++++++----------- 2 files changed, 30 insertions(+), 37 deletions(-) diff --git a/std/algorithm/mutation.d b/std/algorithm/mutation.d index 616f1bba764..1af2e6e8665 100644 --- a/std/algorithm/mutation.d +++ b/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); } - diff --git a/std/typecons.d b/std/typecons.d index ceb1bfe050a..0cad41a4921 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -4263,7 +4263,7 @@ if (!is(T == class) && !(is(T == interface))) import core.exception : onOutOfMemoryError; import core.memory : GC; import core.stdc.stdlib : malloc; - import core.stdc.string : memcpy; + import core.stdc.string : memcpy, memset; _store = cast(Impl*)malloc(Impl.sizeof); if (_store is null) @@ -4285,19 +4285,17 @@ if (!is(T == class) && !(is(T == interface))) // 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); } _store._count = 1;