Offer warning when demonstrably non-side-effect...

Offer warning when demonstrably non-side-effecting expressions appear in
statement position, which should be unintentional by definition. Threw
in removal of six places with useless discarded expressions which the
warning informed me about. No review.

6 files changed, 27 insertions, 7 deletions

diff --git a/src/compiler/scala/reflect/internal/Constants.scala b/src/compiler/scala/reflect/internal/Constants.scala
index 55bb7c2ceb..2edb0d1fe6 100644
--- a/src/compiler/scala/reflect/internal/Constants.scala
+++ b/src/compiler/scala/reflect/internal/Constants.scala
@@ -55,6 +55,8 @@ trait Constants extends api.Constants {
     def isLongRange: Boolean  = ByteTag <= tag && tag <= LongTag
     def isFloatRange: Boolean = ByteTag <= tag && tag <= FloatTag
     def isNumeric: Boolean    = ByteTag <= tag && tag <= DoubleTag
+    def isNonUnitAnyVal       = BooleanTag <= tag && tag <= DoubleTag
+    def isAnyVal              = UnitTag <= tag && tag <= DoubleTag
 
     def tpe: Type = tag match {
       case UnitTag    => UnitClass.tpe
diff --git a/src/compiler/scala/reflect/internal/TreeInfo.scala b/src/compiler/scala/reflect/internal/TreeInfo.scala
index a8ec7e34f5..de46069ee2 100644
--- a/src/compiler/scala/reflect/internal/TreeInfo.scala
+++ b/src/compiler/scala/reflect/internal/TreeInfo.scala
@@ -68,6 +68,10 @@ abstract class TreeInfo {
   }
 
   /** Is tree a stable and pure expression?
+   *  !!! Clarification on what is meant by "pure" here would be appreciated.
+   *  This implementation allows both modules and lazy vals, which are pure in
+   *  the sense that they always return the same result, but which are also
+   *  side effecting.  So for now, "pure" != "not side effecting".
    */
   def isPureExpr(tree: Tree): Boolean = tree match {
     case EmptyTree
@@ -77,6 +81,10 @@ abstract class TreeInfo {
       true
     case Ident(_) =>
       tree.symbol.isStable
+    // this case is mostly to allow expressions like -5 and +7, but any
+    // member of an anyval should be safely pure
+    case Select(Literal(const), name) =>
+      const.isAnyVal && (const.tpe.member(name) != NoSymbol)
     case Select(qual, _) =>
       tree.symbol.isStable && isPureExpr(qual)
     case TypeApply(fn, _) =>
diff --git a/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala b/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
index 6c1245dd46..9eda2e4097 100644
--- a/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
@@ -397,11 +397,9 @@ trait MarkupParsers {
     def xScalaPatterns: List[Tree] = escapeToScala(parser.seqPatterns(), "pattern")
 
     def reportSyntaxError(pos: Int, str: String) = parser.syntaxError(pos, str)
-    def reportSyntaxError(str: String) = {
+    def reportSyntaxError(str: String) {
       reportSyntaxError(curOffset, "in XML literal: " + str)
-      val result = ch
-      nextch
-      result
+      nextch()
     }
 
     /** '<' xPattern  ::= Name [S] { xmlPattern | '{' pattern3 '}' } ETag
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index 7d68abccf0..9adb7dce20 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -928,7 +928,7 @@ trait Namers { self: Analyzer =>
         // is more compact than: def foo[T, T2](a: T, x: T2)(implicit w: ComputeT2[T, T2])
         // moreover, the latter is not an encoding of the former, which hides type inference of T2, so you can specify T while T2 is purely computed
         val checkDependencies: TypeTraverser = new TypeTraverser {
-          def traverse(tp: Type) = {
+          def traverse(tp: Type) {
             tp match {
               case SingleType(_, sym) =>
                 if (sym.owner == meth && sym.isValueParameter && !(okParams contains sym))
@@ -941,7 +941,6 @@ trait Namers { self: Analyzer =>
               case _ =>
                 mapOver(tp)
             }
-            this
           }
         }
         for(vps <- vparamSymss) {
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 725bf90af3..3b6b44316b 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -2165,6 +2165,20 @@ trait Typers extends Modes with Adaptations {
                   if (treeInfo.isSelfConstrCall(result) && result.symbol.pos.pointOrElse(0) >= exprOwner.enclMethod.pos.pointOrElse(0))
                     error(stat.pos, "called constructor's definition must precede calling constructor's definition")
                 }
+                result match {
+                  case EmptyTree | Literal(Constant(()))  => ()
+                  case _ =>
+                    if (treeInfo isPureExpr result) {
+                      val sym = result.symbol
+                      if (sym != null && (sym.isModule || sym.isLazy)) {
+                        debuglog("'Pure' but side-effecting expression in statement position: " + result)
+                      }
+                      else context.warning(stat.pos,
+                        "a pure expression does nothing in statement position; " +
+                        "you may be omitting necessary parentheses"
+                      )
+                    }
+                }
                 result
               }
           }
diff --git a/src/compiler/scala/tools/nsc/util/Statistics.scala b/src/compiler/scala/tools/nsc/util/Statistics.scala
index 95cdc77a09..27239b9b9f 100644
--- a/src/compiler/scala/tools/nsc/util/Statistics.scala
+++ b/src/compiler/scala/tools/nsc/util/Statistics.scala
@@ -71,7 +71,6 @@ abstract class StatisticsInfo {
 
   def countNodes(tree: Tree, counts: ClassCounts) {
     for (t <- tree) counts(t.getClass) += 1
-    counts
   }
 
   def showRelative(base: Long)(value: Long) =