Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 143 additions & 18 deletions std/array.d
Original file line number Diff line number Diff line change
Expand Up @@ -2337,14 +2337,30 @@ 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];}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah it's a bummer we need to resort to such tricks...

auto bigData = bigDataFun();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a simpler idiom:

auto bigData = (() @trusted nothrow => _data.arr.ptr[0 .. len + 1])();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that currently, DMD doesn't inline lambdas. Doing this gives a definit performance hit.

http://d.puremagic.com/issues/show_bug.cgi?id=10848
https://d.puremagic.com/issues/show_bug.cgi?id=10864
dlang/dmd#2483


static if (is(Unqual!T == T))
alias uitem = item;
else
auto ref uitem() @trusted nothrow @property { return cast(Unqual!T)item;}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, auto ref? I don't think ref can work with a cast, e.g.:

float y;
ref int x() { return cast(int)y; }  // Error: cast(int)y is not an lvalue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah that's never going to yield a ref


//The idea is to only call emplace if we must.
static if ( is(typeof(bigData[0].opAssign(uitem))) ||
!is(typeof(bigData[0] = uitem)))
{
//pragma(msg, T.stringof); pragma(msg, U.stringof);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should go

emplace(&bigData[len], uitem);
}
else
{
//pragma(msg, T.stringof); pragma(msg, U.stringof);
bigData[len] = uitem;
}

//We do this at the end, in case of exceptions
_data.arr = bigData;
}
}

Expand Down Expand Up @@ -2382,23 +2398,42 @@ 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))) ||
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at hasElaborateAssign, you'll see it also checks for a ref opAssign method via a simple lvalueOf function (again I really look forward to that pull from Dennis being pulled soon, unless it's been pulled already). The cast() here forces the value to be an r-value, which might mean you won't catch opAssign that takes a ref and mustEmplace will be false. Unless I'm wrong could you fix this and add a test-case with an opAssign that takes a ref parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we have a new isAssignable && hasElaborateAssign trait which are now robust enough to not need such tricks. I'll fix it. And dennis' pull was merged in already, yeah :)

!is(typeof(bigData[0] = cast(Unqual!T)items.front));

static if (is(typeof(_data.arr[] = items[])) && !mustEmplace)
{
//pragma(msg, T.stringof); pragma(msg, Range.stringof);
bigData[len .. newlen] = items[];
}
else static if (is(Unqual!T == ElementType!Range))
{
()@trusted{ return _data.arr.ptr[len .. newlen]; }()[] = items[];
foreach (ref it ; bigData[len .. newlen])
{
static if (mustEmplace)
emplace(&it, items.front);
else
it = items.front;
}
}
else
{
for (size_t i = len; !items.empty; items.popFront(), ++i)
static auto ref getUItem(U)(U item) @trusted {return cast(Unqual!T)item;}
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)
emplace(&it, getUItem(items.front));
else
it = getUItem(items.front);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting fact: There is no items.popFront in this loop.

We are, in fact, repeatedly copying the first item :/

}
}

//We do this at the end, in case of exceptions
_data.arr = bigData;
}
else
{
Expand Down Expand Up @@ -2727,6 +2762,96 @@ 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)
struct S1
{
}
struct S2
{
void opAssign(S2){}
}
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
{
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
Expand Down