Merge pull request #8995 from retronym/topic/pure-expression-defer

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -1196,36 +1196,37 @@ abstract class RefChecks extends Transform {
       try {
         enterSyms(stats)
         var index = -1
-        stats flatMap { stat => index += 1; transformStat(stat, index) }
+        stats.mapConserve(stat => {
+          index += 1;
+          transformStat(stat, index)
+        }).filter(_ ne EmptyTree)
       }
       finally popLevel()
     }
 
-
-
-    def transformStat(tree: Tree, index: Int): List[Tree] = tree match {
+    def transformStat(tree: Tree, index: Int): Tree = tree match {
       case t if treeInfo.isSelfConstrCall(t) =>
         assert(index == 0, index)
-        try transform(tree) :: Nil
+        try transform(tree)
         finally if (currentLevel.maxindex > 0) {
           // An implementation restriction to avoid VerifyErrors and lazyvals mishaps; see scala/bug#4717
           debuglog("refsym = " + currentLevel.refsym)
           reporter.error(currentLevel.refpos, "forward reference not allowed from self constructor invocation")
         }
       case ValDef(_, _, _, _) =>
         val tree1 = transform(tree) // important to do before forward reference check
-        if (tree1.symbol.isLazy) tree1 :: Nil
+        if (tree1.symbol.isLazy) tree1
         else {
           val sym = tree.symbol
           if (sym.isLocalToBlock && index <= currentLevel.maxindex) {
             debuglog("refsym = " + currentLevel.refsym)
             reporter.error(currentLevel.refpos, "forward reference extends over definition of " + sym)
           }
-          tree1 :: Nil
+          tree1
         }
-      case Import(_, _)                                                                       => Nil
-      case DefDef(mods, _, _, _, _, _) if (mods hasFlag MACRO) || (tree.symbol hasFlag MACRO) => Nil
-      case _                                                                                  => transform(tree) :: Nil
+      case Import(_, _)                                                                       => EmptyTree
+      case DefDef(mods, _, _, _, _, _) if (mods hasFlag MACRO) || (tree.symbol hasFlag MACRO) => EmptyTree
+      case _                                                                                  => transform(tree)
     }
 
     /* Check whether argument types conform to bounds of type parameters */
@@ -1763,6 +1764,14 @@ abstract class RefChecks extends Transform {
 
           case Template(parents, self, body) =>
             localTyper = localTyper.atOwner(tree, currentOwner)
+            for (stat <- body) {
+              if (treeInfo.isPureExprForWarningPurposes(stat)) {
+                val msg = "a pure expression does nothing in statement position"
+                val clause = if (body.lengthCompare(1) > 0) "; multiline expressions may require enclosing parentheses" else ""
+                reporter.warning(stat.pos, s"${msg}${clause}")
+              }
+            }
+
             validateBaseTypes(currentOwner)
             checkOverloadedRestrictions(currentOwner, currentOwner)
             // scala/bug#7870 default getters for constructors live in the companion module
@@ -1848,7 +1857,27 @@ abstract class RefChecks extends Transform {
                            // probably not, until we allow parameterised extractors
             tree
 
-
+          case Block(stats, expr) =>
+            val (count, result0, adapted) =
+              expr match {
+                case Block(expr :: Nil, Literal(Constant(()))) => (1, expr, true)
+                case Literal(Constant(()))                     => (0, EmptyTree, false)
+                case _                                         => (1, EmptyTree, false)
+              }
+            def checkPure(t: Tree, supple: Boolean): Unit =
+              if (treeInfo.isPureExprForWarningPurposes(t)) {
+                val msg = "a pure expression does nothing in statement position"
+                val parens = if (stats.length + count > 1) "multiline expressions might require enclosing parentheses" else ""
+                val discard = if (adapted) "; a value can be silently discarded when Unit is expected" else ""
+                val text =
+                  if (supple) s"${parens}${discard}"
+                  else if (!parens.isEmpty) s"${msg}; ${parens}" else msg
+                reporter.warning(t.pos, text)
+              }
+            // sanity check block for unintended expr placement
+            stats.foreach(checkPure(_, supple = false))
+            if (result0.nonEmpty) checkPure(result0, supple = true)
+            tree
           case _ => tree
         }
 

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -2487,31 +2487,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             case _ =>
           }
         }
