From 46828e7683734ce8742de57d632d8f091794176a Mon Sep 17 00:00:00 2001
From: Olivier Blanvillain <olivier.blanvillain@gmail.com>
Date: Thu, 12 Jul 2018 16:03:48 +0200
Subject: [PATCH] Fix #4784: remove unsound constant folding

The optimizer was using isInstanceOf[ConstantType] as a purity check,
which is obviously wrong as illustrated by the added test case.
---
 .../dotc/transform/localopt/ConstantFold.scala | 18 ++----------------
 .../test/dotty/tools/dotc/SimplifyTests.scala  | 18 +++++++++++-------
 2 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala b/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala
index 2517c7318688..7d3416737bb9 100644
--- a/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala
+++ b/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala
@@ -75,7 +75,7 @@ import ast.Trees._
     //    isBool(ift.tpe) && !elsep.const.booleanValue =>
     //       cond.select(defn.Boolean_&&).appliedTo(elsep)
     //   the other case ins't handled intentionally. See previous case for explanation
-    
+
     case If(t @ Select(recv, _), thenp, elsep) if t.symbol eq defn.Boolean_! =>
       If(recv, elsep, thenp)
 
@@ -98,11 +98,6 @@ import ast.Trees._
         case (lhs, Literal(_)) if !lhs.isInstanceOf[Literal] && simplifyPhase.CommutativePrimitiveOperations.contains(sym) =>
           rhs.select(sym).appliedTo(lhs)
 
-        case (l, _) if (sym == defn.Boolean_&&) && isConst(l.tpe) =>
-          val const = asConst(l.tpe).value.booleanValue
-          if (const) Block(lhs :: Nil, rhs)
-          else l
-
         case (l, x: Literal) if sym == defn.Boolean_== && isBool(l.tpe) && isBool(x.tpe) =>
           if (x.const.booleanValue) l
           else l.select(defn.Boolean_!).ensureApplied
@@ -119,11 +114,6 @@ import ast.Trees._
           if (!x.const.booleanValue) l
           else l.select(defn.Boolean_!).ensureApplied
 
-        case (l: Literal, _) if (sym == defn.Boolean_||) && isConst(l.tpe)   =>
-          val const = asConst(l.tpe).value.booleanValue
-          if (l.const.booleanValue) l
-          else Block(lhs :: Nil, rhs)
-
         // case (Literal(Constant(1)), _)    if sym == defn.Int_*  => rhs
         // case (Literal(Constant(0)), _)    if sym == defn.Int_+  => rhs
         // case (Literal(Constant(1L)), _)   if sym == defn.Long_* => rhs
@@ -143,7 +133,7 @@ import ast.Trees._
 
         case (l: Literal, r: Literal)  =>
           (l.tpe.widenTermRefExpr, r.tpe.widenTermRefExpr) match {
-            case (ConstantType(_), ConstantType(_)) => 
+            case (ConstantType(_), ConstantType(_)) =>
               val s = ConstFold.apply(t)
               if ((s ne null) && s.tpe.isInstanceOf[ConstantType]) Literal(s.tpe.asInstanceOf[ConstantType].value)
               else t
@@ -169,8 +159,6 @@ import ast.Trees._
     case t => t
   }
 
-
-
   def isSimilar(t1: Tree, t2: Tree)(implicit ctx: Context): Boolean = t1 match {
     case t1: Apply =>
       t2 match {
@@ -207,6 +195,4 @@ import ast.Trees._
   }
 
   def isBool(tpe: Type)(implicit ctx: Context): Boolean       = tpe.derivesFrom(defn.BooleanClass)
-  def isConst(tpe: Type)(implicit ctx: Context): Boolean      = tpe.isInstanceOf[ConstantType]
-  def asConst(tpe: Type)(implicit ctx: Context): ConstantType = tpe.asInstanceOf[ConstantType]
 }
diff --git a/compiler/test/dotty/tools/dotc/SimplifyTests.scala b/compiler/test/dotty/tools/dotc/SimplifyTests.scala
index 933d6f6da5bd..745de0213e2c 100644
--- a/compiler/test/dotty/tools/dotc/SimplifyTests.scala
+++ b/compiler/test/dotty/tools/dotc/SimplifyTests.scala
@@ -7,16 +7,16 @@ import dotty.tools.dotc.config.CompilerCommand
 import dotty.tools.dotc.core.Contexts.FreshContext
 import scala.tools.asm.tree.MethodNode
 
-class SimplifyPosTests extends SimplifyTests(optimise = true)
-class SimplifyNegTests extends SimplifyTests(optimise = false)
+class SimplifyPosTests extends SimplifyTests(pos = true)
+class SimplifyNegTests extends SimplifyTests(pos = false)
 
-abstract class SimplifyTests(val optimise: Boolean) extends DottyBytecodeTest {
+abstract class SimplifyTests(val pos: Boolean) extends DottyBytecodeTest {
   override def initCtx = {
     val ctx0 = super.initCtx
-    ctx0.setSetting(ctx0.settings.optimise, optimise)
+    ctx0.setSetting(ctx0.settings.optimise, pos)
   }
 
-  def check(source: String, expected: String, shared: String = ""): Unit = {
+  def check(source: String, expected: String, shared: String = "", checkEqual: Boolean = pos): Unit = {
     import ASMConverters._
     val src =
       s"""
@@ -41,7 +41,7 @@ abstract class SimplifyTests(val optimise: Boolean) extends DottyBytecodeTest {
       val A = instructions("A")
       val B = instructions("B")
       val diff = diffInstructions(A, B)
-      if (optimise)
+      if (checkEqual)
         assert(A == B, s"Bytecode doesn't match: (lhs = source, rhs = expected) \n$diff")
       else
         assert(A != B, s"Same Bytecodes without -optimise: you are testing the wrong thing!")
@@ -141,7 +141,7 @@ abstract class SimplifyTests(val optimise: Boolean) extends DottyBytecodeTest {
       """
         |val i = 3
         |val j = i + 4
-        |if(j - i >= (i + 1) / 2) 
+        |if(j - i >= (i + 1) / 2)
         |  print(i + 1)
       """,
       """
@@ -188,6 +188,10 @@ abstract class SimplifyTests(val optimise: Boolean) extends DottyBytecodeTest {
         |}
       """)
 
+  @Test def test4784 =
+    check(
+      """{ println(1); false } || true""",
+      """true""", checkEqual = false)
 
   // @Test def listPatmapExample =
   //   check(