From 373babe48e186d2e9a54042bd35317c928b14bc3 Mon Sep 17 00:00:00 2001 From: Steven Schveighoffer Date: Wed, 16 Aug 2017 11:08:42 -0400 Subject: [PATCH 1/2] fix issue 13262 - Ensure shared data can be sent and received via send/receive. Also now allows all types of shared items to be stored in a Variant. --- std/concurrency.d | 14 ++++++++++ std/traits.d | 10 +++++-- std/variant.d | 71 ++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/std/concurrency.d b/std/concurrency.d index cf77911a5df..2cd714c3aa8 100644 --- a/std/concurrency.d +++ b/std/concurrency.d @@ -2529,3 +2529,17 @@ auto ref initOnce(alias var)(lazy typeof(var) init, Mutex mutex) static assert(!__traits(compiles, initOnce!c(true))); // TLS static assert(!__traits(compiles, initOnce!d(true))); // local variable } + +// test ability to send shared arrays +@system unittest +{ + static shared int[] x = new shared(int)[1]; + auto tid = spawn({ + auto arr = receiveOnly!(shared(int)[]); + arr[0] = 5; + ownerTid.send(true); + }); + tid.send(x); + receiveOnly!(bool); + assert(x[0] == 5); +} diff --git a/std/traits.d b/std/traits.d index ce47cc3cf70..37629c20e5e 100644 --- a/std/traits.d +++ b/std/traits.d @@ -4596,8 +4596,14 @@ template ImplicitConversionTargets(T) else static if (is(T : Object)) alias ImplicitConversionTargets = TransitiveBaseTypeTuple!(T); else static if (isDynamicArray!T && !is(typeof(T.init[0]) == const)) - alias ImplicitConversionTargets = - AliasSeq!(const(Unqual!(typeof(T.init[0])))[]); + { + static if (is(typeof(T.init[0]) == shared)) + alias ImplicitConversionTargets = + AliasSeq!(const(shared(Unqual!(typeof(T.init[0]))))[]); + else + alias ImplicitConversionTargets = + AliasSeq!(const(Unqual!(typeof(T.init[0])))[]); + } else static if (is(T : void*)) alias ImplicitConversionTargets = AliasSeq!(void*); else diff --git a/std/variant.d b/std/variant.d index 574e2c5a375..93cd27b54f6 100644 --- a/std/variant.d +++ b/std/variant.d @@ -613,19 +613,21 @@ public: static if (T.sizeof <= size) { import core.stdc.string : memcpy; - // If T is a class we're only copying the reference, so it - // should be safe to cast away shared so the memcpy will work. + // rhs has already been copied onto the stack, so even if T is + // shared, it's not really shared. Therefore, we can safely + // remove the shared qualifier when copying, as we are only + // copying from the unshared stack. // - // TODO: If a shared class has an atomic reference then using - // an atomic load may be more correct. Just make sure - // to use the fastest approach for the load op. - static if (is(T == class) && is(T == shared)) - memcpy(&store, cast(const(void*)) &rhs, rhs.sizeof); - else - memcpy(&store, &rhs, rhs.sizeof); + // In addition, the storage location is not accessible outside + // the Variant, so even if shared data is stored there, it's + // not really shared, as it's copied out as well. + memcpy(&store, cast(const(void*)) &rhs, rhs.sizeof); static if (hasElaborateCopyConstructor!T) { - typeid(T).postblit(&store); + // Safer than using typeid's postblit function because it + // type-checks the postblit function against the qualifiers + // of the type. + (cast(T*)&store).__xpostblit(); } } else @@ -1375,6 +1377,55 @@ pure nothrow @nogc v.get!S; } +// issue 13262 +@system unittest +{ + static void fun(T)(Variant v){ + T x; + v = x; + auto r = v.get!(T); + } + Variant v; + fun!(shared(int))(v); + fun!(shared(int)[])(v); + + static struct S1 + { + int c; + string a; + } + + static struct S2 + { + string a; + shared int[] b; + } + + static struct S3 + { + string a; + shared int[] b; + int c; + } + + fun!(S1)(v); + fun!(shared(S1))(v); + fun!(S2)(v); + fun!(shared(S2))(v); + fun!(S3)(v); + fun!(shared(S3))(v); + + // ensure structs that are shared, but don't have shared postblits + // can't be used. + static struct S4 + { + int x; + this(this) {x = 0;} + } + + fun!(S4)(v); + static assert(!is(typeof(fun!(shared(S4))(v)))); +} /** _Algebraic data type restricted to a closed set of possible From db28e80a4fd546d463f072ea7ffc8df3a4e8d227 Mon Sep 17 00:00:00 2001 From: Steven Schveighoffer Date: Thu, 17 Aug 2017 09:26:03 -0400 Subject: [PATCH 2/2] Avoid using typeid to call postblit and destructor, as it does not take into account the type qualifiers. --- std/variant.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/variant.d b/std/variant.d index 93cd27b54f6..8b3baacd4c0 100644 --- a/std/variant.d +++ b/std/variant.d @@ -529,14 +529,14 @@ private: case OpID.postblit: static if (hasElaborateCopyConstructor!A) { - typeid(A).postblit(zis); + zis.__xpostblit(); } break; case OpID.destruct: static if (hasElaborateDestructor!A) { - typeid(A).destroy(zis); + zis.__xdtor(); } break;