Skip to content

Commit

Permalink
Merge pull request #3388 from rklaehn/issue/7445
Browse files Browse the repository at this point in the history
 ListMap.tail is returning wrong result
  • Loading branch information
adriaanm committed Jan 24, 2014
2 parents b75a84b + 7f65b37 commit 5dd4633
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 24 deletions.
20 changes: 11 additions & 9 deletions src/library/scala/collection/immutable/HashMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -247,15 +247,17 @@ object HashMap extends ImmutableMapFactory[HashMap] with BitOperations.Int {
override def removed0(key: A, hash: Int, level: Int): HashMap[A, B] =
if (hash == this.hash) {
val kvs1 = kvs - key
if (kvs1 eq kvs)
this
else if (kvs1.isEmpty)
HashMap.empty[A,B]
else if(kvs1.tail.isEmpty) {
val kv = kvs1.head
new HashMap1[A,B](kv._1,hash,kv._2,kv)
} else
new HashMapCollision1(hash, kvs1)
kvs1.size match {
case 0 =>
HashMap.empty[A,B]
case 1 =>
val kv = kvs1.head
new HashMap1(kv._1,hash,kv._2,kv)
case x if x == kvs.size =>
this
case _ =>
new HashMapCollision1(hash, kvs1)
}
} else this

override protected def filter0(p: ((A, B)) => Boolean, negate: Boolean, level: Int, buffer: Array[HashMap[A, B @uV]], offset0: Int): HashMap[A, B] = {
Expand Down
16 changes: 8 additions & 8 deletions src/library/scala/collection/immutable/ListMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ extends AbstractMap[A, B]
def hasNext = !self.isEmpty
def next(): (A,B) =
if (!hasNext) throw new NoSuchElementException("next on empty iterator")
else { val res = (self.key, self.value); self = self.tail; res }
else { val res = (self.key, self.value); self = self.next; res }
}.toList.reverseIterator

protected def key: A = throw new NoSuchElementException("empty map")
protected def value: B = throw new NoSuchElementException("empty map")
override def tail: ListMap[A, B] = throw new NoSuchElementException("empty map")
protected def next: ListMap[A, B] = throw new NoSuchElementException("empty map")

/** This class represents an entry in the `ListMap`.
*/
Expand All @@ -142,7 +142,7 @@ extends AbstractMap[A, B]
override def size: Int = size0(this, 0)

// to allow tail recursion and prevent stack overflows
@tailrec private def size0(cur: ListMap[A, B1], acc: Int): Int = if (cur.isEmpty) acc else size0(cur.tail, acc + 1)
@tailrec private def size0(cur: ListMap[A, B1], acc: Int): Int = if (cur.isEmpty) acc else size0(cur.next, acc + 1)

/** Is this an empty map?
*
Expand All @@ -163,7 +163,7 @@ extends AbstractMap[A, B]
@tailrec private def apply0(cur: ListMap[A, B1], k: A): B1 =
if (cur.isEmpty) throw new NoSuchElementException("key not found: "+k)
else if (k == cur.key) cur.value
else apply0(cur.tail, k)
else apply0(cur.next, k)

/** Checks if this map maps `key` to a value and return the
* value if it exists.
Expand All @@ -175,7 +175,7 @@ extends AbstractMap[A, B]

@tailrec private def get0(cur: ListMap[A, B1], k: A): Option[B1] =
if (k == cur.key) Some(cur.value)
else if (cur.tail.nonEmpty) get0(cur.tail, k) else None
else if (cur.next.nonEmpty) get0(cur.next, k) else None

/** This method allows one to create a new map with an additional mapping
* from `key` to `value`. If the map contains already a mapping for `key`,
Expand All @@ -196,12 +196,12 @@ extends AbstractMap[A, B]
if (cur.isEmpty)
acc.last
else if (k == cur.key)
(cur.tail /: acc) {
(cur.next /: acc) {
case (t, h) => val tt = t; new tt.Node(h.key, h.value) // SI-7459
}
else
remove0(k, cur.tail, cur::acc)
remove0(k, cur.next, cur::acc)

override def tail: ListMap[A, B1] = ListMap.this
override protected def next: ListMap[A, B1] = ListMap.this
}
}
7 changes: 0 additions & 7 deletions test/files/run/t6261.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@ import scala.collection.immutable._

object Test extends App {

def test0() {
val m=ListMap(1->2,3->4)
if(m.tail ne m.tail)
println("ListMap.tail uses a builder, so it is not O(1)")
}

def test1() {
// test that a HashTrieMap with one leaf element is not created!
val x = HashMap.empty + (1->1) + (2->2)
Expand Down Expand Up @@ -92,7 +86,6 @@ object Test extends App {
// StructureTests.printStructure(z)
require(z.size == 2 && z.contains(a._1) && z.contains(c._1))
}
test0()
test1()
test2()
test3()
Expand Down
6 changes: 6 additions & 0 deletions test/files/run/t7445.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import scala.collection.immutable.ListMap

object Test extends App {
val a = ListMap(1 -> 1, 2 -> 2, 3 -> 3, 4 -> 4, 5 -> 5);
require(a.tail == ListMap(2 -> 2, 3 -> 3, 4 -> 4, 5 -> 5));
}

0 comments on commit 5dd4633

Please sign in to comment.