Fix #5350: Fix purity of local lazy vals #5703
Conversation
| @@ -0,0 +1,10 @@ | |||
| object Test { | |||
| def foo = { | |||
| lazy val bar: Unit = println("Hello") | |||
There was a problem hiding this comment.
@Blaisorblade does it make sense for this lazy val to be stable?
There was a problem hiding this comment.
Yes. The definition of isStable doesn't check for Lazy, so Symbol.isStable should only imply idempotence.
To detect if a value is pure, Realizability checks sym.isStable && !sym.is(Lazy | Erased, butNot = Module). Maybe here you should check
sym.isStable && !sym.is(Lazy, butNot = Module)? But on modules, this overlaps with #2266 and #4765.
That all needs testcases and consideration?
There was a problem hiding this comment.
Actually modules do not reach this line. Hence the current condition does not affect the module init elimination.
nicolasstucki
force-pushed
the
fix-#5350
branch
from
620a81d
to
fab746c
Compare
nicolasstucki
force-pushed
the
fix-#5350
branch
from
fab746c
to
ec3b3e1
Compare
|
|
||
| resultValue.map(value => s"$dcl = $value") | ||
| if (d.symbol.is(Flags.Lazy)) Some(dcl) | ||
| else valueOf(d.symbol).map(value => s"$dcl = $value") |
There was a problem hiding this comment.
👍 on printing the flag to the left (in ReplPrinter). On this change, FWIW, Scala 2 also uses <lazy> here. Not sure anybody cares tho, so merging anyway.
scala> lazy val x = 1
x: Int = <lazy>
|
Tests confirm this doesn't affect the handling of modules, and that modules don't get to the changed code (because that code tests stability of functions, hence not of objects). Hence can be merged once tests pass. |
No description provided.