Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put Addable out of its misery #4059

Closed
scabug opened this issue Dec 4, 2010 · 3 comments
Closed

Put Addable out of its misery #4059

scabug opened this issue Dec 4, 2010 · 3 comments
Assignees

Comments

@scabug
Copy link

scabug commented Dec 4, 2010

Reported elsewhere:

scala> val s1 = Set[Any](1, 2, 3)
s1: scala.collection.immutable.Set[Any] = Set(1, 2, 3)

scala> val s2 = Set[Any]("a", "b")
s2: scala.collection.immutable.Set[Any] = Set(a, b)

scala> s1 ++ s2
<console>:8: error: ambiguous reference to overloaded definition,
both method ++ in trait Addable of type (xs: scala.collection.TraversableOnce[Any])scala.collection.immutable.Set[Any]
and  method ++ in trait TraversableLike of type [B >: Any,That](that: scala.collection.TraversableOnce[B])(implicit bf: scala.collection.generic.CanBuildFrom[scala.collection.immutable.Set[Any],B,That])That
match argument types (scala.collection.immutable.Set[Any])
       s1 ++ s2
          ^

I can make this easy. We delete Addable and AddingBuilder. It took me about five seconds, and afterward:

scala> val s1 = Set[Any](1, 2, 3)
s1: scala.collection.immutable.Set[Any] = Set(1, 2, 3)

scala> val s2 = Set[Any]("a", "b")
s2: scala.collection.immutable.Set[Any] = Set(a, b)

scala> s1 ++ s2
res0: scala.collection.immutable.Set[Any] = Set(a, 1, b, 2, 3)

Here is the sad tale of Addable. First off the boat was Map.

-  // note: can't inherit Addable because of variance problems: Map
-  // is covariant in its value type B, but Addable is nonvariant.

We had to suffer longer and more confusingly, but in the end Seq also took its leave:

-  // Note this does not extend Addable because `+` is being phased out of
-  // all Seq-derived classes.

Of course GrowingBuilder has to get its two cents in:

/** The canonical builder for collections that are growable, i.e. that support an
 *  efficient `+=` method which adds an element to the collection.  It is
 *  almost identical to `AddingBuilder`, but necessitated by the existence of
 *  classes which are `Growable` but not `Addable`, which is a result of covariance
 *  interacting surprisingly with any2stringadd thus driving '+' out of the `Seq`
 *  hierarchy.  The tendrils of original sin should never be underestimated.
 *
 *  Addendum: of even greater significance is that '+' on mutable collections now
 *  creates a new collection.  This means using AddingBuilder on them will create
 *  a new intermediate collection for every element given to the builder, taking
 *  '+' from an O(1) to O(n) operation.

And then there was only Set, standing alone as the only Addable collection
and the only customer for AddingBuilder. Which means that "Addable" can as
easily be folded into SetLike, and "AddingBuilder" into SetBuilder, and nothing
is lost except another overload trap.

It's all done: for the low price of a single thumb in the upward direction I can drive nails into both coffins.

@scabug
Copy link
Author

scabug commented Dec 4, 2010

Imported From: https://issues.scala-lang.org/browse/SI-4059?orig=1
Reporter: @paulp

@scabug
Copy link
Author

scabug commented Dec 7, 2010

@odersky said:
I'm OK with this. Let's deprecate for 2.9.

@scabug
Copy link
Author

scabug commented Dec 7, 2010

@paulp said:
(In r23707) Extricated Addable and AddingBuilder from the few classes still
utilizing them and deprecated both. Closes #4059. Already endorsed
by odersky, so no review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants