Possible bug in associative array implementation (and/or @safe checking)

[Steven Schveighoffer]

On 8/16/18 12:51 PM, Aaron D. Trout wrote:
> Hello all! I'm a mathematician at Chatham University in Pittsburgh, PA 
> (USA). I use D for research, and I'm a frequent lurker on the forums. I 
> think I found a bug, but I want to make sure I'm not just doing 
> something silly. (I'm happy to file a bugzilla issue if it turns out to 
> be a legitimate and previously unknown bug.) Thanks in advance for the 
> feedback.
> 
> Tested on 64-bit linux, DMD 2.080.0 and LDC 1.9.0.
> 
> code:
> ---------------------------------------------------
> import std.range, std.algorithm, std.conv, std.stdio;
> 
> // return a newly constructed immutable static array
> immutable(int)[len] toImmutStaticArray(size_t len, R)(R range)
> {
>      int[len] r;
>      copy(range, r[]);
>      return r;
> }
> 
> void main() @safe
> {
>      int[int[]] aaHeapKeys;
>      int[int[]] aaStackKeys;
>      int[int[2]] aaStaticKeys;
> 
>      int[] setA = [2,3,5];
>      int[] setB = [2,4,3,9,5,25];
> 
>      // range of ranges: [[5, 25], [2, 4], [3, 9]]
>      auto sets = setA.map!(j => setB.filter!(i => i % j == 0));
> 
>      foreach(s; sets)
>      {
>          // insert using a slice of imuutable(int) on the heap as the key
>          auto heapSlice = s.array.idup;
>          aaHeapKeys[heapSlice] = 0;
> 
>          // insert using an identical slice of immutable(int) on the stack
>          // as the key
>          auto buffer = s.toImmutStaticArray!2;
>          auto stackSlice = buffer[0 .. s.walkLength];

Here is the problem. stackSlice is pointing to buffer. buffer's stack 
space is REUSED for each iteration through the loop, so it's going to 
change every time through the foreach. In essence, the current buffer 
goes out of scope and a new buffer is allocated in it's place.

In general, it's dangerous to have immutable pointer to stack data, 
because the stack will eventually go away and get reallocated, which 
makes the data not so immutable.

>          assert(stackSlice == heapSlice);
>          aaStackKeys[stackSlice] = 0;
> 
>          // insert using the static array buffer as key
>          aaStaticKeys[buffer] = 0;
>      }
> 
>      assert(aaHeapKeys   == [[5, 25]:0, [2, 4]:0, [3, 9]:0]); // OK
>      // assert(aaStackKeys  == [[5, 25]:0, [2, 4]:0, [3, 9]:0]); // FAILS!
>      assert(aaStaticKeys == [[5, 25]:0, [2, 4]:0, [3, 
> 9]:0].to!(int[int[2]])); // OK
> 
>      // Note that aaStackKeys seems corrupted. Printing out aaStackKeys
>      // gives: [[5, 25]:0, [5, 25]:0, [5, 25]:0]
>      // aaStackKeys.writeln;
> }

Yes, this is the effect I would expect.

D has traditionally simply allowed slicing stack data without question 
(even in @safe code), but that will change when dip1000 is fully 
realized. It will be allowed, but only when assigning to scope variables.

-Steve