From effb9d1313204aa2e98dfe1cbf8329c8f6ca8ef2 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 19 Mar 2022 17:03:06 +0100 Subject: [PATCH 1/4] Don't optimize explicitly written isInstanceOf tests away. Do it only for tests generated during pattern matching. Fixes #14707. See the issue for a rationale of the change. --- .../tools/dotc/transform/TypeTestsCasts.scala | 7 ++----- tests/run/i14705.scala | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 tests/run/i14705.scala diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 0b880afdd995..b245bd6e7559 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -243,11 +243,8 @@ object TypeTestsCasts { else foundClasses.exists(check) end checkSensical - if (expr.tpe <:< testType) - if (expr.tpe.isNotNull) { - if (!inMatch) report.warning(TypeTestAlwaysSucceeds(expr.tpe, testType), tree.srcPos) - constant(expr, Literal(Constant(true))) - } + if (expr.tpe <:< testType) && inMatch then + if expr.tpe.isNotNull then constant(expr, Literal(Constant(true))) else expr.testNotNull else { val nestedCtx = ctx.fresh.setNewTyperState() diff --git a/tests/run/i14705.scala b/tests/run/i14705.scala new file mode 100644 index 000000000000..1ee4f6c7752b --- /dev/null +++ b/tests/run/i14705.scala @@ -0,0 +1,19 @@ +trait Fruit +case class Apple() extends Fruit +case class Orange() extends Fruit + +case class Box[C](fruit: C) extends Fruit + +val apple = Box(fruit = Apple()) +val orange = Box(fruit = Orange()) + + +val result = List(apple, orange).map { + case appleBox: Box[Apple] @unchecked if appleBox.fruit.isInstanceOf[Apple] => //contains apple + "apple" + case _ => + "orange" +} + +@main def Test = + assert(result == List("apple", "orange")) From 1c42de7c617bde720be0146c333d1de58d206021 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 19 Mar 2022 17:19:15 +0100 Subject: [PATCH 2/4] Fix erroneous isInstanceOf test in compiler --- .../dotty/tools/dotc/reporting/ErrorMessageID.scala | 2 +- compiler/src/dotty/tools/dotc/reporting/messages.scala | 10 +++------- .../src/dotty/tools/dotc/transform/TreeChecker.scala | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index f55196f82a8e..2f1fe0e1c058 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -131,7 +131,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID]: DoubleDefinitionID, MatchCaseOnlyNullWarningID, ImportRenamedTwiceID, - TypeTestAlwaysSucceedsID, + TypeTestAlwaysDivergesID, TermMemberNeedsNeedsResultTypeForImplicitSearchID, ClassCannotExtendEnumID, ValueClassParameterMayNotBeCallByNameID, diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 0262e0190677..32773ba7a041 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2177,13 +2177,9 @@ import transform.SymUtils._ def explain = "" } - class TypeTestAlwaysSucceeds(scrutTp: Type, testTp: Type)(using Context) extends SyntaxMsg(TypeTestAlwaysSucceedsID) { - def msg = { - val addendum = - if (scrutTp != testTp) s" is a subtype of ${testTp.show}" - else " is the same as the tested type" - s"The highlighted type test will always succeed since the scrutinee type ${scrutTp.show}" + addendum - } + class TypeTestAlwaysDiverges(scrutTp: Type, testTp: Type)(using Context) extends SyntaxMsg(TypeTestAlwaysDivergesID) { + def msg = + s"This type test will never return a result since the scrutinee type ${scrutTp.show} does not contain any value." def explain = "" } diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 841c0fc15cc4..bd3f4f44984b 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -418,7 +418,7 @@ class TreeChecker extends Phase with SymTransformer { } override def typedSuper(tree: untpd.Super, pt: Type)(using Context): Tree = - assert(tree.qual.tpe.isInstanceOf[ThisType], i"expect prefix of Super to be This, actual = ${tree.qual}") + assert(tree.qual.typeOpt.isInstanceOf[ThisType], i"expect prefix of Super to be This, actual = ${tree.qual}") super.typedSuper(tree, pt) override def typedTyped(tree: untpd.Typed, pt: Type)(using Context): Tree = From e501845e77456c20ecde6438a276d22135d723e8 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 20 Mar 2022 14:17:03 +0100 Subject: [PATCH 3/4] Warn about nonsensical isInstanceOf tests with `Nothing` as the scrutinee type --- compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala | 2 ++ tests/neg-custom-args/fatal-warnings/i14705.scala | 3 +++ 2 files changed, 5 insertions(+) create mode 100644 tests/neg-custom-args/fatal-warnings/i14705.scala diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index b245bd6e7559..d0a293c6809c 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -247,6 +247,8 @@ object TypeTestsCasts { if expr.tpe.isNotNull then constant(expr, Literal(Constant(true))) else expr.testNotNull else { + if expr.tpe.isBottomType then + report.warning(TypeTestAlwaysDiverges(expr.tpe, testType), tree.srcPos) val nestedCtx = ctx.fresh.setNewTyperState() val foundClsSyms = foundClasses(expr.tpe.widen, Nil) val sensical = checkSensical(foundClsSyms)(using nestedCtx) diff --git a/tests/neg-custom-args/fatal-warnings/i14705.scala b/tests/neg-custom-args/fatal-warnings/i14705.scala new file mode 100644 index 000000000000..f5c17baae609 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i14705.scala @@ -0,0 +1,3 @@ +val n = Nil +val b = n.head.isInstanceOf[String] // error + From 9648c631e2d6c3cef6fdef71bf7537b737b29384 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 20 Mar 2022 14:17:39 +0100 Subject: [PATCH 4/4] Fix wrong treatment of Super nodes in TailRec --- .../dotty/tools/dotc/transform/TailRec.scala | 31 ++++++++++++------- tests/pos/tailrec-super.scala | 17 ++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) create mode 100644 tests/pos/tailrec-super.scala diff --git a/compiler/src/dotty/tools/dotc/transform/TailRec.scala b/compiler/src/dotty/tools/dotc/transform/TailRec.scala index cb1119ea71f1..916b9279da09 100644 --- a/compiler/src/dotty/tools/dotc/transform/TailRec.scala +++ b/compiler/src/dotty/tools/dotc/transform/TailRec.scala @@ -3,18 +3,16 @@ package transform import ast.{TreeTypeMap, tpd} import config.Printers.tailrec -import core.Contexts._ -import core.Constants.Constant -import core.Flags._ -import core.NameKinds.{TailLabelName, TailLocalName, TailTempName} -import core.StdNames.nme -import core.Symbols._ -import reporting._ +import core.* +import Contexts.*, Flags.*, Symbols.* +import Constants.Constant +import NameKinds.{TailLabelName, TailLocalName, TailTempName} +import StdNames.nme +import reporting.* import transform.MegaPhase.MiniPhase import util.LinearSet import dotty.tools.uncheckedNN - /** A Tail Rec Transformer. * * What it does: @@ -161,15 +159,26 @@ class TailRec extends MiniPhase { val rhsFullyTransformed = varForRewrittenThis match { case Some(localThisSym) => val thisRef = localThisSym.termRef - new TreeTypeMap( + val substitute = new TreeTypeMap( typeMap = _.substThisUnlessStatic(enclosingClass, thisRef) .subst(rewrittenParamSyms, varsForRewrittenParamSyms.map(_.termRef)), treeMap = { case tree: This if tree.symbol == enclosingClass => Ident(thisRef) case tree => tree } - ).transform(rhsSemiTransformed) - + ) + // The previous map will map `This` references to `Ident`s even under `Super`. + // This violates super's contract. We fix this by cleaning up `Ident`s under + // super, mapping them back to the original `This` reference. This is not + // very elegant, but I did not manage to find a cleaner way to handle this. + // See pos/tailrec-super.scala for a test case. + val cleanup = new TreeMap: + override def transform(t: Tree)(using Context) = t match + case Super(qual: Ident, mix) if !qual.tpe.isInstanceOf[Types.ThisType] => + cpy.Super(t)(This(enclosingClass), mix) + case _ => + super.transform(t) + cleanup.transform(substitute.transform(rhsSemiTransformed)) case none => new TreeTypeMap( typeMap = _.subst(rewrittenParamSyms, varsForRewrittenParamSyms.map(_.termRef)) diff --git a/tests/pos/tailrec-super.scala b/tests/pos/tailrec-super.scala new file mode 100644 index 000000000000..2f7991670010 --- /dev/null +++ b/tests/pos/tailrec-super.scala @@ -0,0 +1,17 @@ +class Tree +case class Inlined(call: Tree, bindings: List[String], expr: Tree) extends Tree +case object EmptyTree extends Tree +class Context + +class Transform: + def transform(tree: Tree)(using Context): Tree = tree + +class Inliner: + var enclosingInlineds: List[String] = Nil + private def expandMacro(using Context) = + val inlinedNormalizer = new Transform: + override def transform(tree: Tree)(using Context) = tree match + case Inlined(EmptyTree, Nil, expr) if enclosingInlineds.isEmpty => transform(expr) + case _ => super.transform(tree) + +object Inliner \ No newline at end of file