Merge pull request #2267 from dotty-staging/fix-constant-lazy-vals

Fix #2266: Do not replace constant type lazy vals with constant.

Author: DarkDimius

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

@@ -3,7 +3,7 @@ package dotc
 package ast
 
 import core._
-import Flags._, Trees._, Types._, Contexts._
+import Flags._, Trees._, Types._, Contexts._, Constants._
 import Names._, StdNames._, NameOps._, Decorators._, Symbols._
 import util.HashSet
 import typer.ConstFold
@@ -426,8 +426,18 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
    */
   def constToLiteral(tree: Tree)(implicit ctx: Context): Tree = {
     val tree1 = ConstFold(tree)
+    def canInlineConstant(value: Constant): Boolean = {
+      val sym = tree1.symbol
+      isIdempotentExpr(tree1) && // see note in documentation
+      // lazy value must be initialized (would not be needed with isPureExpr)
+      !sym.is(Lazy) &&
+      // could hide initialization order issues (ex. val with constant type read before initialized)
+      (!ctx.owner.isLocalDummy || (!sym.is(Method) && !sym.is(Lazy) && value.isZero) ||
+        ctx.scala2Mode // ignore in Scala 2 because of inlined `final val` values
+      )
+    }
     tree1.tpe.widenTermRefExpr match {
-      case ConstantType(value) if isIdempotentExpr(tree1) => Literal(value)
+      case ConstantType(value) if canInlineConstant(value) => Literal(value)
       case _ => tree1
     }
   }

compiler/src/dotty/tools/dotc/core/Constants.scala

@@ -53,6 +53,20 @@ object Constants {
     def isNonUnitAnyVal       = BooleanTag <= tag && tag <= DoubleTag
     def isAnyVal              = UnitTag <= tag && tag <= DoubleTag
 
+    /** Is the zero or un-initialized value of the type */
+    def isZero(implicit ctx: Context): Boolean = tag match {
+      case BooleanTag => !value.asInstanceOf[Boolean]
+      case ByteTag    => value.asInstanceOf[Byte] == 0
+      case ShortTag   => value.asInstanceOf[Short] == 0
+      case CharTag    => value.asInstanceOf[Char] == 0
+      case IntTag     => value.asInstanceOf[Int] == 0
+      case LongTag    => value.asInstanceOf[Long] == 0L
+      case FloatTag   => value.asInstanceOf[Float] == 0.0
+      case DoubleTag  => value.asInstanceOf[Double] == 0.0
+      case NullTag    => true
+      case _          => false
+    }
+
     def tpe(implicit ctx: Context): Type = tag match {
       case UnitTag    => defn.UnitType
       case BooleanTag => defn.BooleanType

compiler/src/dotty/tools/dotc/transform/FirstTransform.scala

@@ -9,7 +9,7 @@ import dotty.tools.dotc.transform.TreeTransforms._
 import ast.Trees._
 import Flags._
 import Types._
-import Constants.Constant
+import Constants._
 import Contexts.Context
 import Symbols._
 import SymDenotations._
@@ -34,6 +34,8 @@ import StdNames._
  *   - drops branches of ifs using the rules
  *          if (true) A else B  --> A
  *          if (false) A else B --> B
+ *          if (C: true) A else B  --> C; A
+ *          if (C: false) A else B  --> C; B
  */
 class FirstTransform extends MiniPhaseTransform with InfoTransformer with AnnotationTransformer { thisTransformer =>
   import ast.tpd._
@@ -190,11 +192,16 @@ class FirstTransform extends MiniPhaseTransform with InfoTransformer with Annota
   override def transformBlock(tree: Block)(implicit ctx: Context, info: TransformerInfo) =
     constToLiteral(tree)
 
-  override def transformIf(tree: If)(implicit ctx: Context, info: TransformerInfo) =
-    tree.cond match {
-      case Literal(Constant(c: Boolean)) => if (c) tree.thenp else tree.elsep
-      case _ => tree
+  override def transformIf(tree: If)(implicit ctx: Context, info: TransformerInfo) = {
+    tree.cond.tpe.widenTermRefExpr match {
+      case ConstantType(Constant(condVal: Boolean)) =>
+        val selected = if (condVal) tree.thenp else tree.elsep
+        if (isPureExpr(tree.cond)) selected
+        else Block(tree.cond :: Nil, selected)
+      case _ =>
+        tree
     }
+  }
 
   // invariants: all modules have companion objects
   // all types are TypeTrees

compiler/test/dotc/scala-collections.blacklist

@@ -81,3 +81,5 @@ scala/util/control/Exception.scala
 # 51 |  implicit def throwableSubtypeToCatcher[Ex <: Throwable: ClassTag, T](pf: PartialFunction[Ex, T]) =
 #    |                                                                                                 ^
 #    |                                                                     cyclic reference involving method mkCatcher
+
+scala/concurrent/duration/Duration.scala

tests/run/if-with-constant-cond.check

@@ -0,0 +1,4 @@
+cond1
+then1
+cond2
+else2

tests/run/if-with-constant-cond.scala

@@ -0,0 +1,9 @@
+
+object Test {
+
+  def main(args: Array[String]): Unit = {
+    if ({ println("cond1"); true }: true) println("then1") else println("else1")
+    if ({ println("cond2"); false }: false) println("then2") else println("else2")
+  }
+
+}

tests/run/inline-constant-in-constructor-1.check

@@ -0,0 +1,3 @@
+assert
+s
+r init

tests/run/inline-constant-in-constructor-1.scala

@@ -0,0 +1,15 @@
+
+
+abstract class A {
+  def s: Boolean = { println("s"); r }
+  def r: Boolean
+}
+
+object Test extends A {
+  assert({ println("assert"); r == s }) // r constant type not replaced by true, r not initialized yet
+  override val r: true = {
+    println("r init")
+    true
+  }
+  def main(args: Array[String]): Unit = {}
+}

tests/run/inline-constant-in-constructor-2.check

@@ -0,0 +1,3 @@
+assert
+s
+r init

tests/run/inline-constant-in-constructor-2.scala

@@ -0,0 +1,14 @@
+
+abstract class A {
+  def s: Boolean = { println("s"); r }
+  def r: Boolean
+}
+
+object Test extends A {
+  assert({ println("assert"); r == s }) // r constant type replaced by false
+  override val r: false = {
+    println("r init")
+    false
+  }
+  def main(args: Array[String]): Unit = {}
+}