SI-7269 Rework MapLike#retains to account for desugaring change

`MapLike#retains` contains a for-comprehension that relied on the strict
`filter` by its generator. You can't, in general, iterate a mutable map
and remove items in the same pass.

Here's the history of the desugaring of:

    def retain[A, B](thiz: mutable.Map[A, B])(p: (A, B) => Boolean): thiz.type = {
      thiz.foreach {
        case (k, v) =>
          if (p(k, v)) thiz -= k
      }

Before regression (c82ecabad6~1):

    thiz.filter(((check$ifrefutable$1) => check$ifrefutable$1: @scala.unchecked match {
       case scala.Tuple2((k @ _), (v @ _)) => true
       case _ => false
    })).withFilter(((x$1) => x$1: @scala.unchecked match {
       case scala.Tuple2((k @ _), (v @ _)) => p(k, v).unary_$bang
    })).foreach(((x$2) => x$2: @scala.unchecked match {
       case scala.Tuple2((k @ _), (v @ _)) => thiz.$minus$eq(k)
    }));

After regression (c82ecabad6, which incorrectly assumed in the parser that
no filter is required for isInstanceOf[Tuple2])

    thiz.withFilter(((x$1) => x$1: @scala.unchecked match {
       case scala.Tuple2((k @ _), (v @ _)) => p(k, v).unary_$bang
    })).foreach(((x$2) => x$2: @scala.unchecked match {
      case scala.Tuple2((k @ _), (v @ _)) => thiz.$minus$eq(k)
    }));

After the reversion of c82ecabad6, v2.10.2
This is also after 365bb2b4e, which uses `withFilter` rather than `filter`.

    thiz.withFilter(((check$q$1) => check$ifrefutable$1: @scala.unchecked match {
       case scala.Tuple2((k @ _), (v @ _)) => true
       case _ => false
    })).withFilter(((x$1) => x$1: @scala.unchecked match {
       case scala.Tuple2((k @ _), (v @ _)) => p(k, v).unary_$bang
    })).foreach(((x$2) => x$2: @scala.unchecked match {
      case scala.Tuple2((k @ _), (v @ _)) => thiz.$minus$eq(k)
    }));

This commit does the same as `SetLike#retains`, and converts the map to
an immutable list before the rest of the operation.

3 files changed, 37 insertions, 3 deletions

diff --git a/src/library/scala/collection/mutable/MapLike.scala b/src/library/scala/collection/mutable/MapLike.scala
index a53aa3b76a..42e5a0a4c4 100644
--- a/src/library/scala/collection/mutable/MapLike.scala
+++ b/src/library/scala/collection/mutable/MapLike.scala
@@ -209,8 +209,8 @@ trait MapLike[A, B, +This <: MapLike[A, B, This] with Map[A, B]]
    * @param p  The test predicate
    */
   def retain(p: (A, B) => Boolean): this.type = {
-    for ((k, v) <- this.seq ; if !p(k, v))
-      this -= k
+    for ((k, v) <- this.toList) // SI-7269 toList avoids ConcurrentModificationException
+      if (!p(k, v)) this -= k
 
     this
   }
diff --git a/src/library/scala/collection/mutable/SetLike.scala b/src/library/scala/collection/mutable/SetLike.scala
index 01f87447ae..71da4c89dc 100644
--- a/src/library/scala/collection/mutable/SetLike.scala
+++ b/src/library/scala/collection/mutable/SetLike.scala
@@ -120,7 +120,9 @@ trait SetLike[A, +This <: SetLike[A, This] with Set[A]]
    *             which `p` returns `true` are retained in the set; all others
    *             are removed.
    */
-  def retain(p: A => Boolean): Unit = for (elem <- this.toList) if (!p(elem)) this -= elem
+  def retain(p: A => Boolean): Unit =
+    for (elem <- this.toList) // SI-7269 toList avoids ConcurrentModificationException
+      if (!p(elem)) this -= elem
 
   /** Removes all elements from the set. After this operation is completed,
    *  the set will be empty.
diff --git a/test/files/run/t7269.scala b/test/files/run/t7269.scala
new file mode 100644
index 0000000000..d22e57dfee
--- /dev/null
+++ b/test/files/run/t7269.scala
@@ -0,0 +1,32 @@
+import scala.collection.JavaConversions._
+import scala.collection.mutable
+
+object Test extends App {
+
+  def testMap(): Unit = {
+    val mapJ = new java.util.HashMap[Int, String]
+    val mapS: mutable.Map[Int, String] = mapJ
+
+    (10 to 20).foreach(i => mapS += ((i, i.toString)))
+    assert(11 == mapS.size)
+
+    // ConcurrentModificationException thrown in the following line
+    mapS.retain((i, str) => i % 2 == 0)
+    assert(6 == mapS.size)
+  }
+
+  def testSet(): Unit = {
+    val mapJ = new java.util.HashSet[Int]
+    val mapS: mutable.Set[Int] = mapJ
+
+    (10 to 20).foreach(i => mapS += i)
+    assert(11 == mapS.size)
+
+    // ConcurrentModificationException thrown in the following line
+    mapS.retain((i) => i % 2 == 0)
+    assert(6 == mapS.size)
+  }
+
+  testSet()
+  testMap()
+}