Skip to content

Commit

Permalink
Remove multi-parameter, vararg variant of + and -
Browse files Browse the repository at this point in the history
Ref scala/scala-dev#496
Ref scala/scala3#4311 (comment)

Using the `compileTimeError` annotation, this removes the multi-parameter, vararg variants of `+` and `-` operators from the collections, and instructs the users to migrate to `++` and `--` instead.

This is part of an effort to remove the ambiguity between tuple literal and parameter list when in an infixed operator notation.
  • Loading branch information
eed3si9n committed Feb 24, 2019
1 parent 6b7657a commit 6ad20eb
Show file tree
Hide file tree
Showing 18 changed files with 88 additions and 67 deletions.
8 changes: 4 additions & 4 deletions src/library/scala/collection/Map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package scala
package collection

import scala.annotation.compileTimeError
import scala.annotation.unchecked.uncheckedVariance
import scala.collection.generic.DefaultSerializable
import scala.collection.mutable.StringBuilder
Expand Down Expand Up @@ -62,7 +63,7 @@ trait Map[K, +V]
// These two methods are not in MapOps so that MapView is not forced to implement them
@deprecated("Use - or remove on an immutable Map", "2.13.0")
def - (key: K): Map[K, V]
@deprecated("Use -- or removeAll on an immutable Map", "2.13.0")
@compileTimeError("use -- or removeAll on an immutable Map instead")
def - (key1: K, key2: K, keys: K*): Map[K, V]

@deprecatedOverriding("Compatibility override", since="2.13.0")
Expand Down Expand Up @@ -301,9 +302,8 @@ trait MapOps[K, +V, +CC[_, _] <: IterableOps[_, AnyConstr, _], +C]
def + [V1 >: V](kv: (K, V1)): CC[K, V1] =
mapFactory.from(new View.Appended(toIterable, kv))

@deprecated("Use ++ with an explicit collection argument instead of + with varargs", "2.13.0")
def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] =
mapFactory.from(new View.Concat(new View.Appended(new View.Appended(toIterable, elem1), elem2), elems))
@compileTimeError("use ++ with an explicit collection argument instead")
def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] = ???

@deprecated("Consider requiring an immutable Map.", "2.13.0")
@`inline` def -- (keys: IterableOnce[K]): C = {
Expand Down
9 changes: 5 additions & 4 deletions src/library/scala/collection/Set.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package scala
package collection

import scala.annotation.compileTimeError
import scala.language.higherKinds
import scala.util.hashing.MurmurHash3
import java.lang.String
Expand Down Expand Up @@ -174,8 +175,8 @@ trait SetOps[A, +CC[_], +C <: SetOps[A, CC, C]]
@deprecated("Consider requiring an immutable Set or fall back to Set.diff", "2.13.0")
def - (elem: A): C = diff(Set(elem))

@deprecated("Use &- with an explicit collection argument instead of - with varargs", "2.13.0")
def - (elem1: A, elem2: A, elems: A*): C = diff(elems.toSet + elem1 + elem2)
@compileTimeError("use &- with an explicit collection argument instead")
def - (elem1: A, elem2: A, elems: A*): C = ???

// The implicit dummy parameter is necessary to avoid erased signature clashes
// between this `concat` and the polymorphic one defined in `IterableOps`.
Expand All @@ -202,8 +203,8 @@ trait SetOps[A, +CC[_], +C <: SetOps[A, CC, C]]
@deprecated("Consider requiring an immutable Set or fall back to Set.union", "2.13.0")
def + (elem: A): C = fromSpecific(new View.Appended(toIterable, elem))

@deprecated("Use ++ with an explicit collection argument instead of + with varargs", "2.13.0")
def + (elem1: A, elem2: A, elems: A*): C = fromSpecific(new View.Concat(new View.Appended(new View.Appended(toIterable, elem1), elem2), elems))
@compileTimeError("use ++ with an explicit collection argument instead")
def + (elem1: A, elem2: A, elems: A*): C = ???

/** Alias for `concat` */
@`inline` final def ++ (that: collection.IterableOnce[A])(implicit dummy: DummyImplicit): C = concat(that)
Expand Down
6 changes: 3 additions & 3 deletions src/library/scala/collection/SortedMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
package scala
package collection

import scala.annotation.implicitNotFound
import scala.annotation.{ compileTimeError, implicitNotFound }
import scala.collection.immutable.TreeMap
import scala.language.higherKinds
import scala.annotation.unchecked.uncheckedVariance
Expand Down Expand Up @@ -189,8 +189,8 @@ trait SortedMapOps[K, +V, +CC[X, Y] <: Map[X, Y] with SortedMapOps[X, Y, CC, _],
@deprecated("Consider requiring an immutable Map or fall back to Map.concat", "2.13.0")
override def + [V1 >: V](kv: (K, V1)): CC[K, V1] = sortedMapFactory.from(new View.Appended(toIterable, kv))

@deprecated("Use ++ with an explicit collection argument instead of + with varargs", "2.13.0")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] = sortedMapFactory.from(new View.Concat(new View.Appended(new View.Appended(toIterable, elem1), elem2), elems))
@compileTimeError("use ++ with an explicit collection argument instead")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] = ???

