[phobos] I think we need to make an emergency release

[Steve Schveighoffer]

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 <andrei at erdani.com>
> To: Discuss the phobos library for D <phobos at puremagic.com>
> 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<dclugston at googlemail.com>
> >>  To: Discuss the phobos library for D<phobos at puremagic.com>
> >>  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
> >> phobos at puremagic.com
> >>  http://lists.puremagic.com/mailman/listinfo/phobos
> >>
> >
> >
> >
> >  _______________________________________________
> > phobos mailing  list
> > phobos at puremagic.com
> > http://lists.puremagic.com/mailman/listinfo/phobos
> _______________________________________________
> phobos  mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
>