-        val statsTyped = typedStats(block.stats, context.owner, warnPure = false)
+        val statsTyped = typedStats(block.stats, context.owner)
         val expr1 = typed(block.expr, mode &~ (FUNmode | QUALmode), pt)
-
-        // sanity check block for unintended expr placement
-        if (!isPastTyper) {
-          val (count, result0, adapted) =
-            expr1 match {
-              case Block(expr :: Nil, Literal(Constant(()))) => (1, expr, true)
-              case Literal(Constant(()))                     => (0, EmptyTree, false)
-              case _                                         => (1, EmptyTree, false)
-            }
-          def checkPure(t: Tree, supple: Boolean): Unit =
-            if (treeInfo.isPureExprForWarningPurposes(t)) {
-              val msg = "a pure expression does nothing in statement position"
-              val parens = if (statsTyped.length + count > 1) "multiline expressions might require enclosing parentheses" else ""
-              val discard = if (adapted) "; a value can be silently discarded when Unit is expected" else ""
-              val text =
-                if (supple) s"${parens}${discard}"
-                else if (!parens.isEmpty) s"${msg}; ${parens}" else msg
-              context.warning(t.pos, text)
-            }
-          statsTyped.foreach(checkPure(_, supple = false))
-          if (result0.nonEmpty) checkPure(result0, supple = true)
-        }
-
         treeCopy.Block(block, statsTyped, expr1)
           .setType(if (treeInfo.isExprSafeToInline(block)) expr1.tpe else expr1.tpe.deconst)
       } finally {
@@ -3193,7 +3170,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       case _                  => log(s"unhandled import: $imp in $unit"); imp
     }
 
-    def typedStats(stats: List[Tree], exprOwner: Symbol, warnPure: Boolean = true): List[Tree] = {
+    def typedStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
       val inBlock = exprOwner == context.owner
       def includesTargetPos(tree: Tree) =
         tree.pos.isRange && context.unit.exists && (tree.pos includes context.unit.targetPos)
@@ -3224,11 +3201,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
                 ConstructorsOrderError(stat)
             }
           }
-          if (warnPure && !isPastTyper && treeInfo.isPureExprForWarningPurposes(result)) {
-            val msg = "a pure expression does nothing in statement position"
-            val clause = if (stats.lengthCompare(1) > 0) "; multiline expressions may require enclosing parentheses" else ""
-            context.warning(stat.pos, s"${msg}${clause}")
-          }
           result
       }
 
@@ -5373,7 +5345,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         val pid1 = typedQualifier(pdef.pid).asInstanceOf[RefTree]
         assert(sym.moduleClass ne NoSymbol, sym)
         val stats1 = newTyper(context.make(tree, sym.moduleClass, sym.info.decls))
-          .typedStats(pdef.stats, NoSymbol)
+                  .typedStats(pdef.stats, NoSymbol)
         treeCopy.PackageDef(tree, pid1, stats1) setType NoType
       }
 

test/files/neg/macro-invalidret.check

@@ -27,8 +27,5 @@ java.lang.NullPointerException
 Macros_Test_2.scala:15: error: macro implementation is missing
   foo4
   ^
-Macros_Test_2.scala:17: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
-  foo6
-  ^
-two warnings found
+one warning found
 5 errors found

test/files/neg/names-defaults-neg-213.check

@@ -1,16 +1,9 @@
-names-defaults-neg-213.scala:7: warning: a pure expression does nothing in statement position
-    f1(x = 1) // named arg in 2.13 (value discard), not ambiguous
-           ^
 names-defaults-neg-213.scala:8: error: unknown parameter name: x
 Note that assignments in argument position are no longer allowed since Scala 2.13.
 To express the assignment expression, wrap it in brackets, e.g., `{ x = ... }`.
     f2(x = 1) // error, no parameter named x. error message mentions change in 2.13
          ^
