LazyList.from(1) match { case LazyList(x, xs @ _*) => xs } never terminates

[smarter]

Same for Stream.from(1).view.toSeq which did terminate in 2.12. The behavior on unapply arises because we go through UnapplySeqWrapper#drop: https://github.com/scala/scala/blob/f23f96c36a3cc0a44a60d28bac0200873686c410/src/library/scala/collection/Factory.scala#L315

/cc @NthPortal, the definitive expert on LazyList :).

[smarter]

https://github.com/scala/scala/blob/2.13.x/test/files/run/matchonstream.scala should have caught this but didn't because the binding for x got dead-code eliminated, however it did catch it when I tried to move dotty to the 2.13 stdlib because we don't do DCE :).

[smarter]

@szeiger I assigned this to the 2.13.1 milestone since it seems pretty critical to me, but it might be too late for that already.

[NthPortal]

it's not clear to me that LazyList.from(1).view.toSeq ought to terminate; i.e. that it ought to remember the type it was backed by. nevertheless, LazyList.from(1) match { case LazyList(x, xs @ _*) => xs } certainly ought to work.

I haven't looked yet, but I'm concerned that fixing LazyList.unapplySeq is going to be impossible for bincompat reasons, though it's possible the return types are compatible. I will have a look in a bit it is binary compatible.

It's also possible that the problem is view.toSeq being used for implementing .unapply, making it implicitly non-lazy (at least without changing what toSeq returns); this would seem to be an error in following the "default to lazy" design goal of the new collections.

[julienrf]

I think that the current behavior is correct, calling toSeq on a View does evaluate the view. The toSeq operation is a noop only on Seq subtypes, which is not the case of SeqView.

[SethTisue]

I agree with Julien (and with Nth's "it's not clear to me that...").

In the 2.13 design, going to a view doesn't preserve exact type information of the input collection; a SeqView is just a SeqView, end of story. This is an intentional simplification that makes the whole thing a lot simpler and more predictable than the 2.8 views, both for users and for implementers. It's a big part of why the old views design was not workable and the new one is.

[smarter]

So why isn't a view of the LazyList just the LazyList itself?

[SethTisue]

because in the new design views aren't collections and collections aren't views

views are just reusable iterators

I'm not sure I understand your thinking here, but it sounds to me like you're still thinking in terms of what 2.8-12 views were. but the 2.13 views are something else, something new, that you should approach with fresh expectations. a different design solution that addresses some of the same problems

[NthPortal]

If you think about it a little differently, a View in 2.13 is a set of lifted operations, much like a Stream in Java. You wouldn't (or at least I wouldn't) expect j.u.List.of(1, 2, 3, 4).stream().collect(Collectors.toList()) to necessarily return the input collection even though it contains no lifted operations; similarly with LazyList.from(1).view.toSeq

[smarter]

But are there any intentional semantic differences between a LazyList and a view of a LazyList?

[szeiger]

LazyList memoizes the results of a computation, a View never does that. Views are reified iterator operations:

scala> val a = scala.collection.immutable.LazyList(1, 2, 3).map(println)
a: scala.collection.immutable.LazyList[Unit] = LazyList(<not computed>)

scala> a.foreach(println)
1
()
2
()
3
()

scala> a.foreach(println)
()
()
()

scala> val a = scala.collection.immutable.LazyList(1, 2, 3).view.map(println)
a: scala.collection.SeqView[Unit] = SeqView(<not computed>)

scala> a.foreach(println)
1
()
2
()
3
()

scala> a.foreach(println)
1
()
2
()
3
()

The new behavior of toSeq is also expected. In the new design views are no longer tied to the type of the collection that created a view. Calling toSeq on a View will always use the default Seq implementation, no matter from which collection type the View was originally created.

[SethTisue]

how about the unapply aspect of this, is it fixable?

[NthPortal]

@SethTisue I believe scala/scala#8340 should fix that

[julienrf]

@smarter I’m curious to know the motivation for using a view over a lazy list (or a Stream in 2.12)

[smarter]

I imagine the motivation is to do something lazily on any kind of collection, this is what UnapplySeqWrapper was trying to do, except it doesn't behave as one might expect on lazy collections.

[NthPortal]

I think the purpose of UnapplySeqWrapper is to have a zero-cost extractor for Seqs, not to be lazy particularly. Although it's not obvious to me why it should accept SeqOps that aren't Seqs.

[julienrf]

Although it's not obvious to me why it should accept SeqOps that aren't Seqs.

Indeed, we could just simplify the signature and make UnapplySeqWrapper take a Seq[A] rather than a SeqOps[A, Seq, Seq[A]], and update the factory traits accordingly. But this would break binary compatibility and, most importantly, this would change nothing to our problem. The problem here is that UnapplySeqWrapper can be used by any subtype of scala.collection.Seq, but it has a method that returns a scala.Seq (which is an alias to scala.collection.immutable.Seq), that’s why we have to go through toSeq, and that’s why calling .view.drop(n).toSeq is useful (because in the case of mutable Seqs, we avoid building an entire collection).

[szeiger]

Yes, I think this this can be fixed. Making it elegant and/or binary compatible might be a problem but in principle I see no reason why UnapplySeqWrapper shouldn't use the concrete SeqFactory to get back from the View to a Seq instead of calling toSeq.