Merge pull request #14715 from dotty-staging/fix-14705-v2

Don't optimize explicitly written isInstanceOf tests away.

Author: odersky

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,

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 = ""
   }
 

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))

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 =

compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala

@@ -243,13 +243,12 @@ 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 {
+            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)

tests/neg-custom-args/fatal-warnings/i14705.scala

@@ -0,0 +1,3 @@
+val n = Nil
+val b = n.head.isInstanceOf[String] // error
+

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

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"))