From 819da05056653b7898e01291d7aa5ef701a01604 Mon Sep 17 00:00:00 2001 From: monarchdodra Date: Wed, 28 Aug 2013 13:20:38 +0200 Subject: [PATCH 1/3] Fix appender form elaborate assign types --- std/array.d | 83 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/std/array.d b/std/array.d index 15a680ef3ca..ec86bf8ba5a 100644 --- a/std/array.d +++ b/std/array.d @@ -258,6 +258,7 @@ unittest { static assert(!__traits(compiles, [ tuple("foo", "bar", "baz") ].assocArray())); static assert(!__traits(compiles, [ tuple("foo") ].assocArray())); + [ tuple("foo", "bar") ].assocArray(); static assert( __traits(compiles, [ tuple("foo", "bar") ].assocArray())); auto aa1 = [ tuple("foo", "bar"), tuple("baz", "quux") ].assocArray(); @@ -2337,14 +2338,27 @@ struct Appender(A : T[], T) { ensureAddable(1); immutable len = _data.arr.length; - //_data.arr.ptr[len] = cast(Unqual!T)item; // assign? emplace? - //_data.arr = _data.arr.ptr[0 .. len + 1]; - - // Cannot return ref because it doesn't work in CTFE - ()@trusted{ return _data.arr.ptr[len .. len + 1]; }()[0] - = // assign? emplace? - ()@trusted{ return cast(Unqual!T)item; } (); - ()@trusted{ _data.arr = _data.arr.ptr[0 .. len + 1]; }(); + + auto bigDataFun() @trusted nothrow { return _data.arr.ptr[0 .. len + 1];} + auto bigData = bigDataFun(); + + auto getTarget() @trusted nothrow { return cast(Unqual!T)item;} + + //The idea is to only call emplace if we must. + static if ( is(typeof(bigData[0].opAssign(getTarget()))) || + !is(typeof(bigData[0] = getTarget()))) + { + //pragma(msg, T.stringof); pragma(msg, U.stringof); + emplace(&bigData[len], getTarget()); + } + else + { + //pragma(msg, T.stringof); pragma(msg, U.stringof); + bigData[len] = getTarget(); + } + + //We do this at the end, in case of exceptions + _data.arr = bigData; } } @@ -2382,23 +2396,38 @@ struct Appender(A : T[], T) ensureAddable(items.length); immutable len = _data.arr.length; immutable newlen = len + items.length; - _data.arr = ()@trusted{ return _data.arr.ptr[0 .. newlen]; }(); - static if (is(typeof(_data.arr[] = items[]))) + + auto bigDataFun() @trusted nothrow { return _data.arr.ptr[0 .. newlen];} + auto bigData = bigDataFun(); + + enum mustEmplace = is(typeof(bigData[0].opAssign(cast(Unqual!T)items.front))) || + !is(typeof(bigData[0] = cast(Unqual!T)items.front)); + + static if (is(typeof(_data.arr[] = items[])) && !mustEmplace) { - ()@trusted{ return _data.arr.ptr[len .. newlen]; }()[] = items[]; + //pragma(msg, T.stringof); pragma(msg, Range.stringof); + bigData[len .. newlen] = items[]; } else { - for (size_t i = len; !items.empty; items.popFront(), ++i) + auto getTarget() @trusted nothrow {return cast(Unqual!T)items.front;} + foreach (ref it ; bigData[len .. newlen]) { - //_data.arr.ptr[i] = cast(Unqual!T)items.front; - - // Cannot return ref because it doesn't work in CTFE - ()@trusted{ return _data.arr.ptr[i .. i + 1]; }()[0] - = // assign? emplace? - ()@trusted{ return cast(Unqual!T)items.front; }(); + static if (mustEmplace) + { + //pragma(msg, T.stringof); pragma(msg, Range.stringof); + emplace(&it, getTarget()); + } + else + { + //pragma(msg, T.stringof); pragma(msg, Range.stringof); + it = getTarget(); + } } } + + //We do this at the end, in case of exceptions + _data.arr = bigData; } else { @@ -2727,6 +2756,24 @@ Appender!(E[]) appender(A : E[], E)(A array) } } +unittest +{ + //10690 + [tuple(1)].filter!(t => true).array; // No error + [tuple("A")].filter!(t => true).array; // error +} + +unittest +{ + //Coverage for put(Range) emplace + struct S + { + void opAssign(S){} + } + auto a = Appender!(S[])(); + a.put([S()]); +} + /++ Convenience function that returns a $(D RefAppender!A) object initialized with $(D array). Don't use null for the $(D array) pointer, use the other From 81bf0aa57084a8c851f233a417f959a9d02f76db Mon Sep 17 00:00:00 2001 From: monarchdodra Date: Wed, 28 Aug 2013 16:12:21 +0200 Subject: [PATCH 2/3] More unittest for appender --- std/array.d | 78 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/std/array.d b/std/array.d index ec86bf8ba5a..a6b7cb42f2b 100644 --- a/std/array.d +++ b/std/array.d @@ -258,7 +258,6 @@ unittest { static assert(!__traits(compiles, [ tuple("foo", "bar", "baz") ].assocArray())); static assert(!__traits(compiles, [ tuple("foo") ].assocArray())); - [ tuple("foo", "bar") ].assocArray(); static assert( __traits(compiles, [ tuple("foo", "bar") ].assocArray())); auto aa1 = [ tuple("foo", "bar"), tuple("baz", "quux") ].assocArray(); @@ -2342,19 +2341,19 @@ struct Appender(A : T[], T) auto bigDataFun() @trusted nothrow { return _data.arr.ptr[0 .. len + 1];} auto bigData = bigDataFun(); - auto getTarget() @trusted nothrow { return cast(Unqual!T)item;} + auto getSource() @trusted nothrow { return cast(Unqual!T)item;} //The idea is to only call emplace if we must. - static if ( is(typeof(bigData[0].opAssign(getTarget()))) || - !is(typeof(bigData[0] = getTarget()))) + static if ( is(typeof(bigData[0].opAssign(getSource()))) || + !is(typeof(bigData[0] = getSource()))) { //pragma(msg, T.stringof); pragma(msg, U.stringof); - emplace(&bigData[len], getTarget()); + emplace(&bigData[len], getSource()); } else { //pragma(msg, T.stringof); pragma(msg, U.stringof); - bigData[len] = getTarget(); + bigData[len] = getSource(); } //We do this at the end, in case of exceptions @@ -2410,18 +2409,18 @@ struct Appender(A : T[], T) } else { - auto getTarget() @trusted nothrow {return cast(Unqual!T)items.front;} + auto getSource()() @trusted {return cast(Unqual!T)items.front;} foreach (ref it ; bigData[len .. newlen]) { static if (mustEmplace) { //pragma(msg, T.stringof); pragma(msg, Range.stringof); - emplace(&it, getTarget()); + emplace(&it, getSource()); } else { //pragma(msg, T.stringof); pragma(msg, Range.stringof); - it = getTarget(); + it = getSource(); } } } @@ -2774,6 +2773,67 @@ unittest a.put([S()]); } +unittest +{ + struct S + { + int* p; + } + + auto a0 = Appender!(S[])(); + auto a1 = Appender!(const(S)[])(); + auto a2 = Appender!(immutable(S)[])(); + auto s0 = S(null); + auto s1 = const(S)(null); + auto s2 = immutable(S)(null); + a1.put(s0); + a1.put(s1); + a1.put(s2); + a1.put([s0]); + a1.put([s1]); + a1.put([s2]); + a0.put(s0); + static assert(!is(typeof(a0.put(a1)))); + static assert(!is(typeof(a0.put(a2)))); + a0.put([s0]); + static assert(!is(typeof(a0.put([a1])))); + static assert(!is(typeof(a0.put([a2])))); + static assert(!is(typeof(a2.put(a0)))); + static assert(!is(typeof(a2.put(a1)))); + a2.put(s2); + static assert(!is(typeof(a2.put([a0])))); + static assert(!is(typeof(a2.put([a1])))); + a2.put([s2]); +} + +unittest +{ //9528 + const(E)[] fastCopy(E)(E[] src) { + auto app = appender!(const(E)[])(); + foreach (i, e; src) + app.put(e); + return app.data; + } + + class C {} + struct S { const(C) c; } + S[] s = [ S(new C) ]; + + auto t = fastCopy(s); // Does not compile +} + +unittest +{ //10753 + struct Foo { + immutable dchar d; + } + struct Bar { + immutable int x; + } + "12".map!Foo.array; + [1, 2].map!Bar.array; +} + /++ Convenience function that returns a $(D RefAppender!A) object initialized with $(D array). Don't use null for the $(D array) pointer, use the other From 8a50b18c3c47943955356aee3a4a359defe1622b Mon Sep 17 00:00:00 2001 From: monarchdodra Date: Sat, 31 Aug 2013 10:57:21 +0200 Subject: [PATCH 3/3] tweak appender, add unittests for not-yet covered paths --- std/array.d | 56 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/std/array.d b/std/array.d index a6b7cb42f2b..6281806bab7 100644 --- a/std/array.d +++ b/std/array.d @@ -2341,19 +2341,22 @@ struct Appender(A : T[], T) auto bigDataFun() @trusted nothrow { return _data.arr.ptr[0 .. len + 1];} auto bigData = bigDataFun(); - auto getSource() @trusted nothrow { return cast(Unqual!T)item;} + static if (is(Unqual!T == T)) + alias uitem = item; + else + auto ref uitem() @trusted nothrow @property { return cast(Unqual!T)item;} //The idea is to only call emplace if we must. - static if ( is(typeof(bigData[0].opAssign(getSource()))) || - !is(typeof(bigData[0] = getSource()))) + static if ( is(typeof(bigData[0].opAssign(uitem))) || + !is(typeof(bigData[0] = uitem))) { //pragma(msg, T.stringof); pragma(msg, U.stringof); - emplace(&bigData[len], getSource()); + emplace(&bigData[len], uitem); } else { //pragma(msg, T.stringof); pragma(msg, U.stringof); - bigData[len] = getSource(); + bigData[len] = uitem; } //We do this at the end, in case of exceptions @@ -2407,21 +2410,25 @@ struct Appender(A : T[], T) //pragma(msg, T.stringof); pragma(msg, Range.stringof); bigData[len .. newlen] = items[]; } + else static if (is(Unqual!T == ElementType!Range)) + { + foreach (ref it ; bigData[len .. newlen]) + { + static if (mustEmplace) + emplace(&it, items.front); + else + it = items.front; + } + } else { - auto getSource()() @trusted {return cast(Unqual!T)items.front;} + static auto ref getUItem(U)(U item) @trusted {return cast(Unqual!T)item;} foreach (ref it ; bigData[len .. newlen]) { static if (mustEmplace) - { - //pragma(msg, T.stringof); pragma(msg, Range.stringof); - emplace(&it, getSource()); - } + emplace(&it, getUItem(items.front)); else - { - //pragma(msg, T.stringof); pragma(msg, Range.stringof); - it = getSource(); - } + it = getUItem(items.front); } } @@ -2764,13 +2771,24 @@ unittest unittest { - //Coverage for put(Range) emplace - struct S + //Coverage for put(Range) + struct S1 + { + } + struct S2 { - void opAssign(S){} + void opAssign(S2){} } - auto a = Appender!(S[])(); - a.put([S()]); + auto a1 = Appender!(S1[])(); + auto a2 = Appender!(S2[])(); + auto au1 = Appender!(const(S1)[])(); + auto au2 = Appender!(const(S2)[])(); + a1.put(S1().repeat().take(10)); + a2.put(S2().repeat().take(10)); + auto sc1 = const(S1)(); + auto sc2 = const(S2)(); + au1.put(sc1.repeat().take(10)); + au2.put(sc2.repeat().take(10)); } unittest