// TODO Also override mapValues
}
Expand Down
8 changes: 3 additions & 5 deletions src/library/scala/collection/StrictOptimizedMapOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

package scala.collection

import scala.annotation.compileTimeError
import scala.language.higherKinds

/**
Expand All @@ -38,9 +39,6 @@ trait StrictOptimizedMapOps[K, +V, +CC[_, _] <: IterableOps[_, AnyConstr, _], +C
override def collect[K2, V2](pf: PartialFunction[(K, V), (K2, V2)]): CC[K2, V2] =
strictOptimizedCollect(mapFactory.newBuilder, pf)

@deprecated("Use ++ with an explicit collection argument instead of + with varargs", "2.13.0")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] = {
val m = ((this + elem1).asInstanceOf[Map[K, V]] + elem2).asInstanceOf[CC[K, V1]]
if(elems.isEmpty) m else m.concat(elems).asInstanceOf[CC[K, V1]]
}
@compileTimeError("use ++ with an explicit collection argument instead")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] = ???
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

package scala.collection

import scala.annotation.implicitNotFound
import scala.annotation.{ compileTimeError, implicitNotFound }

/**
* Trait that overrides sorted map operations to take advantage of strict builders.
Expand All @@ -38,9 +38,6 @@ trait StrictOptimizedSortedMapOps[K, +V, +CC[X, Y] <: Map[X, Y] with SortedMapOp
override def collect[K2, V2](pf: PartialFunction[(K, V), (K2, V2)])(implicit @implicitNotFound(SortedMapOps.ordMsg) ordering: Ordering[K2]): CC[K2, V2] =
strictOptimizedCollect(sortedMapFactory.newBuilder, pf)

@deprecated("Use ++ with an explicit collection argument instead of + with varargs", "2.13.0")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] = {
val m = ((this + elem1).asInstanceOf[Map[K, V]] + elem2).asInstanceOf[CC[K, V1]]
if(elems.isEmpty) m else m.concat(elems).asInstanceOf[CC[K, V1]]
}
@compileTimeError("use ++ with an explicit collection argument instead")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): CC[K, V1] = ???
}
6 changes: 3 additions & 3 deletions src/library/scala/collection/generic/Subtractable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ package collection
package generic

import scala.collection.IterableOnce

import scala.annotation.compileTimeError

/** This trait represents collection-like objects that can be reduced
* using a '+' operator. It defines variants of `-` and `--`
Expand Down Expand Up @@ -54,8 +54,8 @@ trait Subtractable[A, +Repr <: Subtractable[A, Repr]] { self =>
* @return a new $coll that contains all elements of the current $coll
* except one less occurrence of each of the given elements.
*/
def -(elem1: A, elem2: A, elems: A*): Repr =
this - elem1 - elem2 -- elems
@compileTimeError("use -- with an explicit collection argument instead")
def -(elem1: A, elem2: A, elems: A*): Repr = ???

