Change toString for lazy collections

[NthPortal]

Change toString for LazyList, Stream and views
to say "<not computed>", and for cyclic LazyLists and Streams
to say "<cycle>".

Fixes scala/bug#11515

[joshlemer]

I just don't get why MapView isn't picking up the implementation of toString from View?

[joshlemer]

😕

scala> new MapView[Int, Int] { def get(i: Int) = null; def iterator = null }.toString
res6: String = View(?)

scala> new AbstractMapView[Int, Int] { def get(i: Int) = null; def iterator = null }.toString
res7: String = <function1>

[joshlemer]

I point this out because maybe there's something more fundamentally wonky with the hierarchy

[eed3si9n]

MapView extends MapOps, which extends PartialFunction, which extends Function1.

scala> new scala.collection.MapOps[Int, Int, Map, Map[Int, Int]] {
     |   def iterator: Iterator[(Int, Int)] = ???
     |   protected def coll: Map[Int,Int] = ???
     |   protected def fromSpecific(coll: scala.collection.IterableOnce[(Int, Int)]): Map[Int,Int] = ???
     |   def iterableFactory: scala.collection.IterableFactory[Iterable] = ???
     |   protected def newSpecificBuilder: scala.collection.mutable.Builder[(Int, Int),Map[Int,Int]] = ???
     |   def toIterable: Iterable[(Int, Int)] = ???
     |   def get(key: Int): Option[Int] = ???
     |   def mapFactory: scala.collection.MapFactory[Map] = ???
     | }
res0: scala.collection.MapOps[Int,Int,Map,Map[Int,Int]] = <function1>

[joshlemer]

but why wouldn't View's toString override PartialFunction's toString?

