From 83423c95d6b5369022aff4057fbafc9302ab3468 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 21 Feb 2022 20:53:11 -0800 Subject: [PATCH 1/4] Add -Wnonunit-statement and -Wnonunit-if Warn about any interesting value in statement position, for some definition of interest. Generalizes the current warning for "pure" expressions. By default, warns about unibranch if, which lack an else in user code. Specify -Wnonunit-if:false to turn off warning about unibranch if. (That also turns off -Wvalue-discard for those cases.) Improve explicit Unit check Accommodate field when method is this.type Handle Match --- .../scala/tools/nsc/ast/parser/Parsers.scala | 18 +- .../scala/tools/nsc/settings/Warnings.scala | 4 + .../tools/nsc/typechecker/RefChecks.scala | 83 +++++++- .../nsc/typechecker/StdAttachments.scala | 4 - .../scala/tools/nsc/typechecker/Typers.scala | 18 +- .../reflect/internal/StdAttachments.scala | 4 + .../scala/reflect/internal/TreeInfo.scala | 44 ++++ .../reflect/runtime/JavaUniverseForce.scala | 1 + test/files/neg/nonunit-if.check | 57 ++++++ test/files/neg/nonunit-if.scala | 191 ++++++++++++++++++ test/files/neg/nonunit-statement.check | 39 ++++ test/files/neg/nonunit-statement.scala | 191 ++++++++++++++++++ test/files/neg/value-discard.check | 6 +- test/files/neg/value-discard.scala | 14 +- .../files/neg/virtpatmat_unreach_select.scala | 2 +- 15 files changed, 637 insertions(+), 39 deletions(-) create mode 100644 test/files/neg/nonunit-if.check create mode 100644 test/files/neg/nonunit-if.scala create mode 100644 test/files/neg/nonunit-statement.check create mode 100644 test/files/neg/nonunit-statement.scala diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index d32082e3fd31..f809182abdcb 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -1617,8 +1617,22 @@ self => val cond = condExpr() newLinesOpt() val thenp = expr() - val elsep = if (in.token == ELSE) { in.nextToken(); expr() } - else literalUnit + val elsep = + if (in.token == ELSE) { + in.nextToken() + expr() + } + else { + // user asked to silence warnings on unibranch if; also suppresses value discard + if (settings.warnNonUnitIf.isSetByUser && !settings.warnNonUnitIf.value) { + thenp match { + case Block(_, res) => res.updateAttachment(TypedExpectingUnitAttachment) + case _ => () + } + thenp.updateAttachment(TypedExpectingUnitAttachment) + } + literalUnit + } If(cond, thenp, elsep) } parseIf diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 5c38ba7dcb60..b5d12f83683a 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -113,6 +113,10 @@ trait Warnings { ) ) withAbbreviation "-Ywarn-macros" val warnDeadCode = BooleanSetting("-Wdead-code", "Warn when dead code is identified.") withAbbreviation "-Ywarn-dead-code" + val warnNonUnitIf = BooleanSetting("-Wnonunit-if", "Warn when if statements are non-Unit expressions, enabled by -Wnonunit-statement.") + import scala.language.existentials + val warnNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.") + .enablingIfNotSetByUser(warnNonUnitIf :: Nil) val warnValueDiscard = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.") withAbbreviation "-Ywarn-value-discard" val warnNumericWiden = BooleanSetting("-Wnumeric-widen", "Warn when numerics are widened.") withAbbreviation "-Ywarn-numeric-widen" val warnOctalLiteral = BooleanSetting("-Woctal-literal", "Warn on obsolete octal syntax.") withAbbreviation "-Ywarn-octal-literal" diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index d8cd90876047..027c8ff0411c 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1726,19 +1726,83 @@ abstract class RefChecks extends Transform { } // Verify classes extending AnyVal meet the requirements - private def checkAnyValSubclass(clazz: Symbol) = { + private def checkAnyValSubclass(clazz: Symbol) = if (clazz.isDerivedValueClass) { if (clazz.isTrait) reporter.error(clazz.pos, "Only classes (not traits) are allowed to extend AnyVal") else if (clazz.hasAbstractFlag) reporter.error(clazz.pos, "`abstract` modifier cannot be used with value classes") } - } private def checkUnexpandedMacro(t: Tree) = if (!t.isDef && t.hasSymbolField && t.symbol.isTermMacro) reporter.error(t.pos, "macro has not been expanded") + // if expression in statement position (of template or block) + // looks like a useful value that should not be ignored, warn and return true + // User specifies that an expression is boring by ascribing `e: Unit`. + // The subtree `e` will bear an attachment, but may be wrapped in adaptations. + private def checkInterestingResultInStatement(t: Tree): Boolean = { + def isUninterestingSymbol(sym: Symbol): Boolean = + sym != null && ( + sym.isConstructor || + sym.hasPackageFlag || + sym.isPackageObjectOrClass || + sym == BoxedUnitClass || + sym == AnyClass || + sym == AnyRefClass || + sym == AnyValClass + ) + def isUninterestingType(tpe: Type): Boolean = + tpe != null && ( + isUnitType(tpe) || + tpe.typeSymbol.isBottomClass || + tpe =:= UnitTpe || + tpe =:= BoxedUnitTpe || + isTrivialTopType(tpe) + ) + // java lacks this.type idiom to distinguish side-effecting method, so ignore result of invoking java method. + def isJavaApplication(t: Tree): Boolean = t match { + case Apply(f, _) => f.symbol.isJavaDefined && !isUniversalMember(f.symbol) + case _ => false + } + // The quirk of typechecking if is that the LUB often results in boring types. + // Parser adds suppressing attachment on `if (b) expr` when user has `-Wnonunit-if:false`. + def checkInterestingShapes(t: Tree): Boolean = + t match { + case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) | checkInterestingShapes(elsepart) // strict or + //case Block(_, Apply(label, Nil)) if label.symbol != null && nme.isLoopHeaderLabel(label.symbol.name) => false + case Block(_, res) => checkInterestingShapes(res) + case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body)) + case _ => checksForInterestingResult(t) + } + // tests for various flavors of blandness in expressions. + def checksForInterestingResult(t: Tree): Boolean = ( + !t.isDef && !treeInfo.isPureDef(t) // ignore defs + && !isUninterestingSymbol(t.symbol) // ctors, package, Unit, Any + && !isUninterestingType(t.tpe) // bottom types, Unit, Any + && !treeInfo.isThisTypeResult(t) // buf += x + && !treeInfo.isSuperConstrCall(t) // just a thing + && !treeInfo.hasExplicitUnit(t) // suppressed by explicit expr: Unit + && !isJavaApplication(t) // Java methods are inherently side-effecting + ) + // begin checkInterestingResultInStatement + settings.warnNonUnitStatement.value && checkInterestingShapes(t) && { + val msg = "unused value" + val where = t match { + case Block(_, res) => res + case If(_, thenpart, Literal(Constant(()))) => + thenpart match { + case Block(_, res) => res + case _ => thenpart + } + case _ => t + } + refchecksWarning(where.pos, msg, WarningCategory.OtherPureStatement) + true + } + } // end checkInterestingResultInStatement + override def transform(tree: Tree): Tree = { val savedLocalTyper = localTyper val savedCurrentApplication = currentApplication @@ -1778,14 +1842,12 @@ abstract class RefChecks extends Transform { case Template(parents, self, body) => localTyper = localTyper.atOwner(tree, currentOwner) - for (stat <- body) { - if (treeInfo.isPureExprForWarningPurposes(stat)) { + for (stat <- body) + if (!checkInterestingResultInStatement(stat) && 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 "" refchecksWarning(stat.pos, s"$msg$clause", WarningCategory.OtherPureStatement) } - } - validateBaseTypes(currentOwner) checkOverloadedRestrictions(currentOwner, currentOwner) // scala/bug#7870 default getters for constructors live in the companion module @@ -1852,7 +1914,8 @@ abstract class RefChecks extends Transform { // probably not, until we allow parameterised extractors tree - case Block(stats, expr) => + case blk @ Block(stats, expr) => + // diagnostic info val (count, result0, adapted) = expr match { case Block(expr :: Nil, Literal(Constant(()))) => (1, expr, true) @@ -1862,7 +1925,7 @@ abstract class RefChecks extends Transform { val isMultiline = stats.lengthCompare(1 - count) > 0 def checkPure(t: Tree, supple: Boolean): Unit = - if (!analyzer.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 "" @@ -1871,8 +1934,8 @@ abstract class RefChecks extends Transform { else if (!parens.isEmpty) s"$msg; $parens" else msg refchecksWarning(t.pos, text, WarningCategory.OtherPureStatement) } - // check block for unintended expr placement - stats.foreach(checkPure(_, supple = false)) + // check block for unintended "expression in statement position" + stats.foreach { t => if (!checkInterestingResultInStatement(t)) checkPure(t, supple = false) } if (result0.nonEmpty) checkPure(result0, supple = true) def checkImplicitlyAdaptedBlockResult(t: Tree): Unit = diff --git a/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala b/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala index 45b701a33c3f..88f96e34c474 100644 --- a/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala +++ b/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala @@ -196,10 +196,6 @@ trait StdAttachments { * track of other adapted trees. */ case class OriginalTreeAttachment(original: Tree) - - /** Marks a Typed tree with Unit tpt. */ - case object TypedExpectingUnitAttachment - def explicitlyUnit(tree: Tree): Boolean = tree.hasAttachment[TypedExpectingUnitAttachment.type] } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index ad493817d11a..4e9a7fb8ece5 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -750,7 +750,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper if (immediate) { action() } else { - unit.toCheck += (() => action()) + unit.toCheck += (() => action(): Unit) true } } @@ -1112,15 +1112,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper @inline def tpdPos(transformed: Tree) = typedPos(tree.pos, mode, pt)(transformed) @inline def tpd(transformed: Tree) = typed(transformed, mode, pt) - @inline def warnValueDiscard(): Unit = if (!isPastTyper && settings.warnValueDiscard.value) { - def isThisTypeResult = (tree, tree.tpe) match { - case (Apply(Select(receiver, _), _), SingleType(_, sym)) => sym == receiver.symbol - case _ => false - } - if (!isThisTypeResult && !explicitlyUnit(tree)) - context.warning(tree.pos, s"discarded non-Unit value of type ${tree.tpe}", - WarningCategory.WFlagValueDiscard) - } + @inline def warnValueDiscard(): Unit = + if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree)) + context.warning(tree.pos, s"discarded non-Unit value of type ${tree.tpe}", WarningCategory.WFlagValueDiscard) @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) { val isInharmonic = ( @@ -1245,7 +1239,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper case ct @ FoldableConstantType(value) if mode.inNone(TYPEmode | FUNmode) && (ct <:< pt) && canAdaptConstantTypeToLiteral => // (0) adaptConstant(value) case OverloadedType(_, _) if !mode.inFunMode => // (1) - inferExprAlternative(tree, pt) + inferExprAlternative(tree, pt): Unit adaptAfterOverloadResolution(tree, mode, pt, original) case NullaryMethodType(restpe) => // (2) if (hasUndets && settings.lintUniversalMethods && (isCastSymbol(tree.symbol) || isTypeTestSymbol(tree.symbol)) && context.undetparams.exists(_.owner == tree.symbol)) @@ -4722,7 +4716,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper def typedIf(tree: If): If = { val cond1 = checkDead(context, typedByValueExpr(tree.cond, BooleanTpe)) - // One-legged ifs don't need a lot of analysis + // Unibranch if normally has unit value else, but synthetic code may emit empty else. if (tree.elsep.isEmpty) return treeCopy.If(tree, cond1, typed(tree.thenp, UnitTpe), tree.elsep) setType UnitTpe diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index 926e2927b6a4..6045c42adec3 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -145,4 +145,8 @@ trait StdAttachments { // Use of _root_ is in correct leading position of selection case object RootSelection extends PlainAttachment + + /** Marks a Typed tree with Unit tpt. */ + case object TypedExpectingUnitAttachment + def explicitlyUnit(tree: Tree): Boolean = tree.hasAttachment[TypedExpectingUnitAttachment.type] } diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 3ba0f488e27a..ebd0772ee637 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -351,6 +351,40 @@ abstract class TreeInfo { case _ => false } + /** Is tree an application with result `this.type`? + * Accept `b.addOne(x)` and also `xs(i) += x` + * where the op is an assignment operator. + */ + def isThisTypeResult(tree: Tree): Boolean = tree match { + case Applied(fun @ Select(receiver, op), _, _) => + tree.tpe match { + case ThisType(sym) => sym == receiver.symbol + case SingleType(_, sym) => sym == receiver.symbol + case _ => + def check(sym: Symbol): Boolean = + (sym == receiver.symbol) || { + receiver match { + case Apply(_, _) => Precedence(op.decoded).level == 0 // xs(i) += x + case _ => receiver.symbol != null && + (receiver.symbol.isGetter || receiver.symbol.isField) // xs.addOne(x) for var xs + } + } + @tailrec def loop(mt: Type): Boolean = mt match { + case MethodType(_, restpe) => + restpe match { + case ThisType(sym) => check(sym) + case SingleType(_, sym) => check(sym) + case _ => loop(restpe) + } + case PolyType(_, restpe) => loop(restpe) + case _ => false + } + fun.symbol != null && loop(fun.symbol.info) + } + case _ => + tree.tpe.isInstanceOf[ThisType] + } + /** * Named arguments can transform a constructor call into a block, e.g. * (b = foo, a = bar) @@ -746,6 +780,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/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index 27901be7c301..ead2054e9092 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -77,6 +77,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => this.ChangeOwnerAttachment this.InterpolatedString this.RootSelection + this.TypedExpectingUnitAttachment this.noPrint this.typeDebug // inaccessible: this.posAssigner diff --git a/test/files/neg/nonunit-if.check b/test/files/neg/nonunit-if.check new file mode 100644 index 000000000000..02d2b4dcf931 --- /dev/null +++ b/test/files/neg/nonunit-if.check @@ -0,0 +1,57 @@ +nonunit-if.scala:58: warning: discarded non-Unit value of type U + if (!isEmpty) f(a) // warn, check is on + ^ +nonunit-if.scala:62: warning: discarded non-Unit value of type Boolean + f(a) // warn, check is on + ^ +nonunit-if.scala:13: warning: unused value + improved // warn + ^ +nonunit-if.scala:20: warning: unused value + new E().toString // warn + ^ +nonunit-if.scala:26: warning: unused value + Future(42) // warn + ^ +nonunit-if.scala:30: warning: unused value + copy() // warn + ^ +nonunit-if.scala:37: warning: unused value + 27 +: xs // warn + ^ +nonunit-if.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses + null // warn for purity + ^ +nonunit-if.scala:58: warning: unused value + if (!isEmpty) f(a) // warn, check is on + ^ +nonunit-if.scala:62: warning: unused value + f(a) // warn, check is on + ^ +nonunit-if.scala:73: warning: unused value + if (!fellback) action(z) // warn, check is on + ^ +nonunit-if.scala:81: warning: unused value + g // warn, check is on + ^ +nonunit-if.scala:79: warning: unused value + g // warn block statement + ^ +nonunit-if.scala:86: warning: unused value + g // warn + ^ +nonunit-if.scala:84: warning: unused value + g // warn + ^ +nonunit-if.scala:96: warning: unused value + if (b) { // warn, at least one branch looks interesting + ^ +nonunit-if.scala:116: warning: unused value + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + ^ +nonunit-if.scala:146: warning: unused value + while (it.hasNext) it.next() // warn + ^ +error: No warnings can be incurred under -Werror. +18 warnings +1 error diff --git a/test/files/neg/nonunit-if.scala b/test/files/neg/nonunit-if.scala new file mode 100644 index 000000000000..17aee82cc15c --- /dev/null +++ b/test/files/neg/nonunit-if.scala @@ -0,0 +1,191 @@ +// scalac: -Werror -Wnonunit-statement -Wnonunit-if:true -Wvalue-discard +// debug: -Vprint:refchecks -Yprint-trees:format +import collection.ArrayOps +import collection.mutable.{ArrayBuilder, LinkedHashSet, ListBuffer} +import concurrent._ +import scala.reflect.ClassTag + +class C { + import ExecutionContext.Implicits._ + def c = { + def improved = Future(42) + def stale = Future(27) + improved // warn + stale + } +} +class D { + def d = { + class E + new E().toString // warn + new E().toString * 2 + } +} +class F { + import ExecutionContext.Implicits._ + Future(42) // warn +} +// unused template expression uses synthetic method of class +case class K(s: String) { + copy() // warn +} +// mutations returning this are ok +class Mutate { + val b = ListBuffer.empty[Int] + b += 42 // nowarn, returns this.type + val xs = List(42) + 27 +: xs // warn + + def f(x: Int): this.type = this + def g(): Unit = f(42) // nowarn +} +// some uninteresting expressions may warn for other reasons +class WhoCares { + null // warn for purity + ??? // nowarn for impurity +} +// explicit Unit ascription to opt out of warning, even for funky applies +class Absolution { + def f(i: Int): Int = i+1 + import ExecutionContext.Implicits._ + Future(42): Unit // nowarn { F(42)(ctx) }: Unit where annot is on F(42) + f(42): Unit // nowarn +} +// warn uni-branched unless user disables it with -Wnonunit-if:false +class Boxed[A](a: A) { + def isEmpty = false + def foreach[U](f: A => U): Unit = + if (!isEmpty) f(a) // warn, check is on + def forall(f: A => Boolean): Unit = + if (!isEmpty) { + println(".") + f(a) // warn, check is on + } + def take(p: A => Boolean): Option[A] = { + while (isEmpty || !p(a)) () + Some(a).filter(p) + } +} +class Unibranch[A, B] { + def runWith[U](action: B => U): A => Boolean = { x => + val z = null.asInstanceOf[B] + val fellback = false + if (!fellback) action(z) // warn, check is on + !fellback + } + def f(i: Int): Int = { + def g = 17 + if (i < 42) { + g // warn block statement + println("uh oh") + g // warn, check is on + } + while (i < 42) { + g // warn + println("uh oh") + g // warn + } + 42 + } +} +class Dibranch { + def i: Int = ??? + def j: Int = ??? + def f(b: Boolean): Int = { + // if-expr might have an uninteresting LUB + if (b) { // warn, at least one branch looks interesting + println("true") + i + } + else { + println("false") + j + } + 42 + } +} +class Next[A] { + val all = ListBuffer.empty[A] + def f(it: Iterator[A], g: A => A): Unit = + while (it.hasNext) + all += g(it.next()) // nowarn +} +class Setting[A] { + def set = LinkedHashSet.empty[A] + def f(a: A): Unit = { + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + println(set) + } +} +// neither StringBuilder warns, because either append is Java method or returns this.type +// while loop looks like if branch with block1(block2, jump to label), where block2 typed as non-unit +class Strung { + def iterator = Iterator.empty[String] + def addString(b: StringBuilder, start: String, sep: String, end: String): StringBuilder = { + val jsb = b.underlying + if (start.length != 0) jsb.append(start) + val it = iterator + if (it.hasNext) { + jsb.append(it.next()) + while (it.hasNext) { + jsb.append(sep) + jsb.append(it.next()) + } + } + if (end.length != 0) jsb.append(end) + b + } + def f(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) { + b.append("\n") + b.append(it.next()) + } + b.toString + } + def g(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) it.next() // warn + b.toString + } +} +class J { + import java.util.Collections + def xs: java.util.List[Int] = ??? + def f(): Int = { + Collections.checkedList[Int](xs, classOf[Int]) + 42 + } +} +class Variant { + var bs = ListBuffer.empty[Int] + val xs = ListBuffer.empty[Int] + private[this] val ys = ListBuffer.empty[Int] + private[this] var zs = ListBuffer.empty[Int] + def f(i: Int): Unit = { + bs.addOne(i) + xs.addOne(i) + ys.addOne(i) + zs.addOne(i) + println("done") + } +} +final class ArrayOops[A](private val xs: Array[A]) extends AnyVal { + def other: ArrayOps[A] = ??? + def transpose[B](implicit asArray: A => Array[B]): Array[Array[B]] = { + val aClass = xs.getClass.getComponentType + val bb = new ArrayBuilder.ofRef[Array[B]]()(ClassTag[Array[B]](aClass)) + if (xs.length == 0) bb.result() + else { + def mkRowBuilder() = ArrayBuilder.make[B](ClassTag[B](aClass.getComponentType)) + val bs = new ArrayOps(asArray(xs(0))).map((x: B) => mkRowBuilder()) + for (xs <- other) { + var i = 0 + for (x <- new ArrayOps(asArray(xs))) { + bs(i) += x + i += 1 + } + } + for (b <- new ArrayOps(bs)) bb += b.result() + bb.result() + } + } +} diff --git a/test/files/neg/nonunit-statement.check b/test/files/neg/nonunit-statement.check new file mode 100644 index 000000000000..f98bcd3bc607 --- /dev/null +++ b/test/files/neg/nonunit-statement.check @@ -0,0 +1,39 @@ +nonunit-statement.scala:13: warning: unused value + improved // warn + ^ +nonunit-statement.scala:20: warning: unused value + new E().toString // warn + ^ +nonunit-statement.scala:26: warning: unused value + Future(42) // warn + ^ +nonunit-statement.scala:30: warning: unused value + copy() // warn + ^ +nonunit-statement.scala:37: warning: unused value + 27 +: xs // warn + ^ +nonunit-statement.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses + null // warn for purity + ^ +nonunit-statement.scala:79: warning: unused value + g // warn block statement + ^ +nonunit-statement.scala:86: warning: unused value + g // warn + ^ +nonunit-statement.scala:84: warning: unused value + g // warn + ^ +nonunit-statement.scala:96: warning: unused value + if (b) { // warn, at least one branch looks interesting + ^ +nonunit-statement.scala:116: warning: unused value + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + ^ +nonunit-statement.scala:146: warning: unused value + while (it.hasNext) it.next() // warn + ^ +error: No warnings can be incurred under -Werror. +12 warnings +1 error diff --git a/test/files/neg/nonunit-statement.scala b/test/files/neg/nonunit-statement.scala new file mode 100644 index 000000000000..715a7cd3ebf5 --- /dev/null +++ b/test/files/neg/nonunit-statement.scala @@ -0,0 +1,191 @@ +// scalac: -Werror -Wnonunit-statement -Wnonunit-if:false -Wvalue-discard +// debug: -Vprint:refchecks -Yprint-trees:format +import collection.ArrayOps +import collection.mutable.{ArrayBuilder, LinkedHashSet, ListBuffer} +import concurrent._ +import scala.reflect.ClassTag + +class C { + import ExecutionContext.Implicits._ + def c = { + def improved = Future(42) + def stale = Future(27) + improved // warn + stale + } +} +class D { + def d = { + class E + new E().toString // warn + new E().toString * 2 + } +} +class F { + import ExecutionContext.Implicits._ + Future(42) // warn +} +// unused template expression uses synthetic method of class +case class K(s: String) { + copy() // warn +} +// mutations returning this are ok +class Mutate { + val b = ListBuffer.empty[Int] + b += 42 // nowarn, returns this.type + val xs = List(42) + 27 +: xs // warn + + def f(x: Int): this.type = this + def g(): Unit = f(42) // nowarn +} +// some uninteresting expressions may warn for other reasons +class WhoCares { + null // warn for purity + ??? // nowarn for impurity +} +// explicit Unit ascription to opt out of warning, even for funky applies +class Absolution { + def f(i: Int): Int = i+1 + import ExecutionContext.Implicits._ + Future(42): Unit // nowarn { F(42)(ctx) }: Unit where annot is on F(42) + f(42): Unit // nowarn +} +// warn uni-branched unless user disables it with -Wnonunit-if:false +class Boxed[A](a: A) { + def isEmpty = false + def foreach[U](f: A => U): Unit = + if (!isEmpty) f(a) // nowarn, check is off + def forall(f: A => Boolean): Unit = + if (!isEmpty) { + println(".") + f(a) // nowarn, check is off + } + def take(p: A => Boolean): Option[A] = { + while (isEmpty || !p(a)) () + Some(a).filter(p) + } +} +class Unibranch[A, B] { + def runWith[U](action: B => U): A => Boolean = { x => + val z = null.asInstanceOf[B] + val fellback = false + if (!fellback) action(z) // nowarn, check is off + !fellback + } + def f(i: Int): Int = { + def g = 17 + if (i < 42) { + g // warn block statement + println("uh oh") + g // nowarn, check is off + } + while (i < 42) { + g // warn + println("uh oh") + g // warn + } + 42 + } +} +class Dibranch { + def i: Int = ??? + def j: Int = ??? + def f(b: Boolean): Int = { + // if-expr might have an uninteresting LUB + if (b) { // warn, at least one branch looks interesting + println("true") + i + } + else { + println("false") + j + } + 42 + } +} +class Next[A] { + val all = ListBuffer.empty[A] + def f(it: Iterator[A], g: A => A): Unit = + while (it.hasNext) + all += g(it.next()) // nowarn +} +class Setting[A] { + def set = LinkedHashSet.empty[A] + def f(a: A): Unit = { + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + println(set) + } +} +// neither StringBuilder warns, because either append is Java method or returns this.type +// while loop looks like if branch with block1(block2, jump to label), where block2 typed as non-unit +class Strung { + def iterator = Iterator.empty[String] + def addString(b: StringBuilder, start: String, sep: String, end: String): StringBuilder = { + val jsb = b.underlying + if (start.length != 0) jsb.append(start) + val it = iterator + if (it.hasNext) { + jsb.append(it.next()) + while (it.hasNext) { + jsb.append(sep) + jsb.append(it.next()) + } + } + if (end.length != 0) jsb.append(end) + b + } + def f(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) { + b.append("\n") + b.append(it.next()) + } + b.toString + } + def g(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) it.next() // warn + b.toString + } +} +class J { + import java.util.Collections + def xs: java.util.List[Int] = ??? + def f(): Int = { + Collections.checkedList[Int](xs, classOf[Int]) + 42 + } +} +class Variant { + var bs = ListBuffer.empty[Int] + val xs = ListBuffer.empty[Int] + private[this] val ys = ListBuffer.empty[Int] + private[this] var zs = ListBuffer.empty[Int] + def f(i: Int): Unit = { + bs.addOne(i) + xs.addOne(i) + ys.addOne(i) + zs.addOne(i) + println("done") + } +} +final class ArrayOops[A](private val xs: Array[A]) extends AnyVal { + def other: ArrayOps[A] = ??? + def transpose[B](implicit asArray: A => Array[B]): Array[Array[B]] = { + val aClass = xs.getClass.getComponentType + val bb = new ArrayBuilder.ofRef[Array[B]]()(ClassTag[Array[B]](aClass)) + if (xs.length == 0) bb.result() + else { + def mkRowBuilder() = ArrayBuilder.make[B](ClassTag[B](aClass.getComponentType)) + val bs = new ArrayOps(asArray(xs(0))).map((x: B) => mkRowBuilder()) + for (xs <- other) { + var i = 0 + for (x <- new ArrayOps(asArray(xs))) { + bs(i) += x + i += 1 + } + } + for (b <- new ArrayOps(bs)) bb += b.result() + bb.result() + } + } +} diff --git a/test/files/neg/value-discard.check b/test/files/neg/value-discard.check index 642a6739b60a..92e63c680954 100644 --- a/test/files/neg/value-discard.check +++ b/test/files/neg/value-discard.check @@ -1,11 +1,11 @@ value-discard.scala:6: warning: discarded non-Unit value of type Boolean - mutable.Set[String]().remove("") // expected to warn + mutable.Set[String]().remove("") // warn because suspicious receiver ^ value-discard.scala:13: warning: discarded non-Unit value of type scala.collection.mutable.Set[String] - def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn + def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn because suspicious receiver ^ value-discard.scala:17: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses - "" // warn + "" // warn pure expr ^ error: No warnings can be incurred under -Werror. 3 warnings diff --git a/test/files/neg/value-discard.scala b/test/files/neg/value-discard.scala index 2306bc917da4..eed8a29a4d12 100644 --- a/test/files/neg/value-discard.scala +++ b/test/files/neg/value-discard.scala @@ -1,21 +1,21 @@ -// scalac: -Ywarn-value-discard -Xfatal-warnings +// scalac: -Wvalue-discard -Werror final class UnusedTest { import scala.collection.mutable def remove(): Unit = { - mutable.Set[String]().remove("") // expected to warn + mutable.Set[String]().remove("") // warn because suspicious receiver } def removeAscribed(): Unit = { - mutable.Set[String]().remove(""): Unit // no warn + mutable.Set[String]().remove(""): Unit // nowarn } - def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn + def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn because suspicious receiver def warnings(): Unit = { val s: mutable.Set[String] = mutable.Set.empty[String] - "" // warn - "": Unit // no warn - s.subtractOne("") // no warn + "" // warn pure expr + "": Unit // nowarn + s.subtractOne("") // nowarn } } diff --git a/test/files/neg/virtpatmat_unreach_select.scala b/test/files/neg/virtpatmat_unreach_select.scala index b3ca30e2eab0..db6caf85b1a3 100644 --- a/test/files/neg/virtpatmat_unreach_select.scala +++ b/test/files/neg/virtpatmat_unreach_select.scala @@ -1,4 +1,4 @@ -// scalac: -Xfatal-warnings +// scalac: -Werror // class Test { object severity extends Enumeration From b599c60502afbf36fcf6489ffcc245f6c09befb4 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 27 Jul 2022 08:20:30 -0700 Subject: [PATCH 2/4] Tweak setInfo and do not warn on return of arg --- src/reflect/scala/reflect/api/Internals.scala | 2 +- .../reflect/internal/ReificationSupport.scala | 2 +- .../scala/reflect/internal/TreeInfo.scala | 8 +++-- test/files/neg/nonunit-statement.scala | 8 +++++ test/files/pos/skunky-expansion.scala | 31 +++++++++++++++++++ test/files/pos/skunky.scala | 15 +++++++++ 6 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 test/files/pos/skunky-expansion.scala create mode 100644 test/files/pos/skunky.scala diff --git a/src/reflect/scala/reflect/api/Internals.scala b/src/reflect/scala/reflect/api/Internals.scala index 93c4fe85118e..1478f55f897f 100644 --- a/src/reflect/scala/reflect/api/Internals.scala +++ b/src/reflect/scala/reflect/api/Internals.scala @@ -530,7 +530,7 @@ trait Internals { self: Universe => /** Set symbol's type signature to given type. * @return the symbol itself */ - def setInfo[S <: Symbol](sym: S, tpe: Type): S + def setInfo[S <: Symbol](sym: S, tpe: Type): sym.type /** Set symbol's annotations to given annotations `annots`. */ diff --git a/src/reflect/scala/reflect/internal/ReificationSupport.scala b/src/reflect/scala/reflect/internal/ReificationSupport.scala index b47a09ea13b2..4a02e4eba0c3 100644 --- a/src/reflect/scala/reflect/internal/ReificationSupport.scala +++ b/src/reflect/scala/reflect/internal/ReificationSupport.scala @@ -60,7 +60,7 @@ trait ReificationSupport { self: SymbolTable => def setAnnotations[S <: Symbol](sym: S, annots: List[AnnotationInfo]): S = sym.setAnnotations(annots) - def setInfo[S <: Symbol](sym: S, tpe: Type): S = + def setInfo[S <: Symbol](sym: S, tpe: Type): sym.type = sym.setInfo(tpe).markAllCompleted() def mkThis(sym: Symbol): Tree = self.This(sym) diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index ebd0772ee637..5b3c38994cc2 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -356,10 +356,12 @@ abstract class TreeInfo { * where the op is an assignment operator. */ def isThisTypeResult(tree: Tree): Boolean = tree match { - case Applied(fun @ Select(receiver, op), _, _) => + case Applied(fun @ Select(receiver, op), _, argss) => tree.tpe match { - case ThisType(sym) => sym == receiver.symbol - case SingleType(_, sym) => sym == receiver.symbol + case ThisType(sym) => + sym == receiver.symbol + case SingleType(p, sym) => + sym == receiver.symbol || argss.exists(_.exists(sym == _.symbol)) case _ => def check(sym: Symbol): Boolean = (sym == receiver.symbol) || { diff --git a/test/files/neg/nonunit-statement.scala b/test/files/neg/nonunit-statement.scala index 715a7cd3ebf5..192091dbd034 100644 --- a/test/files/neg/nonunit-statement.scala +++ b/test/files/neg/nonunit-statement.scala @@ -189,3 +189,11 @@ final class ArrayOops[A](private val xs: Array[A]) extends AnyVal { } } } +class Depends { + def f[A](a: A): a.type = a + def g() = { + val d = new Depends + f(d) + () + } +} diff --git a/test/files/pos/skunky-expansion.scala b/test/files/pos/skunky-expansion.scala new file mode 100644 index 000000000000..d3f26c83a210 --- /dev/null +++ b/test/files/pos/skunky-expansion.scala @@ -0,0 +1,31 @@ +// scalac: -Werror -Wnonunit-statement +// +import scala.reflect.macros._ +import scala.reflect.api.TypeCreator + +abstract trait Encoder[A] extends scala.AnyRef; +object StringContextOps extends scala.AnyRef { + class StringOpsMacros(c: scala.reflect.macros.whitebox.Context) extends scala.AnyRef { + def sql_impl(argSeq: StringOpsMacros.this.c.universe.Tree*): AnyRef = { + val EncoderType: StringOpsMacros.this.c.universe.Type = StringOpsMacros.this.c.universe.typeOf[Encoder[_]](({ + val $u: StringOpsMacros.this.c.universe.type = StringOpsMacros.this.c.universe; + val $m: $u.Mirror = StringOpsMacros.this.c.universe.rootMirror; + $u.TypeTag.apply[Encoder[_]]($m, { + final class $typecreator1 extends TypeCreator { + def apply[U <: scala.reflect.api.Universe with Singleton]($m$untyped: scala.reflect.api.Mirror[U]): U#Type = { + val $u: U = $m$untyped.universe; + val $m: $u.Mirror = $m$untyped.asInstanceOf[$u.Mirror]; + val symdef$EncoderType1: $u.Symbol = $u.internal.reificationSupport.newNestedSymbol($u.internal.reificationSupport.selectTerm($u.internal.reificationSupport.selectType($m.staticModule("StringContextOps").asModule.moduleClass, "StringOpsMacros"), "sql_impl"), $u.TermName.apply("EncoderType"), $u.NoPosition, $u.internal.reificationSupport.FlagsRepr.apply(549755813888L), false); + val symdef$_$11: $u.Symbol = $u.internal.reificationSupport.newNestedSymbol(symdef$EncoderType1, $u.TypeName.apply("_$1"), $u.NoPosition, $u.internal.reificationSupport.FlagsRepr.apply(34359738384L), false); + $u.internal.reificationSupport.setInfo[$u.Symbol](symdef$EncoderType1, $u.NoType); + $u.internal.reificationSupport.setInfo[$u.Symbol](symdef$_$11, $u.internal.reificationSupport.TypeBounds($m.staticClass("scala.Nothing").asType.toTypeConstructor, $m.staticClass("scala.Any").asType.toTypeConstructor)); + $u.internal.reificationSupport.ExistentialType(scala.collection.immutable.List.apply[$u.Symbol](symdef$_$11), $u.internal.reificationSupport.TypeRef($u.internal.reificationSupport.thisPrefix($m.EmptyPackageClass), $m.staticClass("Encoder"), scala.collection.immutable.List.apply[$u.Type]($u.internal.reificationSupport.TypeRef($u.NoPrefix, symdef$_$11, scala.collection.immutable.Nil)))) + } + }; + new $typecreator1() + }) + }: StringOpsMacros.this.c.universe.TypeTag[Encoder[_]])); + argSeq.head + } + } +} diff --git a/test/files/pos/skunky.scala b/test/files/pos/skunky.scala new file mode 100644 index 000000000000..1c5f5730a0ab --- /dev/null +++ b/test/files/pos/skunky.scala @@ -0,0 +1,15 @@ +// scalac: -Werror -Wnonunit-statement + +import scala.reflect.macros._ + +trait Encoder[A] + +object StringContextOps { + class StringOpsMacros(val c: whitebox.Context) { + import c.universe._ + def sql_impl(argSeq: Tree*): Tree = { + val EncoderType = typeOf[Encoder[_]] + argSeq.head + } + } +} From 87744920adc3c7d33e82dc5aef24b173aa37ebe0 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 27 Jul 2022 09:07:02 -0700 Subject: [PATCH 3/4] Mention type of unused value and mitigation --- .../tools/nsc/typechecker/RefChecks.scala | 2 +- test/files/neg/nonunit-if.check | 30 +++++++++---------- test/files/neg/nonunit-statement.check | 22 +++++++------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 027c8ff0411c..01bfd755f041 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1788,7 +1788,6 @@ abstract class RefChecks extends Transform { ) // begin checkInterestingResultInStatement settings.warnNonUnitStatement.value && checkInterestingShapes(t) && { - val msg = "unused value" val where = t match { case Block(_, res) => res case If(_, thenpart, Literal(Constant(()))) => @@ -1798,6 +1797,7 @@ abstract class RefChecks extends Transform { } case _ => t } + def msg = s"unused value of type ${where.tpe} (add `: Unit` to discard silently)" refchecksWarning(where.pos, msg, WarningCategory.OtherPureStatement) true } diff --git a/test/files/neg/nonunit-if.check b/test/files/neg/nonunit-if.check index 02d2b4dcf931..d1ffb1337ef3 100644 --- a/test/files/neg/nonunit-if.check +++ b/test/files/neg/nonunit-if.check @@ -4,52 +4,52 @@ nonunit-if.scala:58: warning: discarded non-Unit value of type U nonunit-if.scala:62: warning: discarded non-Unit value of type Boolean f(a) // warn, check is on ^ -nonunit-if.scala:13: warning: unused value +nonunit-if.scala:13: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) improved // warn ^ -nonunit-if.scala:20: warning: unused value +nonunit-if.scala:20: warning: unused value of type String (add `: Unit` to discard silently) new E().toString // warn ^ -nonunit-if.scala:26: warning: unused value +nonunit-if.scala:26: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) Future(42) // warn ^ -nonunit-if.scala:30: warning: unused value +nonunit-if.scala:30: warning: unused value of type K (add `: Unit` to discard silently) copy() // warn ^ -nonunit-if.scala:37: warning: unused value +nonunit-if.scala:37: warning: unused value of type List[Int] (add `: Unit` to discard silently) 27 +: xs // warn ^ nonunit-if.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses null // warn for purity ^ -nonunit-if.scala:58: warning: unused value +nonunit-if.scala:58: warning: unused value of type U (add `: Unit` to discard silently) if (!isEmpty) f(a) // warn, check is on ^ -nonunit-if.scala:62: warning: unused value +nonunit-if.scala:62: warning: unused value of type Boolean (add `: Unit` to discard silently) f(a) // warn, check is on ^ -nonunit-if.scala:73: warning: unused value +nonunit-if.scala:73: warning: unused value of type U (add `: Unit` to discard silently) if (!fellback) action(z) // warn, check is on ^ -nonunit-if.scala:81: warning: unused value +nonunit-if.scala:81: warning: unused value of type Int (add `: Unit` to discard silently) g // warn, check is on ^ -nonunit-if.scala:79: warning: unused value +nonunit-if.scala:79: warning: unused value of type Int (add `: Unit` to discard silently) g // warn block statement ^ -nonunit-if.scala:86: warning: unused value +nonunit-if.scala:86: warning: unused value of type Int (add `: Unit` to discard silently) g // warn ^ -nonunit-if.scala:84: warning: unused value +nonunit-if.scala:84: warning: unused value of type Int (add `: Unit` to discard silently) g // warn ^ -nonunit-if.scala:96: warning: unused value +nonunit-if.scala:96: warning: unused value of type Int (add `: Unit` to discard silently) if (b) { // warn, at least one branch looks interesting ^ -nonunit-if.scala:116: warning: unused value +nonunit-if.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A] (add `: Unit` to discard silently) set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned ^ -nonunit-if.scala:146: warning: unused value +nonunit-if.scala:146: warning: unused value of type String (add `: Unit` to discard silently) while (it.hasNext) it.next() // warn ^ error: No warnings can be incurred under -Werror. diff --git a/test/files/neg/nonunit-statement.check b/test/files/neg/nonunit-statement.check index f98bcd3bc607..b15ae34fb7fd 100644 --- a/test/files/neg/nonunit-statement.check +++ b/test/files/neg/nonunit-statement.check @@ -1,37 +1,37 @@ -nonunit-statement.scala:13: warning: unused value +nonunit-statement.scala:13: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) improved // warn ^ -nonunit-statement.scala:20: warning: unused value +nonunit-statement.scala:20: warning: unused value of type String (add `: Unit` to discard silently) new E().toString // warn ^ -nonunit-statement.scala:26: warning: unused value +nonunit-statement.scala:26: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) Future(42) // warn ^ -nonunit-statement.scala:30: warning: unused value +nonunit-statement.scala:30: warning: unused value of type K (add `: Unit` to discard silently) copy() // warn ^ -nonunit-statement.scala:37: warning: unused value +nonunit-statement.scala:37: warning: unused value of type List[Int] (add `: Unit` to discard silently) 27 +: xs // warn ^ nonunit-statement.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses null // warn for purity ^ -nonunit-statement.scala:79: warning: unused value +nonunit-statement.scala:79: warning: unused value of type Int (add `: Unit` to discard silently) g // warn block statement ^ -nonunit-statement.scala:86: warning: unused value +nonunit-statement.scala:86: warning: unused value of type Int (add `: Unit` to discard silently) g // warn ^ -nonunit-statement.scala:84: warning: unused value +nonunit-statement.scala:84: warning: unused value of type Int (add `: Unit` to discard silently) g // warn ^ -nonunit-statement.scala:96: warning: unused value +nonunit-statement.scala:96: warning: unused value of type Int (add `: Unit` to discard silently) if (b) { // warn, at least one branch looks interesting ^ -nonunit-statement.scala:116: warning: unused value +nonunit-statement.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A] (add `: Unit` to discard silently) set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned ^ -nonunit-statement.scala:146: warning: unused value +nonunit-statement.scala:146: warning: unused value of type String (add `: Unit` to discard silently) while (it.hasNext) it.next() // warn ^ error: No warnings can be incurred under -Werror. From 9fe44e8cac5bbb713ab9119b1222049cdd326843 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 28 Jul 2022 01:47:35 -0700 Subject: [PATCH 4/4] Add VBAR per eagle-eyed review --- src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 2 +- src/reflect/scala/reflect/internal/TreeInfo.scala | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 01bfd755f041..2b455e38e17f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1770,7 +1770,7 @@ abstract class RefChecks extends Transform { // Parser adds suppressing attachment on `if (b) expr` when user has `-Wnonunit-if:false`. def checkInterestingShapes(t: Tree): Boolean = t match { - case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) | checkInterestingShapes(elsepart) // strict or + case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart) // either or //case Block(_, Apply(label, Nil)) if label.symbol != null && nme.isLoopHeaderLabel(label.symbol.name) => false case Block(_, res) => checkInterestingShapes(res) case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body)) diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 5b3c38994cc2..97865f121ddb 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -363,7 +363,7 @@ abstract class TreeInfo { case SingleType(p, sym) => sym == receiver.symbol || argss.exists(_.exists(sym == _.symbol)) case _ => - def check(sym: Symbol): Boolean = + def checkSingle(sym: Symbol): Boolean = (sym == receiver.symbol) || { receiver match { case Apply(_, _) => Precedence(op.decoded).level == 0 // xs(i) += x @@ -374,8 +374,8 @@ abstract class TreeInfo { @tailrec def loop(mt: Type): Boolean = mt match { case MethodType(_, restpe) => restpe match { - case ThisType(sym) => check(sym) - case SingleType(_, sym) => check(sym) + case ThisType(sym) => checkSingle(sym) + case SingleType(_, sym) => checkSingle(sym) case _ => loop(restpe) } case PolyType(_, restpe) => loop(restpe)