Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SI-9847 Nuance pure expr statement warning #5266

Merged
merged 1 commit into from Aug 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 33 additions & 8 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -2413,13 +2413,36 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
}
}
val stats1 = if (isPastTyper) block.stats else
block.stats.flatMap(stat => stat match {
block.stats.flatMap {
case vd@ValDef(_, _, _, _) if vd.symbol.isLazy =>
namer.addDerivedTrees(Typer.this, vd)
case _ => stat::Nil
})
val stats2 = typedStats(stats1, context.owner)
case stat => stat::Nil
}
val stats2 = typedStats(stats1, context.owner, warnPure = false)
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 (stats2.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)
}
stats2.foreach(checkPure(_, supple = false))
if (result0.nonEmpty) checkPure(result0, supple = true)
}

treeCopy.Block(block, stats2, expr1)
.setType(if (treeInfo.isExprSafeToInline(block)) expr1.tpe else expr1.tpe.deconst)
} finally {
Expand Down Expand Up @@ -2994,7 +3017,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
case _ => log("unhandled import: "+imp+" in "+unit); imp
}

def typedStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
def typedStats(stats: List[Tree], exprOwner: Symbol, warnPure: Boolean = true): List[Tree] = {
val inBlock = exprOwner == context.owner
def includesTargetPos(tree: Tree) =
tree.pos.isRange && context.unit.exists && (tree.pos includes context.unit.targetPos)
Expand Down Expand Up @@ -3025,9 +3048,11 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
ConstructorsOrderError(stat)
}
}
if (!isPastTyper && treeInfo.isPureExprForWarningPurposes(result)) context.warning(stat.pos,
"a pure expression does nothing in statement position; you may be omitting necessary parentheses"
)
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
}

Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/macro-invalidret.check
Expand Up @@ -27,7 +27,7 @@ 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; you may be omitting necessary parentheses
Macros_Test_2.scala:17: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
foo6
^
two warnings found
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/scopes.check
Expand Up @@ -7,7 +7,7 @@ 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; you may be omitting necessary parentheses
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
Expand Down
4 changes: 2 additions & 2 deletions test/files/neg/stmt-expr-discard.check
@@ -1,7 +1,7 @@
stmt-expr-discard.scala:3: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
stmt-expr-discard.scala:3: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
+ 2
^
stmt-expr-discard.scala:4: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
stmt-expr-discard.scala:4: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
- 4
^
error: No warnings can be incurred under -Xfatal-warnings.
Expand Down
6 changes: 3 additions & 3 deletions test/files/neg/t1181.check
@@ -1,10 +1,10 @@
t1181.scala:8: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
case (Nil, Nil) => map
^
t1181.scala:9: error: type mismatch;
found : scala.collection.immutable.Map[Symbol,Symbol]
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
45 changes: 45 additions & 0 deletions test/files/neg/t9847.check
@@ -0,0 +1,45 @@
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
^
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there some logic in place to only report a single warning per position?

I wonder why we get both warnings here. With M5 I don't get the discarded non-Unit value one:

Welcome to Scala 2.12.0-M5 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_92).
Type in expressions for evaluation. Or try :help.

scala> def f(): Unit = 42
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
       def f(): Unit = 42
                       ^
f: ()Unit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractReporter suppresses multiple ERROR or lower-priority messages (warnings in presence of errors).

The value discard is a flag:

$ scala -Ywarn-value-discard
Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_92).
Type in expressions for evaluation. Or try :help.

scala> def f: Unit = 42
<console>:11: warning: discarded non-Unit value
       def f: Unit = 42
                     ^
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
       def f: Unit = 42
                     ^
f: Unit

scala> :quit
$ scalam -Ywarn-value-discard
Welcome to Scala 2.12.0-M5 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_92).
Type in expressions for evaluation. Or try :help.

scala> def f: Unit = 42
<console>:11: warning: discarded non-Unit value
       def f: Unit = 42
                     ^
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
       def f: Unit = 42
                     ^
f: Unit

scala> :quit
$ skala -Ywarn-value-discard
Welcome to Scala 2.12.0-20160708-085210-f805cf5 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_92).
Type in expressions for evaluation. Or try :help.

scala> def f: Unit = 42
<console>:11: warning: discarded non-Unit value
       def f: Unit = 42
                     ^
<console>:11: warning: a pure expression does nothing in statement position
       def f: Unit = 42
                     ^
f: Unit

scala> :quit

t9847.scala:5: warning: discarded non-Unit value
def g = (42: Unit)
^
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:21: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
class C { 42 }
^
t9847.scala:22: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
class D { 42 ; 17 }
^
t9847.scala:22: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
class D { 42 ; 17 }
^
error: No warnings can be incurred under -Xfatal-warnings.
14 warnings found
one error found
1 change: 1 addition & 0 deletions test/files/neg/t9847.flags
@@ -0,0 +1 @@
-Xfatal-warnings -Ywarn-value-discard
23 changes: 23 additions & 0 deletions test/files/neg/t9847.scala
@@ -0,0 +1,23 @@