-names-defaults-neg-213.scala:13: warning: a pure expression does nothing in statement position
-    f1(x = 1) // ok, named arg (value discard)
-           ^
 names-defaults-neg-213.scala:14: error: unknown parameter name: x
     f2(x = 1) // error (no such parameter). no mention of new semantics in 2.13
          ^
-two warnings found
 two errors found

test/files/neg/names-defaults-neg-warn.check

@@ -13,11 +13,8 @@ names-defaults-neg-warn.scala:23: warning: assignments in argument position are
 names-defaults-neg-warn.scala:34: warning: assignments in argument position are deprecated in favor of named arguments. Wrap the assignment in brackets, e.g., `{ x = ... }`.
     synchronized(x = 1) // deprecation warning in 2.12, error in 2.13
                    ^
-names-defaults-neg-warn.scala:42: warning: a pure expression does nothing in statement position
-    f1(x = 1) // 2.12, 2.13: ok, named arg (value discard)
-           ^
 names-defaults-neg-warn.scala:43: error: reassignment to val
     f2(x = 1) // 2.12, 2.13: error (no such parameter). no deprecation warning in 2.12, x is not a variable.
          ^
-5 warnings found
+four warnings found
 two errors found

test/files/neg/scopes.check

@@ -7,9 +7,6 @@ scopes.scala:5: error: x is already defined as value x
 scopes.scala:8: error: y is already defined as value y
     val y: Float = .0f
         ^
