From 55d9a46c30fb5595b574b0ea8b18b7e79437fad5 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 28 Jun 2023 22:13:58 +0200 Subject: [PATCH 1/4] Improve logic when to emit pattern type error Improve logic when to emit "pattern type is incompatible with expected type" error. Fixes #18083 The whole thing is not very satisfactory. We have an approximation which has both false positives and false negatives. False positives: We are too lenient for numeric types and and/or types False negatives: We ignore user-generated equals methods The new approximation seems to be somewhat better than the old, but it's still an approximation. It begs the question why we attempt to do this at all. --- .../tools/dotc/core/SymDenotations.scala | 6 ++- .../src/dotty/tools/dotc/typer/Typer.scala | 48 ++++++++++++------- tests/neg/i5004.scala | 4 +- tests/pos/i18083.scala | 9 ++++ 4 files changed, 46 insertions(+), 21 deletions(-) create mode 100644 tests/pos/i18083.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 304840396641..1b3a89100317 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2020,8 +2020,10 @@ object SymDenotations { * @return The result may contain false positives, but never false negatives. */ final def mayHaveCommonChild(that: ClassSymbol)(using Context): Boolean = - !this.is(Final) && !that.is(Final) && (this.is(Trait) || that.is(Trait)) || - this.derivesFrom(that) || that.derivesFrom(this.symbol) + this.is(Trait) && !that.isEffectivelyFinal + || that.is(Trait) && !this.isEffectivelyFinal + || this.derivesFrom(that) + || that.derivesFrom(this.symbol) final override def typeParamCreationFlags: FlagSet = ClassTypeParamCreationFlags diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 51787cb5004a..a38ddd534a2c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -4380,23 +4380,37 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer mapOver(tp) } - // Is it certain that a value of `tree.tpe` is never a subtype of `pt`? - // It is true if either - // - the class of `tree.tpe` and class of `pt` cannot have common subclass, or - // - `tree` is an object or enum value, which cannot possibly be a subtype of `pt` - val isDefiniteNotSubtype = { - val clsA = tree.tpe.widenDealias.classSymbol - val clsB = pt.dealias.classSymbol - clsA.exists && clsB.exists - && clsA != defn.NullClass - && (!clsA.isNumericValueClass && !clsB.isNumericValueClass) // approximation for numeric conversion and boxing - && !clsA.asClass.mayHaveCommonChild(clsB.asClass) - || tree.symbol.isOneOf(Module | Enum) - && !(tree.tpe frozen_<:< pt) // fast track - && !(tree.tpe frozen_<:< approx(pt)) - } - - if isDefiniteNotSubtype then + // Is it possible that a value of `clsA` is equal to a value of `clsB`? + // This ignores user-defined equals methods, but makes an exception + // for numeric classes. + def canOverlap(clsA: ClassSymbol, clsB: ClassSymbol): Boolean = + clsA.mayHaveCommonChild(clsB) + || clsA.isNumericValueClass // this is quite coarse, but matches to what was done before + || clsB.isNumericValueClass + + // Can type `a` possiblly have a common instance with type `b`? + def canEqual(a: Type, b: Type): Boolean = trace(i"canEqual $a $b"): + b match + case _: TypeRef | _: AppliedType if b.typeSymbol.isClass => + a match + case a: TermRef if a.symbol.isOneOf(Module | Enum) => + (a frozen_<:< b) // fast track + || (a frozen_<:< approx(b)) + case _: TypeRef | _: AppliedType if a.typeSymbol.isClass => + if a.isNullType then !b.isNotNull + else canOverlap(a.typeSymbol.asClass, b.typeSymbol.asClass) + case a: TypeProxy => + canEqual(a.superType, b) + case a: AndOrType => + canEqual(a.tp1, b) || canEqual(a.tp2, b) + case b: TypeProxy => + canEqual(a, b.superType) + case b: AndOrType => + // we lose precision with and/or types, but it's hard to do better and + // still compute `canEqual(A & B, B & A) = true`. + canEqual(a, b.tp1) || canEqual(a, b.tp2) + + if !canEqual(tree.tpe, pt) then // We could check whether `equals` is overridden. // Reasons for not doing so: // - it complicates the protocol diff --git a/tests/neg/i5004.scala b/tests/neg/i5004.scala index a8acfa231bca..02105104efd1 100644 --- a/tests/neg/i5004.scala +++ b/tests/neg/i5004.scala @@ -1,6 +1,6 @@ object i0 { 1 match { def this(): Int // error - def this() // error -} + def this() +} // error } diff --git a/tests/pos/i18083.scala b/tests/pos/i18083.scala new file mode 100644 index 000000000000..c7e35a51f4d0 --- /dev/null +++ b/tests/pos/i18083.scala @@ -0,0 +1,9 @@ +sealed trait A +case class Sub1() extends A +case object Sub2 extends A + +def test(x: A | Null): Int = + if x == null then return 0 + x match + case Sub1() => 1 + case Sub2 => 2 From 01d68c6efd8053314f3719b9c1afba5fb0bc031c Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 28 Jun 2023 22:32:42 +0200 Subject: [PATCH 2/4] A refactoring --- .../dotty/tools/dotc/core/Definitions.scala | 27 ++++++++++--------- .../dotty/tools/dotc/typer/Synthesizer.scala | 2 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 3e839730c42c..ae96ad9c2171 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -2009,20 +2009,21 @@ class Definitions { vcls } + def boxedClass(cls: Symbol): ClassSymbol = + if cls eq ByteClass then BoxedByteClass + else if cls eq ShortClass then BoxedShortClass + else if cls eq CharClass then BoxedCharClass + else if cls eq IntClass then BoxedIntClass + else if cls eq LongClass then BoxedLongClass + else if cls eq FloatClass then BoxedFloatClass + else if cls eq DoubleClass then BoxedDoubleClass + else if cls eq UnitClass then BoxedUnitClass + else if cls eq BooleanClass then BoxedBooleanClass + else sys.error(s"Not a primitive value type: $cls") + /** The type of the boxed class corresponding to primitive value type `tp`. */ - def boxedType(tp: Type)(using Context): TypeRef = { - val cls = tp.classSymbol - if (cls eq ByteClass) BoxedByteClass - else if (cls eq ShortClass) BoxedShortClass - else if (cls eq CharClass) BoxedCharClass - else if (cls eq IntClass) BoxedIntClass - else if (cls eq LongClass) BoxedLongClass - else if (cls eq FloatClass) BoxedFloatClass - else if (cls eq DoubleClass) BoxedDoubleClass - else if (cls eq UnitClass) BoxedUnitClass - else if (cls eq BooleanClass) BoxedBooleanClass - else sys.error(s"Not a primitive value type: $tp") - }.typeRef + def boxedType(tp: Type)(using Context): TypeRef = + boxedClass(tp.classSymbol).typeRef def unboxedType(tp: Type)(using Context): TypeRef = { val cls = tp.classSymbol diff --git a/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala b/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala index 103961b68c29..5bb6a25dbb56 100644 --- a/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala @@ -166,7 +166,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context): def cmpWithBoxed(cls1: ClassSymbol, cls2: ClassSymbol) = cls2 == defn.NothingClass - || cls2 == defn.boxedType(cls1.typeRef).symbol + || cls2 == defn.boxedClass(cls1) || cls1.isNumericValueClass && cls2.derivesFrom(defn.BoxedNumberClass) if cls1.isPrimitiveValueClass then From 1380c909768a5ecc6298142aea3fc6e2d73a4850 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 5 Jul 2023 15:18:07 +0200 Subject: [PATCH 3/4] Check for implausible patterns as a linter option Use -Wimplausible-patterns to do the checks formerly done in checkEqualityEvidence. Create a new source file typer/Linter.scala for all linting checks done at Typer. --- .../tools/dotc/config/ScalaSettings.scala | 2 +- .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 8 ++ .../src/dotty/tools/dotc/typer/Linter.scala | 121 ++++++++++++++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 121 ++---------------- .../gadt-contradictory-pattern.scala | 1 + .../fatal-warnings}/i5077.scala | 1 + .../fatal-warnings}/i9166.scala | 1 + .../fatal-warnings/i9740.check | 12 ++ .../fatal-warnings}/i9740.scala | 1 + .../fatal-warnings}/i9740b.scala | 1 + .../fatal-warnings}/i9740c.scala | 1 + .../fatal-warnings}/i9740d.scala | 2 + tests/neg/i5004.scala | 4 +- 14 files changed, 161 insertions(+), 116 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/typer/Linter.scala rename tests/{neg => neg-custom-args/fatal-warnings}/gadt-contradictory-pattern.scala (90%) rename tests/{neg => neg-custom-args/fatal-warnings}/i5077.scala (96%) rename tests/{neg => neg-custom-args/fatal-warnings}/i9166.scala (79%) create mode 100644 tests/neg-custom-args/fatal-warnings/i9740.check rename tests/{neg => neg-custom-args/fatal-warnings}/i9740.scala (93%) rename tests/{neg => neg-custom-args/fatal-warnings}/i9740b.scala (93%) rename tests/{neg => neg-custom-args/fatal-warnings}/i9740c.scala (91%) rename tests/{neg => neg-custom-args/fatal-warnings}/i9740d.scala (89%) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index e685d8664037..bd3048a8518d 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -161,7 +161,7 @@ private sealed trait WarningSettings: val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings")) val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.") val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.") - + val WimplausiblePatterns = BooleanSetting("-Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.") val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting( name = "-Wunused", helpArg = "warning", diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 726cdc7131c4..8d16e5c7ceb2 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -199,6 +199,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case ClosureCannotHaveInternalParameterDependenciesID // errorNumber: 183 case MatchTypeNoCasesID // errorNumber: 184 case UnimportedAndImportedID // errorNumber: 185 + case ImplausiblePatternWarningID // erorNumber: 186 def errorNumber = ordinal - 1 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 970c44d54cbc..7cbce57d53ef 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2938,3 +2938,11 @@ class ClosureCannotHaveInternalParameterDependencies(mt: Type)(using Context) i"""cannot turn method type $mt into closure |because it has internal parameter dependencies""" def explain(using Context) = "" + +class ImplausiblePatternWarning(pat: tpd.Tree, selType: Type)(using Context) + extends TypeMsg(ImplausiblePatternWarningID): + def msg(using Context) = + i"""|Implausible pattern: + |$pat could match selector of type $selType + |only if there is an `equals` method identifying elements of the two types.""" + def explain(using Context) = "" \ No newline at end of file diff --git a/compiler/src/dotty/tools/dotc/typer/Linter.scala b/compiler/src/dotty/tools/dotc/typer/Linter.scala new file mode 100644 index 000000000000..882c036ad038 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/typer/Linter.scala @@ -0,0 +1,121 @@ +package dotty.tools +package dotc +package typer + +import core.* +import Types.*, Contexts.*, Symbols.*, Flags.*, Constants.* +import reporting.* +import Decorators.i + +object Linter: + import ast.tpd.* + + /** If -Wnonunit-statement is set, warn about statements in blocks that are non-unit expressions. + * @return true if a warning was issued, false otherwise + */ + private[typer] def warnOnInterestingResultInStatement(t: Tree)(using Context): Boolean = + + def isUninterestingSymbol(sym: Symbol): Boolean = + sym == NoSymbol || + sym.isConstructor || + sym.is(Package) || + sym.isPackageObject || + sym == defn.BoxedUnitClass || + sym == defn.AnyClass || + sym == defn.AnyRefAlias || + sym == defn.AnyValClass + + def isUninterestingType(tpe: Type): Boolean = + tpe == NoType || + tpe.typeSymbol == defn.UnitClass || + defn.isBottomClass(tpe.typeSymbol) || + tpe =:= defn.UnitType || + tpe.typeSymbol == defn.BoxedUnitClass || + tpe =:= defn.AnyValType || + tpe =:= defn.AnyType || + tpe =:= defn.AnyRefType + + def isJavaApplication(t: Tree): Boolean = t match + case Apply(f, _) => f.symbol.is(JavaDefined) && !defn.ObjectClass.isSubClass(f.symbol.owner) + case _ => false + + def checkInterestingShapes(t: Tree): Boolean = t match + case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart) + case Block(_, res) => checkInterestingShapes(res) + case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body)) + case _ => checksForInterestingResult(t) + + def checksForInterestingResult(t: Tree): Boolean = + !t.isDef // ignore defs + && !isUninterestingSymbol(t.symbol) // ctors, package, Unit, Any + && !isUninterestingType(t.tpe) // bottom types, Unit, Any + && !isThisTypeResult(t) // buf += x + && !isSuperConstrCall(t) // just a thing + && !isJavaApplication(t) // Java methods are inherently side-effecting + // && !treeInfo.hasExplicitUnit(t) // suppressed by explicit expr: Unit // TODO Should explicit `: Unit` be added as warning suppression? + + if ctx.settings.WNonUnitStatement.value && !ctx.isAfterTyper && checkInterestingShapes(t) then + val where = t match + case Block(_, res) => res + case If(_, thenpart, Literal(Constant(()))) => + thenpart match { + case Block(_, res) => res + case _ => thenpart + } + case _ => t + report.warning(UnusedNonUnitValue(where.tpe), t.srcPos) + true + else false + end warnOnInterestingResultInStatement + + def warnOnImplausiblePattern(pat: Tree, selType: Type)(using Context): Unit = + // approximate type params with bounds + def approx = new ApproximatingTypeMap { + var alreadyExpanding: List[TypeRef] = Nil + def apply(tp: Type) = tp.dealias match + case tp: TypeRef if !tp.symbol.isClass => + if alreadyExpanding contains tp then tp else + val saved = alreadyExpanding + alreadyExpanding ::= tp + val res = expandBounds(tp.info.bounds) + alreadyExpanding = saved + res + case _ => + mapOver(tp) + } + + // Is it possible that a value of `clsA` is equal to a value of `clsB`? + // This ignores user-defined equals methods, but makes an exception + // for numeric classes. + def canOverlap(clsA: ClassSymbol, clsB: ClassSymbol): Boolean = + clsA.mayHaveCommonChild(clsB) + || clsA.isNumericValueClass // this is quite coarse, but matches to what was done before + || clsB.isNumericValueClass + + // Can type `a` possiblly have a common instance with type `b`? + def canEqual(a: Type, b: Type): Boolean = trace(i"canEqual $a $b"): + b match + case _: TypeRef | _: AppliedType if b.typeSymbol.isClass => + a match + case a: TermRef if a.symbol.isOneOf(Module | Enum) => + (a frozen_<:< b) // fast track + || (a frozen_<:< approx(b)) + case _: TypeRef | _: AppliedType if a.typeSymbol.isClass => + if a.isNullType then !b.isNotNull + else canOverlap(a.typeSymbol.asClass, b.typeSymbol.asClass) + case a: TypeProxy => + canEqual(a.superType, b) + case a: AndOrType => + canEqual(a.tp1, b) || canEqual(a.tp2, b) + case b: TypeProxy => + canEqual(a, b.superType) + case b: AndOrType => + // we lose precision with and/or types, but it's hard to do better and + // still compute `canEqual(A & B, B & A) = true`. + canEqual(a, b.tp1) || canEqual(a, b.tp2) + + if ctx.settings.WimplausiblePatterns.value && !canEqual(pat.tpe, selType) then + report.warning(ImplausiblePatternWarning(pat, selType), pat.srcPos) + end warnOnImplausiblePattern + +end Linter diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index a38ddd534a2c..2952bfbba2af 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3303,7 +3303,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer traverse(xtree :: rest) case stat :: rest => val stat1 = typed(stat)(using ctx.exprContext(stat, exprOwner)) - if !checkInterestingResultInStatement(stat1) then checkStatementPurity(stat1)(stat, exprOwner) + if !Linter.warnOnInterestingResultInStatement(stat1) then checkStatementPurity(stat1)(stat, exprOwner) buf += stat1 traverse(rest)(using stat1.nullableContext) case nil => @@ -4361,123 +4361,18 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer tree match case _: RefTree | _: Literal if !isVarPattern(tree) && !(pt <:< tree.tpe) => - withMode(Mode.GadtConstraintInference) { + withMode(Mode.GadtConstraintInference): TypeComparer.constrainPatternType(tree.tpe, pt) - } - // approximate type params with bounds - def approx = new ApproximatingTypeMap { - var alreadyExpanding: List[TypeRef] = Nil - def apply(tp: Type) = tp.dealias match - case tp: TypeRef if !tp.symbol.isClass => - if alreadyExpanding contains tp then tp else - val saved = alreadyExpanding - alreadyExpanding ::= tp - val res = expandBounds(tp.info.bounds) - alreadyExpanding = saved - res - case _ => - mapOver(tp) - } + Linter.warnOnImplausiblePattern(tree, pt) - // Is it possible that a value of `clsA` is equal to a value of `clsB`? - // This ignores user-defined equals methods, but makes an exception - // for numeric classes. - def canOverlap(clsA: ClassSymbol, clsB: ClassSymbol): Boolean = - clsA.mayHaveCommonChild(clsB) - || clsA.isNumericValueClass // this is quite coarse, but matches to what was done before - || clsB.isNumericValueClass - - // Can type `a` possiblly have a common instance with type `b`? - def canEqual(a: Type, b: Type): Boolean = trace(i"canEqual $a $b"): - b match - case _: TypeRef | _: AppliedType if b.typeSymbol.isClass => - a match - case a: TermRef if a.symbol.isOneOf(Module | Enum) => - (a frozen_<:< b) // fast track - || (a frozen_<:< approx(b)) - case _: TypeRef | _: AppliedType if a.typeSymbol.isClass => - if a.isNullType then !b.isNotNull - else canOverlap(a.typeSymbol.asClass, b.typeSymbol.asClass) - case a: TypeProxy => - canEqual(a.superType, b) - case a: AndOrType => - canEqual(a.tp1, b) || canEqual(a.tp2, b) - case b: TypeProxy => - canEqual(a, b.superType) - case b: AndOrType => - // we lose precision with and/or types, but it's hard to do better and - // still compute `canEqual(A & B, B & A) = true`. - canEqual(a, b.tp1) || canEqual(a, b.tp2) - - if !canEqual(tree.tpe, pt) then - // We could check whether `equals` is overridden. - // Reasons for not doing so: - // - it complicates the protocol - // - such code patterns usually implies hidden errors in the code - // - it's safe/sound to reject the code - report.error(TypeMismatch(tree.tpe, pt, Some(tree), "\npattern type is incompatible with expected type"), tree.srcPos) - else - val cmp = - untpd.Apply( - untpd.Select(untpd.TypedSplice(tree), nme.EQ), - untpd.TypedSplice(dummyTreeOfType(pt))) - typedExpr(cmp, defn.BooleanType) + val cmp = + untpd.Apply( + untpd.Select(untpd.TypedSplice(tree), nme.EQ), + untpd.TypedSplice(dummyTreeOfType(pt))) + typedExpr(cmp, defn.BooleanType) case _ => - private def checkInterestingResultInStatement(t: Tree)(using Context): Boolean = { - def isUninterestingSymbol(sym: Symbol): Boolean = - sym == NoSymbol || - sym.isConstructor || - sym.is(Package) || - sym.isPackageObject || - sym == defn.BoxedUnitClass || - sym == defn.AnyClass || - sym == defn.AnyRefAlias || - sym == defn.AnyValClass - def isUninterestingType(tpe: Type): Boolean = - tpe == NoType || - tpe.typeSymbol == defn.UnitClass || - defn.isBottomClass(tpe.typeSymbol) || - tpe =:= defn.UnitType || - tpe.typeSymbol == defn.BoxedUnitClass || - tpe =:= defn.AnyValType || - tpe =:= defn.AnyType || - tpe =:= defn.AnyRefType - def isJavaApplication(t: Tree): Boolean = t match { - case Apply(f, _) => f.symbol.is(JavaDefined) && !defn.ObjectClass.isSubClass(f.symbol.owner) - case _ => false - } - def checkInterestingShapes(t: Tree): Boolean = t match { - case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart) - case Block(_, res) => checkInterestingShapes(res) - case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body)) - case _ => checksForInterestingResult(t) - } - def checksForInterestingResult(t: Tree): Boolean = ( - !t.isDef // ignore defs - && !isUninterestingSymbol(t.symbol) // ctors, package, Unit, Any - && !isUninterestingType(t.tpe) // bottom types, Unit, Any - && !isThisTypeResult(t) // buf += x - && !isSuperConstrCall(t) // just a thing - && !isJavaApplication(t) // Java methods are inherently side-effecting - // && !treeInfo.hasExplicitUnit(t) // suppressed by explicit expr: Unit // TODO Should explicit `: Unit` be added as warning suppression? - ) - if ctx.settings.WNonUnitStatement.value && !ctx.isAfterTyper && checkInterestingShapes(t) then - val where = t match { - case Block(_, res) => res - case If(_, thenpart, Literal(Constant(()))) => - thenpart match { - case Block(_, res) => res - case _ => thenpart - } - case _ => t - } - report.warning(UnusedNonUnitValue(where.tpe), t.srcPos) - true - else false - } - private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol)(using Context): Unit = if !tree.tpe.isErroneous && !ctx.isAfterTyper diff --git a/tests/neg/gadt-contradictory-pattern.scala b/tests/neg-custom-args/fatal-warnings/gadt-contradictory-pattern.scala similarity index 90% rename from tests/neg/gadt-contradictory-pattern.scala rename to tests/neg-custom-args/fatal-warnings/gadt-contradictory-pattern.scala index 561c0c23d518..e64a97fb38a6 100644 --- a/tests/neg/gadt-contradictory-pattern.scala +++ b/tests/neg-custom-args/fatal-warnings/gadt-contradictory-pattern.scala @@ -1,3 +1,4 @@ +// scalac: -Wimplausible-patterns object Test { sealed abstract class Foo[T] case object Bar1 extends Foo[Int] diff --git a/tests/neg/i5077.scala b/tests/neg-custom-args/fatal-warnings/i5077.scala similarity index 96% rename from tests/neg/i5077.scala rename to tests/neg-custom-args/fatal-warnings/i5077.scala index bcae42b6a5c5..6053d75cd813 100644 --- a/tests/neg/i5077.scala +++ b/tests/neg-custom-args/fatal-warnings/i5077.scala @@ -1,3 +1,4 @@ +// scalac: -Wimplausible-patterns trait Is[A] case object IsInt extends Is[Int] case object IsString extends Is[String] diff --git a/tests/neg/i9166.scala b/tests/neg-custom-args/fatal-warnings/i9166.scala similarity index 79% rename from tests/neg/i9166.scala rename to tests/neg-custom-args/fatal-warnings/i9166.scala index 7bbf2871eed3..eb03b6282d2f 100644 --- a/tests/neg/i9166.scala +++ b/tests/neg-custom-args/fatal-warnings/i9166.scala @@ -1,3 +1,4 @@ +// scalac: -Wimplausible-patterns object UnitTest extends App { def foo(m: Unit) = m match { case runtime.BoxedUnit.UNIT => println("ok") // error diff --git a/tests/neg-custom-args/fatal-warnings/i9740.check b/tests/neg-custom-args/fatal-warnings/i9740.check new file mode 100644 index 000000000000..446fb574d33a --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i9740.check @@ -0,0 +1,12 @@ +-- [E186] Type Error: tests/neg-custom-args/fatal-warnings/i9740.scala:10:9 -------------------------------------------- +10 | case RecoveryCompleted => println("Recovery completed") // error + | ^^^^^^^^^^^^^^^^^ + | Implausible pattern: + | RecoveryCompleted could match selector of type object TypedRecoveryCompleted + | only if there is an `equals` method identifying elements of the two types. +-- [E186] Type Error: tests/neg-custom-args/fatal-warnings/i9740.scala:15:9 -------------------------------------------- +15 | case RecoveryCompleted => // error + | ^^^^^^^^^^^^^^^^^ + | Implausible pattern: + | RecoveryCompleted could match selector of type TypedRecoveryCompleted + | only if there is an `equals` method identifying elements of the two types. diff --git a/tests/neg/i9740.scala b/tests/neg-custom-args/fatal-warnings/i9740.scala similarity index 93% rename from tests/neg/i9740.scala rename to tests/neg-custom-args/fatal-warnings/i9740.scala index 2f342977ef5d..97b46778e455 100644 --- a/tests/neg/i9740.scala +++ b/tests/neg-custom-args/fatal-warnings/i9740.scala @@ -1,3 +1,4 @@ +// scalac: -Wimplausible-patterns abstract class RecoveryCompleted object RecoveryCompleted extends RecoveryCompleted diff --git a/tests/neg/i9740b.scala b/tests/neg-custom-args/fatal-warnings/i9740b.scala similarity index 93% rename from tests/neg/i9740b.scala rename to tests/neg-custom-args/fatal-warnings/i9740b.scala index 8006056684c7..f2b8a554f2bf 100644 --- a/tests/neg/i9740b.scala +++ b/tests/neg-custom-args/fatal-warnings/i9740b.scala @@ -1,3 +1,4 @@ +// scalac: -Wimplausible-patterns enum Recovery: case RecoveryCompleted diff --git a/tests/neg/i9740c.scala b/tests/neg-custom-args/fatal-warnings/i9740c.scala similarity index 91% rename from tests/neg/i9740c.scala rename to tests/neg-custom-args/fatal-warnings/i9740c.scala index 87881c9b20d7..956a5cb3a72c 100644 --- a/tests/neg/i9740c.scala +++ b/tests/neg-custom-args/fatal-warnings/i9740c.scala @@ -1,3 +1,4 @@ +// scalac: -Wimplausible-patterns sealed trait Exp[T] case class IntExp(x: Int) extends Exp[Int] case class StrExp(x: String) extends Exp[String] diff --git a/tests/neg/i9740d.scala b/tests/neg-custom-args/fatal-warnings/i9740d.scala similarity index 89% rename from tests/neg/i9740d.scala rename to tests/neg-custom-args/fatal-warnings/i9740d.scala index 9f2490b697b6..64a330714cb5 100644 --- a/tests/neg/i9740d.scala +++ b/tests/neg-custom-args/fatal-warnings/i9740d.scala @@ -1,3 +1,5 @@ +// scalac: -Wimplausible-patterns + sealed trait Exp[T] case class IntExp(x: Int) extends Exp[Int] case class StrExp(x: String) extends Exp[String] diff --git a/tests/neg/i5004.scala b/tests/neg/i5004.scala index 02105104efd1..97224d930bbf 100644 --- a/tests/neg/i5004.scala +++ b/tests/neg/i5004.scala @@ -1,6 +1,6 @@ object i0 { 1 match { def this(): Int // error - def this() -} // error + def this() // error +} } From a70ac546ba048115cc1f63579531aa4f81ac80d6 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 5 Jul 2023 17:40:11 +0200 Subject: [PATCH 4/4] Cleanups --- compiler/src/dotty/tools/dotc/typer/Linter.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Linter.scala b/compiler/src/dotty/tools/dotc/typer/Linter.scala index 882c036ad038..c0ba581b3732 100644 --- a/compiler/src/dotty/tools/dotc/typer/Linter.scala +++ b/compiler/src/dotty/tools/dotc/typer/Linter.scala @@ -7,13 +7,14 @@ import Types.*, Contexts.*, Symbols.*, Flags.*, Constants.* import reporting.* import Decorators.i +/** A module for linter checks done at typer */ object Linter: import ast.tpd.* /** If -Wnonunit-statement is set, warn about statements in blocks that are non-unit expressions. * @return true if a warning was issued, false otherwise */ - private[typer] def warnOnInterestingResultInStatement(t: Tree)(using Context): Boolean = + def warnOnInterestingResultInStatement(t: Tree)(using Context): Boolean = def isUninterestingSymbol(sym: Symbol): Boolean = sym == NoSymbol || @@ -68,6 +69,10 @@ object Linter: else false end warnOnInterestingResultInStatement + /** If -Wimplausible-patterns is set, warn about pattern values that can match the scrutinee + * type only if there would be some user-defined equality method that equates values of the + * two types. + */ def warnOnImplausiblePattern(pat: Tree, selType: Type)(using Context): Unit = // approximate type params with bounds def approx = new ApproximatingTypeMap {