trait MapView[K, +V]
  extends MapOps[K, V, ({ type l[X, Y] = View[(X, Y)] })#l, View[(K, V)]]
    with View[(K, V)] {
...

Doesn't View override whatever is declared in MapOps?

[joshlemer]

@NthPortal 2.13.x

[joshlemer]

If you simply change AbstractMapView to

@SerialVersionUID(3L)
abstract class AbstractMapView[K, +V] extends MapView[K, V]

then you'll get the normal View toString behaviour (still must override MapView#stringPrefix though)

[joshlemer]

I don't really know the exact reasoning behind the convention used, but for example currently AbstractMapView inherits View twice (AbstractView -> View, MapView -> View) which maybe lead to some confusion. If there's not reason for AbstractMapView to inherit from specifically AbstractView, maybe the convention should be changed to go directly to inheriting from Trait instead of through AbstractTrait

[szeiger]

After reading the spec (https://www.scala-lang.org/files/archive/spec/2.11/05-classes-and-objects.html#class-linearization) and finding it confusing, I looked at the results in the REPL and I think I finally understand the spec and how linearization really works.

Let's restate the problem first:

scala> intp.typeOfExpression("null: scala.collection.AbstractMapView[Int, Int]").memberBasedOnName(TermName("toString"), 0L).ownerChain
res27: List[$r.intp.global.Symbol] = List(method toString, trait Function1, package scala, package <root>)

scala> intp.typeOfExpression("null: scala.collection.MapView[Int, Int]").memberBasedOnName(TermName("toString"), 0L).ownerChain
res28: List[$r.intp.global.Symbol] = List(method toString, trait View, package collection, package scala, package <root>)

This is clearly unexpected. The intention is to use the View implementation of toString in both cases.

Everything still looks normal in the base type sequence (https://www.scala-lang.org/files/archive/spec/2.11/03-types.html#base-types-and-member-definitions):

scala> intp.typeOfExpression("null: scala.collection.AbstractMapView[Int, Int]").baseTypeSeq.toList.foreach(x => println("- "+x))
- scala.collection.AbstractMapView[Int,Int]
- scala.collection.MapView[Int,Int]
- scala.collection.AbstractView[(Int, Int)]
- scala.collection.View[(Int, Int)]
- scala.collection.AbstractIterable[(Int, Int)]
- scala.collection.MapOps[Int,Int,[X, Y]scala.collection.View[(X, Y)],scala.collection.View[(Int, Int)]]
- Iterable[(Int, Int)]
- scala.collection.IterableFactoryDefaults[(Int, Int),Iterable] with scala.collection.IterableFactoryDefaults[(Int, Int),scala.collection.View]
- PartialFunction[Int,Int]
- scala.collection.IterableOps[(Int, Int),Iterable,scala.collection.View[(Int, Int)]] with scala.collection.IterableOps[(Int, Int),Iterable,Iterable[(Int, Int)]] with scala.collection.IterableOps[(Int, Int),scala.collection.View,scala.collection.View[(Int, Int)]]
- Int => Int
- Object
- scala.collection.IterableOnceOps[(Int, Int),Iterable,scala.collection.View[(Int, Int)]] with scala.collection.IterableOnceOps[(Int, Int),Iterable,Iterable[(Int, Int)]] with scala.collection.IterableOnceOps[(Int, Int),scala.collection.View,scala.collection.View[(Int, Int)]]
- scala.collection.IterableOnce[(Int, Int)]
- java.io.Serializable
- Any

scala> intp.typeOfExpression("null: scala.collection.MapView[Int, Int]").baseTypeSeq.toList.foreach(x => println("- "+x))
- scala.collection.MapView[Int,Int]
- scala.collection.View[(Int, Int)]
- scala.collection.MapOps[Int,Int,[X, Y]scala.collection.View[(X, Y)],scala.collection.View[(Int, Int)]]
- Iterable[(Int, Int)]
- scala.collection.IterableFactoryDefaults[(Int, Int),Iterable] with scala.collection.IterableFactoryDefaults[(Int, Int),scala.collection.View]
- PartialFunction[Int,Int]
- scala.collection.IterableOps[(Int, Int),Iterable,Iterable[(Int, Int)]] with scala.collection.IterableOps[(Int, Int),scala.collection.View,scala.collection.View[(Int, Int)]] with scala.collection.IterableOps[(Int, Int),Iterable,scala.collection.View[(Int, Int)]]
- Int => Int
- Object
- scala.collection.IterableOnceOps[(Int, Int),Iterable,Iterable[(Int, Int)]] with scala.collection.IterableOnceOps[(Int, Int),scala.collection.View,scala.collection.View[(Int, Int)]] with scala.collection.IterableOnceOps[(Int, Int),Iterable,scala.collection.View[(Int, Int)]]
- scala.collection.IterableOnce[(Int, Int)]
- java.io.Serializable
- Any

But the linearization is different. Note that in AbstractMapView PartialFunction now comes before View:

scala> intp.typeOfExpression("null: scala.collection.AbstractMapView[Int, Int]").baseClasses.toList.foreach(x => println("- "+x))
- class AbstractMapView
- trait MapView
- trait MapOps
- trait PartialFunction
- trait Function1
- class AbstractView
- trait View
- trait Serializable
- class AbstractIterable
- trait Iterable
- trait IterableFactoryDefaults
- trait IterableOps
- trait IterableOnceOps
- trait IterableOnce
- class Object
- class Any

scala> intp.typeOfExpression("null: scala.collection.MapView[Int, Int]").baseClasses.toList.foreach(x => println("- "+x))
- trait MapView
- trait View
- trait Serializable
- trait Iterable
- trait IterableFactoryDefaults
- trait MapOps
- trait PartialFunction
- trait Function1
- trait IterableOps
- trait IterableOnceOps
- trait IterableOnce
- class Object
- class Any

AbstractMapView is defined as

abstract class AbstractMapView[K, +V] extends AbstractView[(K, V)] with MapView[K, V]

Both AbstractView and MapView extend View. MapView also extends Function1. Linearization reverses with, so we get AbstractMapView, (MapView, View, Function1), (AbstractView, View) as a first approximation. View appears twice in this list and in linearization "elements of the right operand replace identical elements of the left operand". This gives us the unexpected linearization AbstractMapView, MapView, Function1, AbstractView, View where Function comes before View.

Judging by this example alone, using the base type sequence for linearization would be a lot more predictable and intuitive, but impossible to implement on the JVM. AbstractView is a class (which has to come first in ... extends ClassType with ...). Its linearization is fixed. We can only pile more overrides on top of it but not reorder its inheritance hierarchy. Linearization must favor the right operand over the left because class types always occur on the right.

[szeiger]

As to what this means for the collections, my first thought was to remove inheritance between abstract collection classes (thereby reducing their usefulness as shared base classes) but that doesn't really change anything because this is a fundamental problem of linearization. Even if the reason for doing it is dealing with classes, the same mechanism is applied to traits.

[NthPortal]

any thoughts on changing ... for LazyList and Stream for cycles to <cycle> while we're at it?

[eed3si9n]

@NthPortal I think <cycle> or <cyclic list> makes sense.

[NthPortal]

do we know the size of the cycle, actually? it would be really nice if we could print that in the string as well.

[NthPortal]

this should be className + "(<not computed>)"