On Thu, Apr 18, 2013 at 02:43:54PM -0700, Ali Çehreli wrote:
> On 04/18/2013 02:06 PM, Brad Anderson wrote:
> >Is this supposed to be allowed:
> >
> >ubyte[] a;
> >ubyte[16] b;
> >a = b;
> >assert(a.ptr == b.ptr);
> >
> >Because if so that makes it terribly easy to do a bug like this (as I
> >just saw in IRC):
> >
> >struct A
> >{
> >     ubyte[] a;
> >     this(ubyte c)
> >     {
> >         ubyte[16] b;
> >         b[] = c;
> >         this.a = b;  // a now points at an immediately invalid static
> >array
> >     }
> >}
> 
> There is a similar problem with the automatically generated array arguments.
> 
> The following constructor takes any number of ints that come in array form:
> 
> import std.stdio;
> 
> struct S
> {
>     int[] a;
> 
>     this(int[] args...)
>     {
>         a = args;
>     }
> 
>     void foo()
>     {
>         writeln(a);
>     }
> }
[...]

Yeah I got bitten by this before. Took me several days to find the problem, 'cos it was nested deep inside a complex data structure, and at a glance it doesn't *look* wrong.

I'm all for making this @system at the very least, if not outright compile error. Storing a persistent reference to a stack-allocated object is outright wrong... in the case of variadic array args, if the compiler can't prove that args will *always* be a dynamic array, then any attempt to save a reference to it should be rejected outright IMO.


T

-- 
It is impossible to make anything foolproof because fools are so ingenious. -- Sammy

On Thu, 18 Apr 2013 18:13:14 -0400, Ali Çehreli <acehreli@yahoo.com> wrote:


> Interesting.
>
> Personally, I would not even bother with the second one and expect the caller to simply put square brackets around the arguments:

[snip]

>
> It is now safe, right?

Yes, for this case it is OK.  But I want a consistent interface in
dcollections.  Other containers do not use arrays as storage, so the
bracketed version for something like a TreeSet would be a complete waste
(and in fact, only the variadic version exists there).  The special case
for ArrayList is the slice version, where it actually takes on that slice
as its internal storage.  ArrayList is meant to bridge the gap between D
slices and dcollections.  It's range type is a D slice as well.

-Steve

On Thu, 18 Apr 2013 18:08:48 -0400, bearophile <bearophileHUGS@lycos.com>
wrote:

> To avoid those bugs I have suggested the simpler possible thing: (V[] elems...) to dup the data on the heap every time. In theory if you write "(scope V[] elems...)" it will be free to not dup the data, avoiding the heap allocation and the associated little performance loss. In practice as you know "scope" is not yet implemented. D2 language is not close to being fully implemented.

I am OK with this as long as it is done AFTER scope works (and I would
update my calls appropriately).

But in my specific case, I still want the second overload, because the
idea is ArrayList uses that slice as its actual storage.

-Steve

On Thu, 18 Apr 2013 19:15:06 -0400, H. S. Teoh <hsteoh@quickfur.ath.cx> wrote:

> I'm all for making this @system at the very least, if not outright
> compile error. Storing a persistent reference to a stack-allocated
> object is outright wrong... in the case of variadic array args, if the
> compiler can't prove that args will *always* be a dynamic array, then
> any attempt to save a reference to it should be rejected outright IMO.

I like bearophile's solution, we increasingly need scope to work for D to be easy to avoid mistakes.

-Steve

Steven Schveighoffer:

> we increasingly need scope to work for D to be easy to avoid mistakes.

But I don't see any plan/implementation timeframe for "scope" :-( I don't know why. I'd like to know Hara opinion on this.

Bye,
bearophile

Steven Schveighoffer:

> we increasingly need scope to work for D to be easy to avoid mistakes.

I have added a note about scope:
http://d.puremagic.com/issues/show_bug.cgi?id=5212

Bye,
bearophile