From d5093d1afb08b5c3cdb994eb144a8d9ce1464271 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Wed, 25 May 2016 12:07:29 +0100 Subject: [PATCH 01/13] Add std.typecons.Rebindable!struct --- std/typecons.d | 166 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 163 insertions(+), 3 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index bdec03e8a0c..06041702d4c 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1666,11 +1666,11 @@ Convenience function for creating a $(D 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) @@ -1680,6 +1680,17 @@ Rebindable!T rebindable(T)(T obj) 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); +} + /** This function simply returns the $(D Rebindable) object passed in. It's useful in generic programming cases when a given object may be either a regular @@ -1787,6 +1798,155 @@ unittest 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 + { + // fields of payload must be treated as tail const (unless S is mutable) + private Unqual!S payload; + + this()(S s) @trusted + { + // we preserve tail immutable guarantees so cast is OK + payload = cast(Unqual!S)s; + } + + void opAssign()(S s) + { + this = Rebindable(s); + } + + static if (!is(S == immutable)) + ref S Rebindable_getRef() @property + { + // payload exposed as const ref when S is const + return payload; + } + + static if (is(S == immutable)) + S Rebindable_get() @property @trusted + { + // we return a copy so cast to immutable is OK + return cast(S)payload; + } + + static if (is(S == immutable)) + alias Rebindable_get this; + else + alias Rebindable_getRef this; + + ~this() @trusted + { + import std.algorithm : move; + // call destructor with proper constness + S s = cast(S)move(payload); + } + } +} + +/// +@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;})); + + 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 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 = ND(1); + assert(rb.i == 1); + rb = immutable ND(2); + assert(rb.i == 2); + rb = rebindable(ND(3)); + assert(rb.i == 3); + static assert(!__traits(compiles, rb.i++)); +} + + /** Similar to $(D Rebindable!(T)) but strips all qualifiers from the reference as opposed to just constness / immutability. Primary intended use case is with From f9599c67c714e8ed1247c8e2ceb092b1975afcd0 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Sun, 29 May 2016 19:15:03 +0100 Subject: [PATCH 02/13] Fix possible tail mutation with mutable S.opAssign --- std/typecons.d | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 06041702d4c..166bc749951 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1815,13 +1815,17 @@ if (is(S == struct)) else struct Rebindable { - // fields of payload must be treated as tail const (unless S is mutable) - private Unqual!S payload; + // fields of mutPayload must be treated as tail const + private union + { + Unqual!S mutPayload = void; + S payload; + } this()(S s) @trusted { - // we preserve tail immutable guarantees so cast is OK - payload = cast(Unqual!S)s; + // we preserve tail immutable guarantees so mutable union is OK + payload = s; } void opAssign()(S s) @@ -1830,7 +1834,7 @@ if (is(S == struct)) } static if (!is(S == immutable)) - ref S Rebindable_getRef() @property + ref S Rebindable_getRef() @property @trusted { // payload exposed as const ref when S is const return payload; @@ -1839,8 +1843,8 @@ if (is(S == struct)) static if (is(S == immutable)) S Rebindable_get() @property @trusted { - // we return a copy so cast to immutable is OK - return cast(S)payload; + // we return a copy so mutable union is OK + return payload; } static if (is(S == immutable)) @@ -1852,7 +1856,7 @@ if (is(S == struct)) { import std.algorithm : move; // call destructor with proper constness - S s = cast(S)move(payload); + S s = cast(S)move(mutPayload); } } } From d6523f112c67f41b85b902e1cf5bcd8ba77d7bd1 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 30 May 2016 11:30:14 +0100 Subject: [PATCH 03/13] Fix opAssign using swap --- std/typecons.d | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 166bc749951..3b4d1b2df63 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1822,15 +1822,17 @@ if (is(S == struct)) S payload; } - this()(S s) @trusted + this(S s) @trusted { // we preserve tail immutable guarantees so mutable union is OK payload = s; } - void opAssign()(S s) + void opAssign(S s) @trusted { - this = Rebindable(s); + auto rs = Rebindable(s); + import std.algorithm.mutation : swap; + swap(mutPayload, rs.mutPayload); } static if (!is(S == immutable)) From b8282375402c5a8b54e2eeb2dc5fc31dc9f8864d Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 30 May 2016 11:41:47 +0100 Subject: [PATCH 04/13] Use local @trusted attribute --- std/typecons.d | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 3b4d1b2df63..5c4aff30378 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1821,14 +1821,16 @@ if (is(S == struct)) Unqual!S mutPayload = void; S payload; } + + @trusted: - this(S s) @trusted + this(S s) { // we preserve tail immutable guarantees so mutable union is OK payload = s; } - void opAssign(S s) @trusted + void opAssign(S s) { auto rs = Rebindable(s); import std.algorithm.mutation : swap; @@ -1836,14 +1838,14 @@ if (is(S == struct)) } static if (!is(S == immutable)) - ref S Rebindable_getRef() @property @trusted + ref S Rebindable_getRef() @property { // payload exposed as const ref when S is const return payload; } static if (is(S == immutable)) - S Rebindable_get() @property @trusted + S Rebindable_get() @property { // we return a copy so mutable union is OK return payload; @@ -1854,7 +1856,7 @@ if (is(S == struct)) else alias Rebindable_getRef this; - ~this() @trusted + ~this() { import std.algorithm : move; // call destructor with proper constness From 90a1306d197d914bb0281cbd022320e8d8997b83 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 30 May 2016 11:46:39 +0100 Subject: [PATCH 05/13] Add opAssign(typeof(this)), tests --- std/typecons.d | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/std/typecons.d b/std/typecons.d index 5c4aff30378..4412e38605e 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1821,7 +1821,7 @@ if (is(S == struct)) Unqual!S mutPayload = void; S payload; } - + @trusted: this(S s) @@ -1837,6 +1837,11 @@ if (is(S == struct)) swap(mutPayload, rs.mutPayload); } + void opAssign(typeof(this) other) + { + this = other.payload; + } + static if (!is(S == immutable)) ref S Rebindable_getRef() @property { @@ -1918,6 +1923,25 @@ if (is(S == struct)) 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 From 08a95dcdbf8b1a118f480f2c81f32bc1334fd45f Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 30 May 2016 15:55:01 +0100 Subject: [PATCH 06/13] Don't use @trusted where S.this, S.opAssign, S.~this are @system --- std/typecons.d | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 4412e38605e..de30f2b1345 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1822,35 +1822,33 @@ if (is(S == struct)) S payload; } - @trusted: - this(S s) { // we preserve tail immutable guarantees so mutable union is OK payload = s; } - void opAssign(S s) + void opAssign(S s) @trusted { auto rs = Rebindable(s); import std.algorithm.mutation : swap; swap(mutPayload, rs.mutPayload); } - void opAssign(typeof(this) other) + void opAssign(typeof(this) other) @trusted { this = other.payload; } static if (!is(S == immutable)) - ref S Rebindable_getRef() @property + ref S Rebindable_getRef() @property @trusted { // payload exposed as const ref when S is const return payload; } static if (is(S == immutable)) - S Rebindable_get() @property + S Rebindable_get() @property @trusted { // we return a copy so mutable union is OK return payload; @@ -1861,11 +1859,17 @@ if (is(S == struct)) else alias Rebindable_getRef this; - ~this() + private + auto movePayload() @trusted { import std.algorithm : move; + return cast(S)move(mutPayload); + } + + ~this() + { // call destructor with proper constness - S s = cast(S)move(mutPayload); + S s = movePayload; } } } From 49b8f36eabea93064215784d1ff8bec41a6d25f4 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 30 May 2016 16:17:49 +0100 Subject: [PATCH 07/13] Make opAssign infer safety of S.this(this) --- std/typecons.d | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index de30f2b1345..e8826899505 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1828,30 +1828,42 @@ if (is(S == struct)) payload = s; } - void opAssign(S s) @trusted + private + void swapPayloads(ref Rebindable rs) @trusted { - auto rs = Rebindable(s); import std.algorithm.mutation : swap; swap(mutPayload, rs.mutPayload); } - void opAssign(typeof(this) other) @trusted + void opAssign(S s) { - this = other.payload; + auto rs = Rebindable(s); + swapPayloads(rs); + } + + private + ref getPayload() @trusted + { + return payload; + } + + void opAssign(typeof(this) other) + { + this = other.getPayload; } static if (!is(S == immutable)) - ref S Rebindable_getRef() @property @trusted + ref S Rebindable_getRef() @property { // payload exposed as const ref when S is const - return payload; + return getPayload; } static if (is(S == immutable)) - S Rebindable_get() @property @trusted + S Rebindable_get() @property { // we return a copy so mutable union is OK - return payload; + return getPayload; } static if (is(S == immutable)) From 85ebede2165b6256c7da45f0d39e2d5f9274891d Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Thu, 2 Jun 2016 12:47:43 +0100 Subject: [PATCH 08/13] Use void[S.sizeof] mutPayload because union is too inflexible Unions don't allow struct members which have postblit or dtors. --- std/typecons.d | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index e8826899505..fb309e41678 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1815,41 +1815,45 @@ if (is(S == struct)) else struct Rebindable { - // fields of mutPayload must be treated as tail const - private union + private: + // mutPayload's pointers must be treated as tail const + void[S.sizeof] mutPayload; + + @trusted swapPayload(ref S s) { - Unqual!S mutPayload = void; - S payload; + // we preserve tail immutable guarantees so casts are OK + auto pOurs = cast(Unqual!S*)mutPayload.ptr; + auto pTheirs = cast(Unqual!S*)&s; + import std.algorithm.mutation : swap; + swap(*pOurs, *pTheirs); } - this(S s) + @trusted swapPayload(ref Rebindable rs) { - // we preserve tail immutable guarantees so mutable union is OK - payload = s; + swapPayload(*cast(S*)rs.mutPayload.ptr); } - private - void swapPayloads(ref Rebindable rs) @trusted + public: + this(S s) { - import std.algorithm.mutation : swap; - swap(mutPayload, rs.mutPayload); + swapPayload(s); } void opAssign(S s) { - auto rs = Rebindable(s); - swapPayloads(rs); + swapPayload(s); } - private - ref getPayload() @trusted + void opAssign(Rebindable other) { - return payload; + swapPayload(other); } - void opAssign(typeof(this) other) + private + ref const getPayload() @trusted { - this = other.getPayload; + // can only cast to const, not immutable + return *cast(const Unqual!S*)mutPayload.ptr; } static if (!is(S == immutable)) @@ -1862,7 +1866,7 @@ if (is(S == struct)) static if (is(S == immutable)) S Rebindable_get() @property { - // we return a copy so mutable union is OK + // we return a copy for immutable S return getPayload; } @@ -1875,7 +1879,7 @@ if (is(S == struct)) auto movePayload() @trusted { import std.algorithm : move; - return cast(S)move(mutPayload); + return cast(S)move(*cast(Unqual!S*)mutPayload.ptr); } ~this() From 3caca47934d205d8953db05b9d29faca008721c1 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Fri, 3 Jun 2016 12:02:34 +0100 Subject: [PATCH 09/13] Fix swapPayload violating const --- std/typecons.d | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index fb309e41678..554a78362d7 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1819,34 +1819,31 @@ if (is(S == struct)) // mutPayload's pointers must be treated as tail const void[S.sizeof] mutPayload; - @trusted swapPayload(ref S s) + void emplace(ref S s) { - // we preserve tail immutable guarantees so casts are OK - auto pOurs = cast(Unqual!S*)mutPayload.ptr; - auto pTheirs = cast(Unqual!S*)&s; - import std.algorithm.mutation : swap; - swap(*pOurs, *pTheirs); - } - - @trusted swapPayload(ref Rebindable rs) - { - swapPayload(*cast(S*)rs.mutPayload.ptr); + import std.conv : emplace; + static if (__traits(compiles, () @safe {S tmp = s;})) + () @trusted {emplace!S(mutPayload, s);}(); + else + emplace!S(mutPayload, s); } public: + // TODO: auto ref & for opAssign? this(S s) { - swapPayload(s); + emplace(s); } void opAssign(S s) { - swapPayload(s); + movePayload; + emplace(s); } void opAssign(Rebindable other) { - swapPayload(other); + this = other.getPayload; } private From d128695fe30a2c1fb8a4c9f2e84727b39a0bffa8 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 6 Jun 2016 11:53:23 +0100 Subject: [PATCH 10/13] getPayload cannot be returned as ref S --- std/typecons.d | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index 554a78362d7..d5f02a8d78c 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1843,28 +1843,28 @@ if (is(S == struct)) void opAssign(Rebindable other) { - this = other.getPayload; + this = other.trustedPayload; } + // must not escape when S is immutable private - ref const getPayload() @trusted + ref trustedPayload() @trusted { - // can only cast to const, not immutable - return *cast(const Unqual!S*)mutPayload.ptr; + 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 getPayload; + return trustedPayload; } static if (is(S == immutable)) S Rebindable_get() @property { // we return a copy for immutable S - return getPayload; + return trustedPayload; } static if (is(S == immutable)) From bf3bc5bf851ff8d78b3abc2fbb94e952ef25e5c4 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 6 Jun 2016 13:11:13 +0100 Subject: [PATCH 11/13] Use auto ref for ctor and opAssign --- std/typecons.d | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/std/typecons.d b/std/typecons.d index d5f02a8d78c..6f8e170f617 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1829,18 +1829,30 @@ if (is(S == struct)) } public: - // TODO: auto ref & for opAssign? - this(S s) + this()(auto ref S s) { emplace(s); } - void opAssign(S 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) + { + emplace(s); + } + void opAssign(Rebindable other) { this = other.trustedPayload; @@ -1986,11 +1998,11 @@ unittest } static assert(!__traits(compiles, Rebindable!ND())); - Rebindable!(const ND) rb = ND(1); + Rebindable!(const ND) rb = const ND(1); assert(rb.i == 1); rb = immutable ND(2); assert(rb.i == 2); - rb = rebindable(ND(3)); + rb = rebindable(const ND(3)); assert(rb.i == 3); static assert(!__traits(compiles, rb.i++)); } From 45eee71fdbd28cfaa3d8c0d51d90b962825cf93a Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Tue, 21 Feb 2017 12:56:29 +0000 Subject: [PATCH 12/13] Fix destroying existing immutable S on opAssign --- std/typecons.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/std/typecons.d b/std/typecons.d index 6f8e170f617..e297379ba68 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1850,6 +1850,7 @@ if (is(S == struct)) static if (!is(S == immutable)) void opAssign()(immutable S s) { + movePayload; emplace(s); } @@ -1894,7 +1895,7 @@ if (is(S == struct)) ~this() { // call destructor with proper constness - S s = movePayload; + movePayload; } } } From ac5abaedd192381a325e00d45b7e576b8bc4add2 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Tue, 21 Feb 2017 12:59:55 +0000 Subject: [PATCH 13/13] Test copying Rebindable!immutable --- std/typecons.d | 3 +++ 1 file changed, 3 insertions(+) diff --git a/std/typecons.d b/std/typecons.d index e297379ba68..40afe9ad606 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1945,6 +1945,9 @@ if (is(S == struct)) 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;}));