From 7aa5850ed9f42060e404ebb3a993818226697e77 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Wed, 25 May 2016 12:07:29 +0100 Subject: [PATCH 1/8] Add std.typecons.Rebindable!struct --- std/typecons.d | 231 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 228 insertions(+), 3 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index d068cd82158..0a0d3f13321 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2337,11 +2337,11 @@ Convenience function for creating a `Rebindable` using automatic type inference. Params: - obj = A reference to an object, interface, associative array, or an array slice - to initialize the `Rebindable` with. + obj = A reference to an object, interface, associative array, or an array slice. + s = Struct instance. Returns: - A newly constructed `Rebindable` initialized with the given reference. + A newly constructed `Rebindable` initialized with the given data. */ Rebindable!T rebindable(T)(T obj) if (is(T == class) || is(T == interface) || isDynamicArray!T || isAssociativeArray!T) @@ -2351,6 +2351,17 @@ if (is(T == class) || is(T == interface) || isDynamicArray!T || isAssociativeArr return ret; } +/// ditto +Rebindable!S rebindable(S)(S s) +if (is(S == struct) && !isInstanceOf!(Rebindable, S)) +{ + // workaround for rebindableS = rebindable(s) + static if (isMutable!S) + return s; + else + return Rebindable!S(s); +} + /// @system unittest { @@ -2498,6 +2509,220 @@ Rebindable!T rebindable(T)(Rebindable!T obj) assert(rebindable(pr3341_aa)[321] == 543); } +/** Models safe reassignment of otherwise constant struct instances. + * + * A struct with a field of reference type cannot be assigned to a constant + * struct of the same type. `Rebindable!(const S)` allows assignment to + * a `const S` while enforcing only constant access to its fields. + * + * `Rebindable!(immutable S)` does the same but field access may create a + * temporary copy of `S` in order to enforce _true immutability. + */ +template Rebindable(S) +if (is(S == struct)) +{ + static if (isMutable!S) + alias Rebindable = S; + else + struct Rebindable + { + private: + // mutPayload's pointers must be treated as tail const + void[S.sizeof] mutPayload; + + void emplace(ref S s) + { + import std.conv : emplace; + static if (__traits(compiles, () @safe {S tmp = s;})) + () @trusted {emplace!S(mutPayload, s);}(); + else + emplace!S(mutPayload, s); + } + + public: + this()(auto ref S s) + { + emplace(s); + } + + // immutable S cannot be passed to auto ref S above + static if (!is(S == immutable)) + this()(immutable S s) + { + emplace(s); + } + + void opAssign()(auto ref S s) + { + movePayload; + emplace(s); + } + + static if (!is(S == immutable)) + void opAssign()(immutable S s) + { + movePayload; + emplace(s); + } + + void opAssign(Rebindable other) + { + this = other.trustedPayload; + } + + // must not escape when S is immutable + private + ref trustedPayload() @trusted + { + return *cast(S*)mutPayload.ptr; + } + + static if (!is(S == immutable)) + ref S Rebindable_getRef() @property + { + // payload exposed as const ref when S is const + return trustedPayload; + } + + static if (is(S == immutable)) + S Rebindable_get() @property + { + // we return a copy for immutable S + return trustedPayload; + } + + static if (is(S == immutable)) + alias Rebindable_get this; + else + alias Rebindable_getRef this; + + private + auto movePayload() @trusted + { + import std.algorithm : move; + return cast(S)move(*cast(Unqual!S*)mutPayload.ptr); + } + + ~this() + { + // call destructor with proper constness + movePayload; + } + } +} + +/// +@safe unittest +{ + static struct S + { + int* ptr; + } + S s = S(new int); + + const cs = s; + // Can't assign s.ptr to cs.ptr + static assert(!__traits(compiles, {s = cs;})); + + Rebindable!(const S) rs = s; + assert(rs.ptr is s.ptr); + // rs.ptr is const + static assert(!__traits(compiles, {rs.ptr = null;})); + + // Can't assign s.ptr to rs.ptr + static assert(!__traits(compiles, {s = rs;})); + + const S cs2 = rs; + // Rebind rs + rs = cs2; + rs = S(); + assert(rs.ptr is null); +} + +// Test Rebindable!immutable +@safe unittest +{ + static struct S + { + int* ptr; + } + S s = S(new int); + + Rebindable!(immutable S) ri = S(new int); + assert(ri.ptr !is null); + static assert(!__traits(compiles, {ri.ptr = null;})); + + // ri is not compatible with mutable S + static assert(!__traits(compiles, {s = ri;})); + static assert(!__traits(compiles, {ri = s;})); + + auto ri2 = ri; + assert(ri2.ptr == ri.ptr); + + const S cs3 = ri; + static assert(!__traits(compiles, {ri = cs3;})); + + immutable S si = ri; + // Rebind ri + ri = si; + ri = S(); + assert(ri.ptr is null); + + // Test RB!immutable -> RB!const + Rebindable!(const S) rc = ri; + assert(rc.ptr is null); + ri = S(new int); + rc = ri; + assert(rc.ptr !is null); + + // test rebindable, opAssign + rc.destroy; + assert(rc.ptr is null); + rc = rebindable(cs3); + rc = rebindable(si); + assert(rc.ptr !is null); + + ri.destroy; + assert(ri.ptr is null); + ri = rebindable(si); + assert(ri.ptr !is null); +} + +// Test Rebindable!mutable +@safe unittest +{ + static struct S + { + int* ptr; + } + S s; + + Rebindable!S rs = s; + static assert(is(typeof(rs) == S)); + rs = rebindable(S()); +} + +// Test disabled default ctor +unittest +{ + static struct ND + { + int i; + @disable this(); + this(int i) inout {this.i = i;} + } + static assert(!__traits(compiles, Rebindable!ND())); + + Rebindable!(const ND) rb = const ND(1); + assert(rb.i == 1); + rb = immutable ND(2); + assert(rb.i == 2); + rb = rebindable(const ND(3)); + assert(rb.i == 3); + static assert(!__traits(compiles, rb.i++)); +} + + /** Similar to `Rebindable!(T)` but strips all qualifiers from the reference as opposed to just constness / immutability. Primary intended use case is with From 909b1dc16a93a0f10c7ed1aebfefed5b66e39530 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Wed, 7 Feb 2018 02:23:40 +0100 Subject: [PATCH 2/8] style fixups --- std/typecons.d | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 0a0d3f13321..0fa9bd71564 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2571,21 +2571,20 @@ if (is(S == struct)) } // must not escape when S is immutable - private - ref trustedPayload() @trusted + private ref trustedPayload() @trusted { - return *cast(S*)mutPayload.ptr; + return *cast(S*) mutPayload.ptr; } static if (!is(S == immutable)) - ref S Rebindable_getRef() @property + private ref S Rebindable_getRef() @property { // payload exposed as const ref when S is const return trustedPayload; } static if (is(S == immutable)) - S Rebindable_get() @property + private S Rebindable_get() @property { // we return a copy for immutable S return trustedPayload; @@ -2596,11 +2595,10 @@ if (is(S == struct)) else alias Rebindable_getRef this; - private - auto movePayload() @trusted + private auto movePayload() @trusted { import std.algorithm : move; - return cast(S)move(*cast(Unqual!S*)mutPayload.ptr); + return cast(S) move(*cast(Unqual!S*) mutPayload.ptr); } ~this() From 49e0048b3da9bdf002f201a0fe59c3bc8191a202 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Wed, 7 Feb 2018 12:07:13 +0100 Subject: [PATCH 3/8] Add more test to Rebindable --- std/typecons.d | 104 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/std/typecons.d b/std/typecons.d index 0fa9bd71564..04bb8dc469d 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2701,7 +2701,7 @@ if (is(S == struct)) } // Test disabled default ctor -unittest +@safe unittest { static struct ND { @@ -2720,6 +2720,108 @@ unittest static assert(!__traits(compiles, rb.i++)); } +@safe unittest +{ + int del; + int post; + struct S + { + int* ptr; + int level; + this(this) { + post++; + level++; + } + ~this() { + del++; + } + } + + // test const + { + Rebindable!(const S) rc = S(new int); + assert(post == 1); + assert(rc.level == 1); + assert(post == 1); + assert(rc.level == 1); // no copy is created + } + assert(post == 1); + assert(del == 2); + del = 0, post = 0; + + // immutable + { + // on every field access a temporary immutable copy gets created + Rebindable!(immutable S) ri = S(new int); + assert(post == 1); + assert(ri.level == 2); + assert(post == 2); + assert(ri.level == 2); // however it's a copy of the payload's level + } + assert(post == 3); + assert(del == 4); + del = 0, post = 0; + + { + // the initial value is copied and destructed + Rebindable!(const S) rc = S(new int); + assert(post == 1); + assert(del == 1); + assert(rc.level == 1); + + // on an assignment to another value is simply post-blitted + const S cs = rc; + assert(del == 1); + assert(post == 2); + assert(rc.level == 1); + assert(cs.level == 2); + + // however on an assignment, the old payload gets destructed + rc = cs; + assert(del == 2); + assert(post == 3); + assert(cs.level == 2); + assert(rc.level == 3); + } + assert(post == 3); + assert(del == 4); + del = 0, post = 0; + + { + // the initial value is copied and destructed + Rebindable!(immutable S) ri = S(new int); + assert(post == 1); + assert(del == 1); + // creates temporary (gets immediately destroyed), actual level is still 1 + assert(ri.level == 2); + assert(del == 2); + assert(post == 2); + + // on an assignment to another value is simply post-blitted + const S cs = ri; + assert(del == 2); + assert(post == 3); + // creates temporary (gets immediately destroyed), actual level is still 1 + assert(ri.level == 2); + assert(cs.level == 2); + assert(del == 3); + assert(post == 4); + + // however, on an assignment to Rebindable the old value gets destructed + auto ri2 = ri; + static assert(is(typeof(ri2) == Rebindable!(immutable S))); + static assert(TemplateOf!(typeof(ri2)).stringof == "Rebindable(S) if (is(S == struct))"); + assert(del == 3); + assert(post == 4); + assert(ri2.level == 2); // should be 3?? + assert(del == 4); + + // similarly an assignment + //ri = ri2; // fails to compile due to overload conflicts + } + assert(post == 5); + assert(del == 7); +} /** Similar to `Rebindable!(T)` but strips all qualifiers from the reference as From 35307baef74fe712ea7f2d3fcaa73b5f471e7f06 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 26 Mar 2018 13:36:43 +0200 Subject: [PATCH 4/8] rename emplace -> emplacePayload --- std/typecons.d | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 04bb8dc469d..2f6f71f5d4d 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2530,7 +2530,7 @@ if (is(S == struct)) // mutPayload's pointers must be treated as tail const void[S.sizeof] mutPayload; - void emplace(ref S s) + void emplacePayload(ref S s) { import std.conv : emplace; static if (__traits(compiles, () @safe {S tmp = s;})) @@ -2542,27 +2542,27 @@ if (is(S == struct)) public: this()(auto ref S s) { - emplace(s); + emplacePayload(s); } // immutable S cannot be passed to auto ref S above static if (!is(S == immutable)) this()(immutable S s) { - emplace(s); + emplacePayload(s); } void opAssign()(auto ref S s) { movePayload; - emplace(s); + emplacePayload(s); } static if (!is(S == immutable)) void opAssign()(immutable S s) { movePayload; - emplace(s); + emplacePayload(s); } void opAssign(Rebindable other) From 83cdcee4fa91d59f0d8631912490652784b97aa9 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Thu, 29 Mar 2018 12:24:17 +0100 Subject: [PATCH 5/8] Remove `private` for alias this getters Otherwise alias this would not work outside std.typecons. --- std/typecons.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 2f6f71f5d4d..66329bef2fd 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2577,14 +2577,14 @@ if (is(S == struct)) } static if (!is(S == immutable)) - private ref S Rebindable_getRef() @property + ref S Rebindable_getRef() @property { // payload exposed as const ref when S is const return trustedPayload; } static if (is(S == immutable)) - private S Rebindable_get() @property + S Rebindable_get() @property { // we return a copy for immutable S return trustedPayload; From 78167ab5058405941b39e8b205f603955910de0c Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Thu, 29 Mar 2018 12:24:43 +0100 Subject: [PATCH 6/8] Fix & improve docs Wording was backward: `const S = S` is fine, `S = const S` disallowed. --- std/typecons.d | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 66329bef2fd..fce9493a9f4 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2511,10 +2511,13 @@ Rebindable!T rebindable(T)(Rebindable!T obj) /** Models safe reassignment of otherwise constant struct instances. * - * A struct with a field of reference type cannot be assigned to a constant - * struct of the same type. `Rebindable!(const S)` allows assignment to - * a `const S` while enforcing only constant access to its fields. + * A constant struct with a field of reference type cannot be assigned to a mutable + * struct of the same type. This protects the constant reference field from being + * mutably aliased, potentially allowing mutation of `immutable` data. However, the + * assignment could be safe if all reference fields are only exposed as `const`. * + * `Rebindable!(const S)` accepts assignment from + * a `const S` while enforcing only constant access to its fields. * `Rebindable!(immutable S)` does the same but field access may create a * temporary copy of `S` in order to enforce _true immutability. */ From 7405fe9658213ba762934c9c20826d338073bbba Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 23 Apr 2018 16:25:57 +0100 Subject: [PATCH 7/8] Add generic maxElement algorithm example --- std/typecons.d | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/std/typecons.d b/std/typecons.d index fce9493a9f4..31c5e8fdd31 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2640,6 +2640,36 @@ if (is(S == struct)) assert(rs.ptr is null); } +/// Using Rebindable in a generic algorithm: +@safe unittest +{ + import std.range.primitives : front, popFront; + + // simple version of std.algorithm.searching.maxElement + typeof(R.init.front) maxElement(R)(R r) + { + auto max = rebindable(r.front); + r.popFront; + foreach (e; r) + if (e > max) + max = e; // Rebindable allows const-correct reassignment + return max; + } + struct S + { + char[] arr; + alias arr this; // for comparison + } + // can't convert to mutable + const S cs; + static assert(!__traits(compiles, { S s = cs; })); + + alias CS = const S; + CS[] arr = [CS("harp"), CS("apple"), CS("pot")]; + CS ms = maxElement(arr); + assert(ms.arr == "pot"); +} + // Test Rebindable!immutable @safe unittest { From 11e6624f6cdecc34c43aa8f7eaca182104db9b95 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Wed, 11 Apr 2018 13:42:54 +0100 Subject: [PATCH 8/8] Copy for Rebindable!(const S) when S has head const fields Example of previous failing code: struct S { immutable int i; } Rebindable!(const S) r = const S(1); immutable int* p = &r.i; r = S(5); assert(*p == 1); //fails --- std/typecons.d | 79 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 65 insertions(+), 14 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 31c5e8fdd31..fe5847b38f8 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -2516,10 +2516,16 @@ Rebindable!T rebindable(T)(Rebindable!T obj) * mutably aliased, potentially allowing mutation of `immutable` data. However, the * assignment could be safe if all reference fields are only exposed as `const`. * - * `Rebindable!(const S)` accepts assignment from - * a `const S` while enforcing only constant access to its fields. - * `Rebindable!(immutable S)` does the same but field access may create a - * temporary copy of `S` in order to enforce _true immutability. + * `Rebindable!(const S)` accepts (re)assignment from + * a `const S` while enforcing only `const` access to its fields. + * It implicitly converts to `ref const S` when possible. Otherwise, + * a copy is made on each implicit conversion to `const S`. Copies must be made + * e.g. when the struct contains a head-immutable data field. + * + * `Rebindable!(immutable S)` always makes a copy on implicit conversion to + * `immutable S`. This is required to preserve `immutable`'s + * guarantees. A copy is currently always made for conversion to `const S` + * (due to `alias this` limitations). */ template Rebindable(S) if (is(S == struct)) @@ -2573,32 +2579,42 @@ if (is(S == struct)) this = other.trustedPayload; } - // must not escape when S is immutable - private ref trustedPayload() @trusted + import std.traits : Fields, isMutable; + private template isConst(T) + { + static if (is(T == struct) || is(T == union)) + enum isConst = !isMutable!T || haveConstHead!T; + else + enum isConst = !isMutable!T; + } + private enum bool haveConstHead(T) = anySatisfy!(isConst, Fields!T); + private enum bool unsafeRef = is(S == immutable) || haveConstHead!(Unqual!S); + + // must not escape when unsafeRef + private ref S trustedPayload() @trusted { return *cast(S*) mutPayload.ptr; } - static if (!is(S == immutable)) + // expose payload as const ref when members have no head const data + static if (!unsafeRef) ref S Rebindable_getRef() @property { - // payload exposed as const ref when S is const return trustedPayload; } - static if (is(S == immutable)) - S Rebindable_get() @property + static if (unsafeRef) + S Rebindable_getCopy() @property { - // we return a copy for immutable S return trustedPayload; } - static if (is(S == immutable)) - alias Rebindable_get this; + static if (unsafeRef) + alias Rebindable_getCopy this; else alias Rebindable_getRef this; - private auto movePayload() @trusted + private S movePayload() @trusted { import std.algorithm : move; return cast(S) move(*cast(Unqual!S*) mutPayload.ptr); @@ -2753,6 +2769,41 @@ if (is(S == struct)) static assert(!__traits(compiles, rb.i++)); } +// Test head const fields aren't exposed as ref const S +@safe unittest +{ + struct S(T) + { + T i; + } + auto r = rebindable(const S!(immutable int)()); + // check we can't reference `i`, as it's a field of a temporary S copy + // If r.i was an lvalue, it would break immutable when r was rebound + static assert(!__traits(compiles, {const ref get(){ return r.i; }})); + + // test when Rebindable!const should make a copy + @property rcs(T)(){ return rebindable(const S!T()); } + // values + static assert(!rcs!(int).unsafeRef); + static assert(rcs!(const int).unsafeRef); + static assert(rcs!(immutable int).unsafeRef); + // arrays + static assert(!rcs!(string).unsafeRef); + static assert(rcs!(const char[]).unsafeRef); + // class refs + static assert(!rcs!(Object).unsafeRef); + static assert(rcs!(const Object).unsafeRef); + // nested structs + static assert(!rcs!(S!int).unsafeRef); + static assert(!rcs!(S!string).unsafeRef); + static assert(!rcs!(S!Object).unsafeRef); + static assert(rcs!(const S!int).unsafeRef); + static assert(rcs!(const S!string).unsafeRef); + static assert(rcs!(S!(const Object)).unsafeRef); + static assert(rcs!(const S!Object).unsafeRef); +} + +// Test copying @safe unittest { int del;