Skip to content

Commit d418b84

Browse files
authored
Merge pull request #117 from Ichoran/issue-87
Fixed NPE in VectorStepper due to dirty display vector.
2 parents d678369 + c6ec67a commit d418b84

File tree

4 files changed

+90
-6
lines changed

4 files changed

+90
-6
lines changed

src/main/java/scala/compat/java8/runtime/CollectionInternals.java

+7
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,19 @@
1717
public class CollectionInternals {
1818
public static <A> Object[] getTable(scala.collection.mutable.FlatHashTable<A> fht) { return fht.hashTableContents().table(); }
1919
public static <A, E extends scala.collection.mutable.HashEntry<A,E>> scala.collection.mutable.HashEntry<A, E>[] getTable(scala.collection.mutable.HashTable<A,E> ht) { return ht.hashTableContents().table(); }
20+
public static <A> boolean getDirt(scala.collection.immutable.Vector<A> v) { return v.dirty(); }
2021
public static <A> Object[] getDisplay0(scala.collection.immutable.Vector<A> v) { return v.display0(); }
22+
public static <A> Object[] getDisplay0(scala.collection.immutable.VectorIterator<A> p) { return p.display0(); }
2123
public static <A> Object[] getDisplay1(scala.collection.immutable.Vector<A> v) { return v.display1(); }
24+
public static <A> Object[] getDisplay1(scala.collection.immutable.VectorIterator<A> p) { return p.display1(); }
2225
public static <A> Object[] getDisplay2(scala.collection.immutable.Vector<A> v) { return v.display2(); }
26+
public static <A> Object[] getDisplay2(scala.collection.immutable.VectorIterator<A> p) { return p.display2(); }
2327
public static <A> Object[] getDisplay3(scala.collection.immutable.Vector<A> v) { return v.display3(); }
28+
public static <A> Object[] getDisplay3(scala.collection.immutable.VectorIterator<A> p) { return p.display3(); }
2429
public static <A> Object[] getDisplay4(scala.collection.immutable.Vector<A> v) { return v.display4(); }
30+
public static <A> Object[] getDisplay4(scala.collection.immutable.VectorIterator<A> p) { return p.display4(); }
2531
public static <A> Object[] getDisplay5(scala.collection.immutable.Vector<A> v) { return v.display5(); }
32+
public static <A> Object[] getDisplay5(scala.collection.immutable.VectorIterator<A> p) { return p.display5(); }
2633
public static <A> scala.Tuple2< scala.Tuple2< scala.collection.Iterator<A>, Object >, scala.collection.Iterator<A> > trieIteratorSplit(scala.collection.Iterator<A> it) {
2734
if (it instanceof scala.collection.immutable.TrieIterator) {
2835
scala.collection.immutable.TrieIterator<A> trie = (scala.collection.immutable.TrieIterator<A>)it;

src/main/scala/scala/compat/java8/converterImpl/StepsLikeIndexed.scala

+3
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,19 @@ private[java8] abstract class StepsLikeIndexed[A, STA >: Null <: StepsLikeIndexe
4444
private[java8] abstract class StepsDoubleLikeIndexed[STD >: Null <: StepsDoubleLikeIndexed[_]](_i0: Int, _iN: Int)
4545
extends AbstractStepsLikeIndexed[DoubleStepper, STD](_i0, _iN)
4646
with DoubleStepper
47+
with java.util.Spliterator.OfDouble // Compiler wants this for mixin forwarder
4748
{}
4849

4950
/** Abstracts the operation of stepping over an indexable collection of Ints */
5051
private[java8] abstract class StepsIntLikeIndexed[STI >: Null <: StepsIntLikeIndexed[_]](_i0: Int, _iN: Int)
5152
extends AbstractStepsLikeIndexed[IntStepper, STI](_i0, _iN)
5253
with IntStepper
54+
with java.util.Spliterator.OfInt // Compiler wants this for mixin forwarder
5355
{}
5456

5557
/** Abstracts the operation of stepping over an indexable collection of Longs */
5658
private[java8] abstract class StepsLongLikeIndexed[STL >: Null <: StepsLongLikeIndexed[_]](_i0: Int, _iN: Int)
5759
extends AbstractStepsLikeIndexed[LongStepper, STL](_i0, _iN)
5860
with LongStepper
61+
with java.util.Spliterator.OfLong // Compiler wants this for mixin forwarder
5962
{}

src/main/scala/scala/compat/java8/converterImpl/StepsVector.scala

+52-6
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ package scala.compat.java8.converterImpl
1414

1515
import scala.annotation.switch
1616

17+
import scala.collection.immutable.VectorIterator
18+
1719
import scala.compat.java8.collectionImpl._
1820
import scala.compat.java8.runtime._
1921

@@ -25,20 +27,27 @@ import Stepper._
2527

2628
private[java8] trait StepsVectorLike[A] {
2729
protected def myVector: Vector[A]
30+
protected def myVectorIterator: VectorIterator[A]
31+
protected def myVectorLength: Int
2832
protected var index: Int = 32
2933
protected var data: Array[AnyRef] = null
3034
protected var index1: Int = 32
3135
protected var data1: Array[AnyRef] = null
3236
protected final def advanceData(iX: Int): Unit = {
3337
index1 += 1
34-
if (index >= 32) initTo(iX)
38+
if (index >= 32) {
39+
if (myVector != null) initTo(iX)
40+
else initVpTo(iX)
41+
}
3542
else {
3643
data = data1(index1).asInstanceOf[Array[AnyRef]]
3744
index = 0
3845
}
3946
}
4047
protected final def initTo(iX: Int): Unit = {
41-
myVector.length match {
48+
// WARNING--initVpTo is an exact copy of this except for the type! If you change one you must change the other!
49+
// (Manually specialized this way for speed.)
50+
myVectorLength match {
4251
case x if x <= 0x20 =>
4352
index = iX
4453
data = CollectionInternals.getDisplay0(myVector)
@@ -64,12 +73,43 @@ private[java8] trait StepsVectorLike[A] {
6473
data = data1(index1).asInstanceOf[Array[AnyRef]]
6574
}
6675
}
76+
protected final def initVpTo(iX: Int): Unit = {
77+
// WARNING--this is an exact copy of initTo! If you change one you must change the other!
78+
// (Manually specialized this way for speed.)
79+
myVectorLength match {
80+
case x if x <= 0x20 =>
81+
index = iX
82+
data = CollectionInternals.getDisplay0(myVectorIterator)
83+
case x if x <= 0x400 =>
84+
index1 = iX >>> 5
85+
data1 = CollectionInternals.getDisplay1(myVectorIterator)
86+
index = iX & 0x1F
87+
data = data1(index1).asInstanceOf[Array[AnyRef]]
88+
case x =>
89+
var N = 0
90+
var dataN: Array[AnyRef] =
91+
if (x <= 0x8000) { N = 2; CollectionInternals.getDisplay2(myVectorIterator) }
92+
else if (x <= 0x100000) { N = 3; CollectionInternals.getDisplay3(myVectorIterator) }
93+
else if (x <= 0x2000000) { N = 4; CollectionInternals.getDisplay4(myVectorIterator) }
94+
else /*x <= 0x40000000*/{ N = 5; CollectionInternals.getDisplay5(myVectorIterator) }
95+
while (N > 2) {
96+
dataN = dataN((iX >>> (5*N))&0x1F).asInstanceOf[Array[AnyRef]]
97+
N -= 1
98+
}
99+
index1 = (iX >>> 5) & 0x1F
100+
data1 = dataN((iX >>> 10) & 0x1F).asInstanceOf[Array[AnyRef]]
101+
index = iX & 0x1F
102+
data = data1(index1).asInstanceOf[Array[AnyRef]]
103+
}
104+
}
67105
}
68106

69107
private[java8] class StepsAnyVector[A](underlying: Vector[A], _i0: Int, _iN: Int)
70108
extends StepsLikeIndexed[A, StepsAnyVector[A]](_i0, _iN)
71109
with StepsVectorLike[A] {
72-
protected def myVector = underlying
110+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
111+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
112+
protected val myVectorLength = underlying.length
73113
def next() = if (hasNext()) {
74114
index += 1
75115
if (index >= 32) advanceData(i0)
@@ -88,7 +128,9 @@ with StepsVectorLike[A] {
88128
private[java8] class StepsDoubleVector(underlying: Vector[Double], _i0: Int, _iN: Int)
89129
extends StepsDoubleLikeIndexed[StepsDoubleVector](_i0, _iN)
90130
with StepsVectorLike[Double] {
91-
protected def myVector = underlying
131+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
132+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
133+
protected val myVectorLength = underlying.length
92134
def nextDouble() = if (hasNext()) {
93135
index += 1
94136
if (index >= 32) advanceData(i0)
@@ -107,7 +149,9 @@ with StepsVectorLike[Double] {
107149
private[java8] class StepsIntVector(underlying: Vector[Int], _i0: Int, _iN: Int)
108150
extends StepsIntLikeIndexed[StepsIntVector](_i0, _iN)
109151
with StepsVectorLike[Int] {
110-
protected def myVector = underlying
152+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
153+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
154+
protected val myVectorLength = underlying.length
111155
def nextInt() = if (hasNext()) {
112156
index += 1
113157
if (index >= 32) advanceData(i0)
@@ -126,7 +170,9 @@ with StepsVectorLike[Int] {
126170
private[java8] class StepsLongVector(underlying: Vector[Long], _i0: Int, _iN: Int)
127171
extends StepsLongLikeIndexed[StepsLongVector](_i0, _iN)
128172
with StepsVectorLike[Long] {
129-
protected def myVector = underlying
173+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
174+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
175+
protected val myVectorLength = underlying.length
130176
def nextLong() = if (hasNext()) {
131177
index += 1
132178
if (index >= 32) advanceData(i0)

src/test/scala/scala/compat/java8/StreamConvertersTest.scala

+28
Original file line numberDiff line numberDiff line change
@@ -288,4 +288,32 @@ class StreamConvertersTest {
288288
val stepper2 = steppize2(coll2).stepper
289289
assertTrue(stepper2.getClass.getName.contains("StepsIntVector"))
290290
}
291+
292+
@Test
293+
def issue_87(): Unit = {
294+
// Vectors that are generated from other vectors tend _not_ to
295+
// have all their display vectors consistent; the cached vectors
296+
// are correct, but the higher-level vector does _not_ contain
297+
// the cached vector in the correct place (for efficiency)! This
298+
// is called being "dirty", and needs to be handled specially.
299+
val dirtyDisplayVector = Vector.fill(120)("a").slice(0, 40)
300+
val shouldNotNPE =
301+
dirtyDisplayVector.seqStream.collect(Collectors.toList())
302+
assertEq(shouldNotNPE.toArray(new Array[String](0)).toVector, dirtyDisplayVector, "Vector[Any].seqStream (with dirty display)")
303+
304+
val dirtyDisplayVectorInt = Vector.fill(120)(999).slice(0, 40)
305+
val shouldNotNPEInt =
306+
dirtyDisplayVectorInt.seqStream.sum()
307+
assertEq(shouldNotNPEInt, dirtyDisplayVectorInt.sum, "Vector[Int].seqStream (with dirty display)")
308+
309+
val dirtyDisplayVectorLong = Vector.fill(120)(99999999999L).slice(0, 40)
310+
val shouldNotNPELong =
311+
dirtyDisplayVectorLong.seqStream.sum()
312+
assertEq(shouldNotNPELong, dirtyDisplayVectorLong.sum, "Vector[Long].seqStream (with dirty display)")
313+
314+
val dirtyDisplayVectorDouble = Vector.fill(120)(0.1).slice(0, 40)
315+
val shouldNotNPEDouble =
316+
math.rint(dirtyDisplayVectorDouble.seqStream.sum() * 10)
317+
assertEq(shouldNotNPEDouble, math.rint(dirtyDisplayVectorDouble.sum * 10), "Vector[Double].seqStream (with dirty display)")
318+
}
291319
}

0 commit comments

Comments
 (0)