/** Creates a new $coll from this $coll by removing all elements of another
* collection.
Expand Down
5 changes: 3 additions & 2 deletions src/library/scala/collection/immutable/Map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ package scala
package collection
package immutable

import scala.annotation.compileTimeError
import scala.annotation.unchecked.uncheckedVariance
import scala.collection.generic.DefaultSerializable
import scala.collection.immutable.Map.Map4
Expand Down Expand Up @@ -76,8 +77,8 @@ trait MapOps[K, +V, +CC[X, +Y] <: MapOps[X, Y, CC, _], +C <: MapOps[K, V, CC, C]
@deprecatedOverriding("This method should be final, but is not due to scala/bug#10853", "2.13.0")
/*@`inline` final*/ def - (key: K): C = removed(key)

@deprecated("Use -- with an explicit collection", "2.13.0")
def - (key1: K, key2: K, keys: K*): C = removed(key1).removed(key2).removedAll(keys)
@compileTimeError("use -- with an explicit collection argument instead")
def - (key1: K, key2: K, keys: K*): C = ???

/** Creates a new $coll from this $coll by removing all elements of another
* collection.
Expand Down
9 changes: 3 additions & 6 deletions src/library/scala/collection/mutable/AnyRefMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ package collection
package mutable

import java.io.{ObjectInputStream, ObjectOutputStream}

import scala.annotation.compileTimeError
import scala.collection.generic.DefaultSerializationProxy


Expand Down Expand Up @@ -379,11 +379,8 @@ class AnyRefMap[K <: AnyRef, V] private[collection] (defaultEntry: K => V, initi
@deprecated("Consider requiring an immutable Map or fall back to Map.concat", "2.13.0")
override def + [V1 >: V](kv: (K, V1)): AnyRefMap[K, V1] = AnyRefMap.from(new View.Appended(toIterable, kv))

@deprecated("Use ++ with an explicit collection argument instead of + with varargs", "2.13.0")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): AnyRefMap[K, V1] = {
val m = this + elem1 + elem2
if(elems.isEmpty) m else m.concat(elems)
}
@compileTimeError("use ++ with an explicit collection argument instead")
override def + [V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): AnyRefMap[K, V1] = ???

override def concat[V2 >: V](xs: scala.collection.IterableOnce[(K, V2)]): AnyRefMap[K, V2] = {
val arm = clone().asInstanceOf[AnyRefMap[K, V2]]
Expand Down
5 changes: 3 additions & 2 deletions src/library/scala/collection/mutable/Growable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package collection
package mutable

import scala.collection.IterableOnce
import scala.annotation.compileTimeError

/** This trait forms part of collections that can be augmented
* using a `+=` operator and that can be cleared of all elements using
Expand Down Expand Up @@ -48,8 +49,8 @@ trait Growable[-A] extends Clearable {
* @param elems the remaining elements to $add.
* @return the $coll itself
*/
@deprecated("Use `++=` (addAll) instead of varargs `+=`", "2.13.0")
@`inline` final def += (elem1: A, elem2: A, elems: A*): this.type = this += elem1 += elem2 ++= (elems: IterableOnce[A])
@compileTimeError("use `++=` or addAll instead")
@`inline` final def += (elem1: A, elem2: A, elems: A*): this.type = ???

/** ${Add}s all elements produced by an IterableOnce to this $coll.
*
Expand Down
9 changes: 3 additions & 6 deletions src/library/scala/collection/mutable/LongMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ package scala.collection
package mutable

import java.io.{ObjectInputStream, ObjectOutputStream}

import scala.annotation.compileTimeError
import scala.collection.generic.DefaultSerializationProxy


Expand Down Expand Up @@ -454,11 +454,8 @@ final class LongMap[V] private[collection] (defaultEntry: Long => V, initialBuff
lm
}

@deprecated("Use ++ with an explicit collection argument instead of + with varargs", "2.13.0")
override def + [V1 >: V](elem1: (Long, V1), elem2: (Long, V1), elems: (Long, V1)*): LongMap[V1] = {
val m = this + elem1 + elem2
if(elems.isEmpty) m else m.concat(elems)
}
@compileTimeError("use ++ with an explicit collection argument instead")
override def +[V1 >: V](elem1: (Long, V1), elem2: (Long, V1), elems: (Long, V1)*): LongMap[V1] = ???

override def concat[V1 >: V](xs: scala.collection.IterableOnce[(Long, V1)]): LongMap[V1] = {
val lm = clone().asInstanceOf[LongMap[V1]]
Expand Down
5 changes: 3 additions & 2 deletions src/library/scala/collection/mutable/Map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ package scala
package collection
package mutable

import scala.annotation.compileTimeError
import scala.language.higherKinds

/** Base type of mutable Maps */
Expand Down Expand Up @@ -77,9 +78,9 @@ trait MapOps[K, V, +CC[X, Y] <: MapOps[X, Y, CC, _], +C <: MapOps[K, V, CC, C]]
@deprecatedOverriding("This method should be final, but is not due to scala/bug#10853", "2.13.0")
/*final*/ def - (key: K): C = clone() -= key

