D array expansion and non-deterministic re-allocation

[Bartosz Milewski]

Steven Schveighoffer Wrote:

> On Tue, 24 Nov 2009 16:52:52 -0500, Bartosz Milewski  
> <bartosz-nospam at relisoft.com> wrote:
> 
> > Steven Schveighoffer Wrote:
> >> > Imagine you're reviewing this code written by a relative newbee:
> >> >
> >> > char[] quote(char[] word) {
> >> >   word.length += 2;
> >> >   word[length-1] = '"';
> >> >   for(int i = word.length-2; i > 0; --i)
> >> >     word[i] = word[i - 1];
> >> >   word[0] = '"';
> >> >   return word;
> >> > }
> >> >
> >> > void storeFoos(char[] buffer, string pattern, Container c)
> >> > {
> >> > 	char[] res = find(buffer, pattern);
> >> > 	c.store(quote(res[0..pattern.len]);
> >> > }
> >> >
> >> > Is this buggy? If so, how and when will the bug manifest itself?
> >>
> >> In my mind it is not buggy.  If quote or storeFoos is not meant to be  
> >> able
> >> to molest the data, it should require a const(char)[].  If this was D1
> >> code, I'd say it depends on the purpose of storeFoos, if storeFoos is
> >> expecting to own the input buffer, it's not buggy.  If it's not, then
> >> storeFoos is buggy (in all cases).  But we aren't talking about D1.
> > Yes, a lot depends on what's expected of storeFoos. Suppose that the  
> > surrounding code expects the buffer not to be modified. The newbee  
> > didn't want to idup the buffer because in most buffers the pattern is  
> > not found.
> 
> Then he forgot the const decoration for the parameter.  No duping is  
> necessary:
> 
> void storeFoos(const(char)[] buffer, string pattern, Container c)
> {
>     ... // will result in compiler errors for given quote function, fix  
> that.
> }

Well, he tried that, but then his quote function wouldn't work and he needs it in other places as well. You know how they are ;-). Besides he thinks "const" is for wimps. He won't stop arguing until you prove to him that there is an actual bug in his code.

> >
> >> > Or another story. An enthusiastic programmer comes to you with a very
> >> > clever rewrite of a function. This is the original:
> >> >
> >> > T[] duplicate(T)(T[] src) {
> >> >    static if (is(T == invariant))
> >> >       return src.idup;
> >> >    else
> >> >       return src.dup;
> >> > }
> >> >
> >> > This is his improved version:
> >> >
> >> > T[] duplicate(T)(T[] src) {
> >> >    T tmp = src[$-1];
> >> >    src.length -= 1;
> >> >    src ~= tmp;
> >> >    return src;
> >> > }
> >> >
> >> > No need for static if! The extension is bound to re-allocate because  
> >> of
> >> > stomping rules. Will this always work, or is there a gotcha?
> >>
> >> He forgot to check for length 0.
> >>
> >
> > That he did. So now he goes back to the drawing board and adds an early  
> > return for length 0. Is his code correct now?
> 
> Yes.
> 
> I have a feeling you have some trick up your sleeve that you think makes  
> it incorrect :)

Well, I was wondering if it could be rewritten as:
T[] duplicate(T)(T[] src) {
    assert(src.length != 0);
    T tmp = src[$-1];
    src.length -= 1;
    src.length += 1;
    src[$-1] = tmp;
    return src;
}
and whether it's okay for the optimizer to do some simple arithmetic optimization.