Fix #2266: Do not replace constant type lazy vals with constant. #2267
Conversation
| @@ -427,7 +427,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => | |||
| def constToLiteral(tree: Tree)(implicit ctx: Context): Tree = { | |||
| val tree1 = ConstFold(tree) | |||
| tree1.tpe.widenTermRefExpr match { | |||
| case ConstantType(value) if isIdempotentExpr(tree1) => Literal(value) | |||
| case ConstantType(value) if isIdempotentExpr(tree1) && !tree1.symbol.is(Lazy) => Literal(value) | |||
There was a problem hiding this comment.
I don't think this is specific to lazy vals.
abstract class A {
def s: Boolean = r
def r: Boolean
assert(Test.r == this.s)
}
object Test extends A {
override val r: true = true
def main(args: Array[String]): Unit = {}
}There was a problem hiding this comment.
// updated example to make it more likely to trigger
There was a problem hiding this comment.
What is the issue you would expect with that example?
There was a problem hiding this comment.
assertion will fail, while it shouldn't.
There was a problem hiding this comment.
Sorry, forgot to update the example, though I've left the comment that I did update it.
There was a problem hiding this comment.
Indeed, that one fails for a similar reason.
There was a problem hiding this comment.
I added a fix for that issue.
| // lazy value must be initialized (would not be needed with isPureExpr) | ||
| !tree1.symbol.is(Lazy) && | ||
| // could hide initialization order issues (ex. val with constant type read before initialized) | ||
| (!ctx.owner.isLocalDummy || (tree1.symbol.is(Method) || value.isZero) || |
There was a problem hiding this comment.
tree1.symbol.is(Method)
Why is inlining the return value of a (possibly-side-effectfull) method safe?
nicolasstucki
force-pushed
the
fix-constant-lazy-vals
branch
from
57942db to
983ce8d
Compare
|
LGTM |
No description provided.