scala / collection-strawman

Implementation of the new Scala 2.13 Collections
Apache License 2.0
200 stars 72 forks source link

LazyList.#:: not lazy due to parser rewriting of right-associative infix operators #127

Closed pnf closed 6 years ago

pnf commented 7 years ago

Evaluating

  def bleh(i: Int) = {println(s"bleh $i"); i}
  val xs20 = bleh(1) #:: bleh(2) #:: bleh(3) #:: LazyList.empty

will immediately execute all the printlns, even though .force is never called and .evaluated remains false. If one defines a left associative prepend with the same definition as #::, then

    val x21 = LazyList.empty.prepend(bleh(3)).prepend(bleh(2)).prepend(bleh(1))

will behave in a properly lazy manner.

This is no great mystery, as the parser converts the definition of xs20 to:

 val xs20 = {   <synthetic> <artifact> val x$68 = bleh(1);
    {  <synthetic> <artifact> val x$67 = bleh(2);
  {  <synthetic> <artifact> val x$66 = bleh(3);
  LazyList.empty.$hash$colon$colon(x$66)}.$hash$colon$colon(x$67)}.$hash$colon$colon(x$68)  };

This seems to be scala/bug#7333, which was closed as a duplicate of the somewhat less general scala/bug#1980

SethTisue commented 7 years ago

what do you have in mind in opening this ticket, given the existence of scala/bug#1980? (I corrected your links to point to GitHub instead of JIRA)

pnf commented 7 years ago

Thank you for the correction. Given that scala/bug#1980 has been around since 2009 and has now been relegated to the backlog., perhaps removing the #:: operator from LazyList would be a good idea? I was initially trying to add an assertion to verify laziness in lazyListTestOps, but that didn't work out so well.

smarter commented 7 years ago

FWIW, this should be much easier to fix in dotty where the problematic desugaring is not done during parsing but during typechecking, where we should have enough information to make the intermediate vals lazy. I opened https://github.com/lampepfl/dotty/issues/2808 to track this.

pnf commented 7 years ago

Cool. I was concocting a scheme to restore the laziness post-typer in a plugin, but it would clearly be cleaner to have the desugaring entirely post typer.

In the mean time, having a LazyList with an eager cons seems at the very least misleading, so maybe this issue has a collection-specific value independent of the existing scalac issues.

On Jun 24, 2017, at 8:07 PM, Guillaume Martres notifications@github.com wrote:

FWIW, this should be much easier to fix in dotty where the problematic desugaring is not done during parser but during typechecking, where we should have enough information to make the intermediate vals lazy. I opened lampepfl/dotty#2808 to track this.

― You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

szeiger commented 7 years ago

There is a hack to make right-associative operations lazy and the current collections library already uses it. Try the same code with Stream instead of LazyList and take a look at ConsWrapper. We should be able to do the same for LazyList.

pnf commented 7 years ago

Good point. I added something similar to #128. Among other benefits, the de rigueur Fibonacci example now works without blowing stack.

szeiger commented 7 years ago

It looks like we can fix it in Scala 2.13 after all: https://github.com/scala/scala/pull/5969. It requires a spec change so we should get Dotty on board, too.

julienrf commented 7 years ago

Putting on hold because we have to wait for Scala 2.13.0-M3.

optician commented 6 years ago

Tested with 2.13.0-M3. Works fine.

scala> def bleh(i: Int) = {println("bleh " + i); i}
bleh: (i: Int)Int

scala>   val xs20 = bleh(1) #:: bleh(2) #:: bleh(3) #:: LazyList.empty
xs20: strawman.collection.immutable.LazyList[Int] = LazyList(?)

scala> xs20.head
bleh 1
res0: Int = 1

Scala 2.13.0-M2 evaluates only first element.

julienrf commented 6 years ago

Yes, see #345.

julienrf commented 6 years ago

Closed by https://github.com/scala/scala/pull/6509