-scopes.scala:6: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
-  {
-  ^
 scopes.scala:11: error: x is already defined as value x
   def f1(x: Int, x: Float) = x
                  ^
@@ -22,5 +19,4 @@ scopes.scala:13: error: x is already defined as value x
 scopes.scala:15: error: x is already defined as value x
     case x::x => x
             ^
-one warning found
 7 errors found

test/files/neg/t1181.check

@@ -3,8 +3,4 @@ t1181.scala:9: error: type mismatch;
  required: Symbol
      _ => buildMap(map.updated(keyList.head, valueList.head), keyList.tail, valueList.tail)
                   ^
-t1181.scala:8: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
-     case (Nil, Nil) => map
-                        ^
-one warning found
 one error found

test/files/neg/t9847.check

@@ -1,36 +1,36 @@
 t9847.scala:4: warning: discarded non-Unit value
   def f(): Unit = 42
                   ^
-t9847.scala:4: warning: a pure expression does nothing in statement position
-  def f(): Unit = 42
-                  ^
 t9847.scala:5: warning: discarded non-Unit value
   def g = (42: Unit)
            ^
+t9847.scala:12: warning: discarded non-Unit value
+  + 1
+  ^
+t9847.scala:16: warning: discarded non-Unit value
+    x + 1
+      ^
+t9847.scala:19: warning: discarded non-Unit value
+  def j(): Unit = x + 1
+                    ^
+t9847.scala:4: warning: a pure expression does nothing in statement position
+  def f(): Unit = 42
+                  ^
 t9847.scala:5: warning: a pure expression does nothing in statement position
   def g = (42: Unit)
            ^
 t9847.scala:7: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
     1
     ^
-t9847.scala:12: warning: discarded non-Unit value
-  + 1
-  ^
-t9847.scala:12: warning: a pure expression does nothing in statement position
-  + 1
-  ^
 t9847.scala:11: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
     1
     ^
 t9847.scala:12: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
   + 1
   ^
-t9847.scala:16: warning: discarded non-Unit value
-    x + 1
-      ^
-t9847.scala:19: warning: discarded non-Unit value
-  def j(): Unit = x + 1
-                    ^
+t9847.scala:12: warning: a pure expression does nothing in statement position
+  + 1
+  ^
 t9847.scala:21: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
   class C { 42 }
             ^

test/files/neg/unit-returns-value.check

@@ -1,9 +1,9 @@
-unit-returns-value.scala:4: warning: a pure expression does nothing in statement position
-    if (b) return 5
-                  ^
 unit-returns-value.scala:4: warning: enclosing method f has result type Unit: return value discarded
     if (b) return 5
            ^
+unit-returns-value.scala:4: warning: a pure expression does nothing in statement position
+    if (b) return 5
+                  ^
 unit-returns-value.scala:22: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
     i1 // warn
     ^

test/files/run/contrib674.check

@@ -1,6 +1,6 @@
-contrib674.scala:15: warning: a pure expression does nothing in statement position
+contrib674.scala:15: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
     1
     ^
-contrib674.scala:15: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
+contrib674.scala:15: warning: a pure expression does nothing in statement position
     1
     ^

test/files/run/names-defaults.check

@@ -1,7 +1,7 @@
-names-defaults.scala:269: warning: a pure expression does nothing in statement position
+names-defaults.scala:269: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
     spawn(b = { val ttt = 1; ttt }, a = 0)
                              ^
-names-defaults.scala:269: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
+names-defaults.scala:269: warning: a pure expression does nothing in statement position
     spawn(b = { val ttt = 1; ttt }, a = 0)
                              ^
 warning: there were four deprecation warnings

test/files/run/patmatnew.check

@@ -1,21 +1,21 @@
+patmatnew.scala:670: warning: This catches all Throwables. If this is really intended, use `case e : Throwable` to clear this warning.
+            case e => {
+                 ^
+patmatnew.scala:489: warning: unreachable code
+        case _ if false =>
+                        ^
 patmatnew.scala:351: warning: a pure expression does nothing in statement position
         case 1 => "OK"
                   ^
-patmatnew.scala:352: warning: a pure expression does nothing in statement position
+patmatnew.scala:352: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
         case 2 => assert(false); "KO"
                                  ^
-patmatnew.scala:352: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
+patmatnew.scala:352: warning: a pure expression does nothing in statement position
         case 2 => assert(false); "KO"
                                  ^
-patmatnew.scala:353: warning: a pure expression does nothing in statement position
+patmatnew.scala:353: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
         case 3 => assert(false); "KO"
                                  ^
-patmatnew.scala:353: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
+patmatnew.scala:353: warning: a pure expression does nothing in statement position
         case 3 => assert(false); "KO"
                                  ^
-patmatnew.scala:670: warning: This catches all Throwables. If this is really intended, use `case e : Throwable` to clear this warning.
-            case e => {
-                 ^
-patmatnew.scala:489: warning: unreachable code
-        case _ if false =>
-                        ^

test/files/run/pure-warning-post-macro/Macro_1.scala

@@ -0,0 +1,13 @@
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+object Macro {
+  def blockToList[T](block: T): List[T] = macro impl[T]
+  def impl[T: c.WeakTypeTag](c: Context)(block: c.Tree): c.Tree = {
+    import c.universe._
+    block match {
+      case Block(stats, expr) =>
+        q"_root_.scala.List.apply[${weakTypeOf[T]}](..${stats :+ expr})"
+    }
+  }
+}

test/files/run/pure-warning-post-macro/test_2.scala

@@ -0,0 +1,13 @@
+// scalac: -Xfatal-errors
+object Test {
+  def main(args: Array[String]): Unit = {
+    // We don't want a "pure expression discarded" warning here as the macro will
+    // eliminate the block
+    val is = Macro.blockToList[Int] {
+      1
+      2
+      3
+    }
+    assert(is == List(1, 2, 3))
+  }
+}

test/files/run/reify_lazyunit.check

@@ -1,6 +1,3 @@
-reify_lazyunit.scala:6: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
-    lazy val x = { 0; println("12")}
-                   ^
 12
 one
 two

test/files/run/t3488.check

@@ -1,13 +1,13 @@
-t3488.scala:4: warning: a pure expression does nothing in statement position
+t3488.scala:4: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
   println(foo { val List(_*)=List(0); 1 } ())
                                       ^
-t3488.scala:4: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
+t3488.scala:4: warning: a pure expression does nothing in statement position
   println(foo { val List(_*)=List(0); 1 } ())
                                       ^
-t3488.scala:5: warning: a pure expression does nothing in statement position
+t3488.scala:5: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
   println(foo { val List(_*)=List(0); 1 } (1))
                                       ^
-t3488.scala:5: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
+t3488.scala:5: warning: a pure expression does nothing in statement position
   println(foo { val List(_*)=List(0); 1 } (1))
                                       ^
 0