Skip to content

Commit

Permalink
Merge pull request scala#8421 from lrytz/t11737
Browse files Browse the repository at this point in the history
Don't use iterator to modify the underlying Map in `mapValuesInPlace`
  • Loading branch information
lrytz authored Sep 16, 2019
2 parents a3791d4 + 3e3f22b commit 4b35778
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 2 deletions.
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ val mimaFilterSettings = Seq {
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.immutable.VectorPointer.gotoPosWritable1"),
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.immutable.VectorPointer.gotoPosWritable1$default$4"),
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.immutable.VectorPointer.nullSlotAndCopy$default$3"),
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.mutable.HashMap.mapValuesInPlaceImpl"),
),
}

Expand Down
15 changes: 15 additions & 0 deletions src/library/scala/collection/mutable/HashMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,21 @@ class HashMap[K, V](initialCapacity: Int, loadFactor: Double)
this
}

// TODO in 2.14: rename to `mapValuesInPlace` and override the base version (not binary compatible)
private[mutable] def mapValuesInPlaceImpl(f: (K, V) => V): this.type = {
val len = table.length
var i = 0
while (i < len) {
var n = table(i)
while (n ne null) {
n.value = f(n.key, n.value)
n = n.next
}
i += 1
}
this
}

override def mapFactory: MapFactory[HashMap] = HashMap

override protected[this] def stringPrefix = "HashMap"
Expand Down
13 changes: 11 additions & 2 deletions src/library/scala/collection/mutable/Map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,17 @@ trait MapOps[K, V, +CC[X, Y] <: MapOps[X, Y, CC, _], +C <: MapOps[K, V, CC, C]]
* @return the map itself.
*/
def mapValuesInPlace(f: (K, V) => V): this.type = {
iterator foreach {
case (key, value) => update(key, f(key, value))
if (nonEmpty) this match {
case hm: mutable.HashMap[K, V] => hm.mapValuesInPlaceImpl(f)
case _ =>
val array = this.toArray[Any]
val arrayLength = array.length
var i = 0
while (i < arrayLength) {
val (k, v) = array(i).asInstanceOf[(K, V)]
update(k, f(k, v))
i += 1
}
}
this
}
Expand Down
7 changes: 7 additions & 0 deletions test/junit/scala/collection/mutable/HashMapTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,13 @@ class HashMapTest {
assertEquals(hashMapCount4.updateWith(countingKey2)(transform), None)
assertEquals(2, count) // once during hashtable construction, once during updateWith
assertEquals(hashMapCount4, mutable.HashMap(countingKey1 -> "a"))
}

@Test
def t11737(): Unit = {
val m = mutable.HashMap(
0 -> 1, 1 -> 1, 2 -> 1, 3 -> 1, 4 -> 1, 5 -> 1, 6 -> 1, 7 -> 1, 8 -> 1, 9 -> 1, 241 -> 1)
.mapValuesInPlace((_, _) => -1)
assertTrue(m.toString, m.forall(_._2 == -1))
}
}

0 comments on commit 4b35778

Please sign in to comment.