@deprecated("Use -- or removeAll on an immutable Map", "2.13.0")
@compileTimeError("use -- or removeAll on an immutable Map")
@deprecatedOverriding("This method should be final, but is not due to scala/bug#10853", "2.13.0")
/*final*/ def - (key1: K, key2: K, keys: K*): C = clone() -= key1 -= key2 --= keys
/*final*/ def - (key1: K, key2: K, keys: K*): C = ???

/** Adds a new key/value pair to this map and optionally returns previously bound value.
* If the map already contains a
Expand Down
9 changes: 3 additions & 6 deletions src/library/scala/collection/mutable/Shrinkable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
package scala
package collection.mutable

import scala.annotation.tailrec
import scala.annotation.{ compileTimeError, tailrec }

/** This trait forms part of collections that can be reduced
* using a `-=` operator.
Expand Down Expand Up @@ -42,11 +42,8 @@ trait Shrinkable[-A] {
* @param elems the remaining elements to remove.
* @return the $coll itself
*/
def -= (elem1: A, elem2: A, elems: A*): this.type = {
this -= elem1
this -= elem2
this --= elems
}
@compileTimeError("use --= or subtractAll with an explicit collection argument instead")
def -= (elem1: A, elem2: A, elems: A*): this.type = ???

/** Removes all elements produced by an iterator from this $coll.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,11 @@ class MapAppendBenchmark {
bh.consume(m)
}

@Benchmark def plus2(bh: Blackhole): Unit = {
@Benchmark def plusSeq(bh: Blackhole): Unit = {
var m = Map.empty[Int, Unit]
var i = 0
while(i < size) {
m = m + ((i -> ()), ((i+size) -> ()))
i += 1
}
bh.consume(m)
}

@Benchmark def plus2Empty(bh: Blackhole): Unit = {
val empty = IndexedSeq.empty
var m = Map.empty[Int, Unit]
var i = 0
while(i < size) {
m = m + ((i -> ()), ((i+size) -> ()), empty: _*)
m = m ++ Seq((i -> ()), ((i+size) -> ()))
i += 1
}
bh.consume(m)
Expand Down
25 changes: 25 additions & 0 deletions test/files/neg/collection-removal.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
collection-removal.scala:4: error: use of method + in trait MapOps is unsupported: use ++ with an explicit collection argument instead
val m1 = scala.collection.Map("foo" -> 1) + (("bar" -> 2), ("baz" -> 3))
^
collection-removal.scala:5: error: use of method - in trait Map is unsupported: use -- or removeAll on an immutable Map instead
val m2 = scala.collection.Map("foo" -> 1) - ("foo", "bar", "baz")
^
collection-removal.scala:7: error: use of method + in trait SortedMapOps is unsupported: use ++ with an explicit collection argument instead
val sm1 = scala.collection.SortedMap("foo" -> 1) + (("bar" -> 2), ("baz" -> 3))
^
collection-removal.scala:9: error: use of method - in trait MapOps is unsupported: use -- with an explicit collection argument instead
val im1 = scala.collection.immutable.Map("foo" -> 1) - ("foo", "bar", "baz")
^
collection-removal.scala:11: error: use of method + in class LongMap is unsupported: use ++ with an explicit collection argument instead
val lm1 = scala.collection.mutable.LongMap(1L -> 1) + ((2L -> 2), (3L -> 3))
^
collection-removal.scala:13: error: use of method - in trait SetOps is unsupported: use &- with an explicit collection argument instead
val s1 = scala.collection.Set(1, 2) - (1, 2, 3)
^
collection-removal.scala:14: error: use of method + in trait SetOps is unsupported: use ++ with an explicit collection argument instead
val s2 = scala.collection.Set(1, 2) + (1, 2, 3)
^
collection-removal.scala:16: error: use of method += in trait Growable is unsupported: use `++=` or addAll instead
val lb1 = scala.collection.mutable.ListBuffer(1) += (1, 2, 3)
^
8 errors found
17 changes: 17 additions & 0 deletions test/files/neg/collection-removal.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// scalac: -deprecation -Xfatal-warnings

object Test1 {
val m1 = scala.collection.Map("foo" -> 1) + (("bar" -> 2), ("baz" -> 3))
val m2 = scala.collection.Map("foo" -> 1) - ("foo", "bar", "baz")

val sm1 = scala.collection.SortedMap("foo" -> 1) + (("bar" -> 2), ("baz" -> 3))

val im1 = scala.collection.immutable.Map("foo" -> 1) - ("foo", "bar", "baz")

val lm1 = scala.collection.mutable.LongMap(1L -> 1) + ((2L -> 2), (3L -> 3))

val s1 = scala.collection.Set(1, 2) - (1, 2, 3)
val s2 = scala.collection.Set(1, 2) + (1, 2, 3)

val lb1 = scala.collection.mutable.ListBuffer(1) += (1, 2, 3)
}
4 changes: 2 additions & 2 deletions test/files/run/collections.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ object Test extends App {
println("***** "+msg+":")
var s = s0
s = s.clone() += 2
s = s.clone += (3, 4000, 10000)
s = s.clone ++= Seq(3, 4000, 10000)
println("test1: "+sum(s))
time {
s = s ++ (List.range(0, iters) map (2*))
Expand Down Expand Up @@ -88,7 +88,7 @@ object Test extends App {
println("***** "+msg+":")
var s = s0
s = s.clone() += (2 -> 2)
s = s.clone() += (3 -> 3, 4000 -> 4000, 10000 -> 10000)
s = s.clone() ++= Seq(3 -> 3, 4000 -> 4000, 10000 -> 10000)
println("test1: "+sum(s map (_._2)))
time {
s = s ++ (List.range(0, iters) map (x => x * 2 -> x * 2))
Expand Down
4 changes: 2 additions & 2 deletions test/junit/scala/collection/SetMapConsistencyTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ class SetMapConsistencyTest {

assert {
val lm2 = new LongMap[String](_.toString)
lm2 += (5L -> "fish", 0L -> "unicorn")
lm2 ++= Seq(5L -> "fish", 0L -> "unicorn")
val hm2 = (new HashMap[Long,String]) ++= lm2
List(Long.MinValue, 0L, 1L, 5L).forall(i =>
lm2.get(i) == hm2.get(i) &&
Expand Down Expand Up @@ -443,7 +443,7 @@ class SetMapConsistencyTest {

assert {
val arm2 = new AnyRefMap[String, String](x => if (x==null) "null" else x)
arm2 += ("cod" -> "fish", "Rarity" -> "unicorn")
arm2 ++= Seq("cod" -> "fish", "Rarity" -> "unicorn")
val hm2 = (new HashMap[String,String]) ++= arm2
List(null, "cod", "sparrow", "Rarity").forall(i =>
arm2.get(i) == hm2.get(i) &&
Expand Down
2 changes: 1 addition & 1 deletion test/junit/scala/collection/mutable/ArrayDequeTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class ArrayDequeTest {
assertEquals(buffer.reverse, buffer2.reverse)
}

apply(_ += (1, 2, 3, 4, 5))
apply(_ ++= Seq(1, 2, 3, 4, 5))
apply(_.prepend(6).prepend(7).prepend(8))
apply(_.trimStart(2))
apply(_.trimEnd(3))
Expand Down

0 comments on commit 6ad20eb

Please sign in to comment.