I guess shift is just not a very common operation on sparse arrays. It’s almost like mutually exclusive use cases. Still good to have them fixed.
Thanks for the reviews! Hannes > Am 15.12.2016 um 13:47 schrieb Attila Szegedi <szege...@gmail.com>: > > +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 >
