From 6b5adcca633dff71a4fe666b736be21298e7ede4 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 9 Jun 2022 12:24:59 -0700 Subject: [PATCH] Improve explicit Unit check --- .../scala/tools/nsc/typechecker/RefChecks.scala | 11 ++--------- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 2 +- src/reflect/scala/reflect/internal/TreeInfo.scala | 10 ++++++++++ test/files/neg/nonunit-if.check | 5 +---- test/files/neg/nonunit-statement.check | 5 +---- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 034d04119ad6..ccd5710deeb9 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1780,7 +1780,7 @@ abstract class RefChecks extends Transform { && !isUninterestingType(t.tpe) // bottom types, Unit, Any && !treeInfo.isThisTypeResult(t) // buf += x && !treeInfo.isSuperConstrCall(t) // just a thing - && !explicitlyUnit(t) // suppressed by explicit expr: Unit + && !treeInfo.hasExplicitUnit(t) // suppressed by explicit expr: Unit && !isJavaApplication(t) // Java methods are inherently side-effecting ) // begin checkInterestingResultInStatement @@ -1912,13 +1912,6 @@ abstract class RefChecks extends Transform { tree case blk @ Block(stats, expr) => - // if block is value discard, copy any TypedExpectingUnitAttachment to outermost Apply in case there were implicit args - blk match { - case Block((inner @ treeInfo.Applied(_, _, _)) :: Nil, Literal(Constant(()))) => - if (inner.exists(explicitlyUnit)) - inner.updateAttachment(TypedExpectingUnitAttachment) - case _ => - } // diagnostic info val (count, result0, adapted) = expr match { @@ -1929,7 +1922,7 @@ abstract class RefChecks extends Transform { val isMultiline = stats.lengthCompare(1 - count) > 0 def checkPure(t: Tree, supple: Boolean): Unit = - if (!explicitlyUnit(t) && treeInfo.isPureExprForWarningPurposes(t)) { + if (!treeInfo.hasExplicitUnit(t) && treeInfo.isPureExprForWarningPurposes(t)) { val msg = "a pure expression does nothing in statement position" val parens = if (isMultiline) "multiline expressions might require enclosing parentheses" else "" val discard = if (adapted) "; a value can be silently discarded when Unit is expected" else "" diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 7eb7d2b11b08..70b131af3628 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1112,7 +1112,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper @inline def tpd(transformed: Tree) = typed(transformed, mode, pt) @inline def warnValueDiscard(): Unit = - if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !explicitlyUnit(tree)) + if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree)) context.warning(tree.pos, "discarded non-Unit value", WarningCategory.WFlagValueDiscard) @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) { diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 660b935ed26c..50f75029f323 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -779,6 +779,16 @@ abstract class TreeInfo { ((sym ne null) && sym.initialize.isTrait) } + def hasExplicitUnit(tree: Tree): Boolean = + explicitlyUnit(tree) || { + tree match { + case Apply(f, _) => hasExplicitUnit(f) + case TypeApply(f, _) => hasExplicitUnit(f) + case AppliedTypeTree(f, _) => hasExplicitUnit(f) + case _ => false + } + } + /** Applications in Scala can have one of the following shapes: * * 1) naked core: Ident(_) or Select(_, _) or basically anything else diff --git a/test/files/neg/nonunit-if.check b/test/files/neg/nonunit-if.check index 25bf5bba42c9..276b16a5902f 100644 --- a/test/files/neg/nonunit-if.check +++ b/test/files/neg/nonunit-if.check @@ -1,6 +1,3 @@ -nonunit-if.scala:53: warning: discarded non-Unit value - Future(42): Unit // nowarn { F(42)(ctx) }: Unit where annot is on F(42), TODO spurious -Wvalue-discard - ^ nonunit-if.scala:60: warning: discarded non-Unit value if (!isEmpty) f(a) // warn ^ @@ -56,5 +53,5 @@ nonunit-if.scala:149: warning: unused value while (it.hasNext) it.next() // warn ^ error: No warnings can be incurred under -Werror. -19 warnings +18 warnings 1 error diff --git a/test/files/neg/nonunit-statement.check b/test/files/neg/nonunit-statement.check index 8ea037ee67b1..b59aca6cbdb5 100644 --- a/test/files/neg/nonunit-statement.check +++ b/test/files/neg/nonunit-statement.check @@ -1,6 +1,3 @@ -nonunit-statement.scala:54: warning: discarded non-Unit value - Future(42): Unit // nowarn { F(42)(ctx) }: Unit where annot is on F(42), TODO spurious -Wvalue-discard - ^ nonunit-statement.scala:14: warning: unused value improved // warn ^ @@ -38,5 +35,5 @@ nonunit-statement.scala:150: warning: unused value while (it.hasNext) it.next() // warn ^ error: No warnings can be incurred under -Werror. -13 warnings +12 warnings 1 error