trait T {

def f(): Unit = 42
def g = (42: Unit)
def h = {
1
+ 1
}
def hh(): Unit = {
1
+ 1
}
def i(): Unit = {
val x = 1
x + 1
}
def x = 42
def j(): Unit = x + 1

class C { 42 }
class D { 42 ; 17 }
}
6 changes: 3 additions & 3 deletions test/files/neg/unit-returns-value.check
@@ -1,13 +1,13 @@
unit-returns-value.scala:4: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
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:22: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
unit-returns-value.scala:22: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
i1 // warn
^
unit-returns-value.scala:23: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
unit-returns-value.scala:23: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
i2 // warn
^
error: No warnings can be incurred under -Xfatal-warnings.
Expand Down
5 changes: 4 additions & 1 deletion test/files/run/contrib674.check
@@ -1,3 +1,6 @@
contrib674.scala:15: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
contrib674.scala:15: warning: a pure expression does nothing in statement position
1
^
contrib674.scala:15: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
1
^
2 changes: 1 addition & 1 deletion test/files/run/contrib674.scala
@@ -1,7 +1,7 @@
// causes VerifyError with scala-2.5.1

object Test extends App {
def bad() {
def bad(): Unit = {
try {
1
} catch {
Expand Down
4 changes: 2 additions & 2 deletions test/files/run/delay-bad.check
@@ -1,7 +1,7 @@
delay-bad.scala:53: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
delay-bad.scala:53: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
f(new C { 5 })
^
delay-bad.scala:73: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
delay-bad.scala:73: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
f(new { val x = 5 } with E() { 5 })
^
warning: there was one deprecation warning (since 2.11.0); re-run with -deprecation for details
Expand Down
4 changes: 2 additions & 2 deletions test/files/run/delay-good.check
@@ -1,7 +1,7 @@
delay-good.scala:53: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
delay-good.scala:53: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
f(new C { 5 })
^
delay-good.scala:73: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
delay-good.scala:73: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
f(new { val x = 5 } with E() { 5 })
^

Expand Down
2 changes: 1 addition & 1 deletion test/files/run/exceptions-2.check
@@ -1,4 +1,4 @@
exceptions-2.scala:267: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
exceptions-2.scala:267: warning: a pure expression does nothing in statement position
try { 1 } catch { case e: java.io.IOException => () }
^
nested1:
Expand Down
4 changes: 2 additions & 2 deletions test/files/run/lazy-locals.check
@@ -1,7 +1,7 @@
lazy-locals.scala:153: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
lazy-locals.scala:153: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
{
^
lazy-locals.scala:159: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
lazy-locals.scala:159: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
{
^
forced lazy val q
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/macro-duplicate.check
@@ -1,3 +1,3 @@
Test_2.scala:5: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
Test_2.scala:5: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
Macros.foo
^
16 changes: 8 additions & 8 deletions test/files/run/misc.check
@@ -1,25 +1,25 @@
misc.scala:46: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:46: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
42;
^
misc.scala:47: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:47: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
42l;
^
misc.scala:48: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:48: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
23.5f;
^
misc.scala:49: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:49: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
23.5;
^
misc.scala:50: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:50: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
"Hello";
^
misc.scala:51: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:51: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
32 + 45;
^
misc.scala:62: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:62: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
x;
^
misc.scala:74: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
misc.scala:74: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
1 < 2;
^
### Hello
Expand Down
5 changes: 4 additions & 1 deletion test/files/run/names-defaults.check
@@ -1,4 +1,7 @@
names-defaults.scala:269: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
names-defaults.scala:269: warning: a pure expression does nothing in statement position
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
spawn(b = { val ttt = 1; ttt }, a = 0)
^
warning: there were four deprecation warnings; re-run with -deprecation for details
Expand Down
12 changes: 9 additions & 3 deletions test/files/run/patmatnew.check
@@ -1,10 +1,16 @@
patmatnew.scala:351: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
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; you may be omitting necessary parentheses
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; you may be omitting necessary parentheses
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:353: warning: a pure expression does nothing in statement position
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
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.
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/reify_lazyunit.check
@@ -1,4 +1,4 @@
reify_lazyunit.scala:6: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
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
Expand Down
12 changes: 6 additions & 6 deletions test/files/run/repl-bare-expr.check
@@ -1,12 +1,12 @@

scala> 2 ; 3
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
<console>:11: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
2 ;;
^
res0: Int = 3

scala> { 2 ; 3 }
<console>:12: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
<console>:12: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
{ 2 ; 3 }
^
res1: Int = 3
Expand All @@ -15,16 +15,16 @@ scala> 5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Mooo
1 +
2 +
3 } ; bippy+88+11
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
<console>:11: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
^
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
<console>:11: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
^
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
<console>:11: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
^
<console>:11: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
<console>:11: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
^
defined object Cow
Expand Down