D array expansion and non-deterministic re-allocation

[Steven Schveighoffer]

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.
}

>
>> > 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 :)

-Steve