+1 Wow, this is lots of fixes. Some of these bugs are so glaring I’m astonished they we did not spot them earlier, like failing to set a new length in shiftLeft. It’s great you fixed these.
Attila. > On 15 Dec 2016, at 11:48, Hannes Wallnöfer <hannes.wallnoe...@oracle.com> > wrote: > > Please review: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8171219 > Webrev: http://cr.openjdk.java.net/~hannesw/8171219/webrev/ > > This fixes a number of bugs around the Array.prototype.shift on sparse array > data (and its underlying dense data). Unlikely as it may seem, all these > issues where exposed by the short test script included in this webrev: > > - UntouchedArrayData did not convert to a real ArrayData instance when > ensuring place for first element (index 0) > - Dense array data did not check their size before trying System.arraycopy in > shiftLeft > - Dense array data did not set the new length in shiftLeft, thereby leaving > empty slots > - Sparse array data did not ensure space in the underlying dense array when > moving elements from sparse to dense in shiftLeft > - Sparse array data did not delete new in-between elements when moving > elements from sparse to dense in shiftLeft > - Sparse array data did not set the length after shrinking underlying dense > data in shiftRight, thereby creating new empty slots > - DeletedRangeArrayData could not return the underlying data if it turned > empty in shiftLeft, causing problems later on. > > I also moved the invocation of ensure() on the underlying dense array from > SparseArrayData.ensure() to where it is actually required, which is cleaner > and safe since underlying is a private field. > > Thanks, > Hannes
