I had been using the idiom you describe, but upon looking into it, it's for reasons that no longer apply. Appender used to have a release() method that gave you access to the underlying array, but insisted on reallocating it under certain circumstances before it did. Now that this "feature" is gone, I wouldn't mind too much seeing the unsafe pointer constructor go.
In general, though, since Appender is entirely a performance hack anyhow, it should err on the side of performance, not safety. I think there needs to be an @system way of initializing from an existing array and stomping issues be damned. On Wed, Aug 25, 2010 at 3:48 PM, Steve Schveighoffer <[email protected]>wrote: > Alright, I have now a working Appender to replace the current one (along > with > the appropriate changes to phobos). > > This one should be completely usable in safe mode save a couple of > functions > (clear and shrinkTo). > > Before I commit, I want to get the opinions of others on this: > > One very very notable difference from the original appender is it does not > take > a pointer as the parameter. This means it does not update the original > array > you pass to it (well, it does, but will not modify the length). There was > only > really a couple places in phobos that used this to an advantage, most > places > that used appender had code like this: > > string buf; > auto app = appender(&buf); > > And then never used buf again. > > So I'm wondering how much this feature is needed (affecting the original > array). I feel its a very unsafe method of passing an array around, > especially > when we have perfectly safe ways, and the updated version of the array is > available via the data property. > > Other than that, here is the updated API (with private data shown for an > idea of > how the implementation works): > > struct Appender(A : T[], T) > { > private struct Data > { > size_t capacity; > Unqual!(T)[] arr; > } > > private Data* _data; > > /** > Construct an appender with a given array. Note that this does not copy the > data, but appending to the array will copy the data, since Appender does > not > know if this data has valid data residing after it. The initial capacity > will > be > arr.length. > */ > this(T[] arr); > > /** > Reserve at least newCapacity elements for appending. Note that more > elements > may be reserved than requested. If newCapacity < capacity, then nothing is > done. > */ > void reserve(size_t newCapacity); > > /** > Returns the capacity of the array (the maximum number of elements the > managed array can accommodate before triggering a reallocation). If any > appending will reallocate, capacity returns 0. > */ > @property size_t capacity(); > > /** > Returns the managed array. > */ > @property T[] data(); > > /** > Appends one item to the managed array. > */ > void put(U)(U item) if (isImplicitlyConvertible!(U, T) || > isSomeChar!T && isSomeChar!U); > > /** > Appends an entire range to the managed array. > */ > void put(Range)(Range items) if (isForwardRange!Range > && is(typeof(Appender.init.put(items.front)))); > > /** > Clears the managed array. This leaves the capacity intact. > */ > void clear(); > > /** > Shrinks the managed array to the given length. Passing in a length > that's greater than the current array length throws an enforce exception. > */ > void shrinkTo(size_t newlength); > } > > > > > ----- Original Message ---- > > From: Steve Schveighoffer <[email protected]> > > To: Discuss the phobos library for D <[email protected]> > > Sent: Wed, August 25, 2010 9:50:37 AM > > Subject: Re: [phobos] I think we need to make an emergency release > > > > > > > > OK, looked at it for a while, I wasn't clear originally on what Appender > is > > exactly supposed to do. > > > > I found a superficial bug before I wrote the message below -- you were > >stomping > > > > over the capacity without rewriting it. > > > > But in general, I think the method you are using to store the capacity > is > > flawed. It's prone to stomping (i.e. using appender on a slice) and > it's > > somewhat inefficient (you are constantly re-storing the capacity). It's > going > > > > to run into problems if you try using ~= on an array that you appended > to with > > > > Appender. > > > > Part of the problem is that Appender is trying to apply its metadata to > an > > existing array. Appender is using data it doesn't own or isn't given > >permission > > > > to change. This can be bad on many levels (including violating > immutability > > rules). > > > > I'd recommend scrapping Appender as written and creating one that does > the > > following: > > > > 1. if initialized with an existing array, initialize capacity to 0, > forcing a > > > realloc on first append > > 2. Store the capacity outside the array data (i.e. in a heap-allocated > > array+capacity struct). > > > > Yes, 1 can be inefficient but it errs on the side of safety. Once it > > reallocates the first time, it will be very efficient. This works as > long as > > > the assumption is that you use Appender to append large amounts of data. > > > > I'll work on a proposed replacement Appender struct, it shouldn't take > too > >long. > > > > -Steve > > > > > > ----- Original Message ---- > > > From: Andrei Alexandrescu <[email protected]> > > > To: Discuss the phobos library for D <[email protected]> > > > Sent: Wed, August 25, 2010 9:09:33 AM > > > Subject: Re: [phobos] I think we need to make an emergency release > > > > > > Thanks, and also thanks for announcing this. I was about to start > > > looking at it... > > > > > > Andrei > > > > > > On 8/25/10 6:08 PDT, Steve Schveighoffer wrote: > > > > I think I know the problem with the code. I'll check in a fix, and > update > > > > >the > > > > bug, see if it helps. > > > > > > > > -Steve > > > > > > > > > > > > > > > > ----- Original Message ---- > > > >> From: Don Clugston<[email protected]> > > > >> To: Discuss the phobos library for D<[email protected]> > > > >> Sent: Tue, August 24, 2010 3:35:09 PM > > > >> Subject: [phobos] I think we need to make an emergency release > > > >> > > > >> David suggested we need an emergency release for the array > appender > > > >> bug, and I'm inclined to agree with him. My personal experience > is > > > >> that it makes 2.048 unusable, and I've had to go back to 2.047. > Here's > > > >> the bug: > > > >> > > > >> Bug 4681 Appender access violation > > > >> This is a really horrible memory corruption regression, caused by > svn > > > >> 1813. Is there any reason we shouldn't just roll that back? > > > >> > > > >> --- > > > >> BTW, one compiler bug popped up in the last release, too. Fairly > > > >> obscure though. I created a patch: > > > >> 4655 Regression(1.063, 2.048) goto to a try block ICEs > > > >> > > > >> I also made a patch for this compiler bug from three releases > back: > > > >> 4302 Regression(1.061, 2.046) compiler errors using startsWith in > >CTFE > > > >> > > > >> All other known compiler regressions were introduced in 2.041 or > > earlier. > > > >> _______________________________________________ > > > >> phobos mailing list > > > >> [email protected] > > > >> http://lists.puremagic.com/mailman/listinfo/phobos > > > >> > > > > > > > > > > > > > > > > _______________________________________________ > > > > phobos mailing list > > > > [email protected] > > > > http://lists.puremagic.com/mailman/listinfo/phobos > > > _______________________________________________ > > > phobos mailing list > > > [email protected] > > > http://lists.puremagic.com/mailman/listinfo/phobos > > > > > > > > > > > _______________________________________________ > > phobos mailing list > > [email protected] > > http://lists.puremagic.com/mailman/listinfo/phobos > > > > > > _______________________________________________ > phobos mailing list > [email protected] > http://lists.puremagic.com/mailman/listinfo/phobos >
_______________________________________________ phobos mailing list [email protected] http://lists.puremagic.com/mailman/listinfo/phobos
