Merge isInstanceOfEvaluator with TypeTestCasts

As the saying goes: "let erasure handle it... "

The reason for the change is that things were a mess before

 - isInstanceOfEvaluator did not handle &/| types. It would
   be hard to change that since it was looking at erased types.
 - isInstanceEvaluator, but not TypeTestCasts added null-checks
 - therefore, necessary null-checks were not emitted on some tests of
   the form `x.isInstanceOf[A & B]`.
 - the pattern matcher patched this up by inserting some null checks
   on its own, whereas it should just have let the translation of
   isInstanceOf do the job.

Author: odersky

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -70,7 +70,6 @@ class Compiler {
            new CrossCastAnd,        // Normalize selections involving intersection types.
            new Splitter),           // Expand selections involving union types into conditionals
       List(new VCInlineMethods,     // Inlines calls to value class methods
-           new IsInstanceOfEvaluator, // Issues warnings when unreachable statements are present in match/if expressions
            new SeqLiterals,         // Express vararg arguments as arrays
            new InterceptedMethods,  // Special handling of `==`, `|=`, `getClass` methods
            new Getters,             // Replace non-private vals and vars with getter defs (fields are added later)

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -601,7 +601,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
     override def TypeApply(tree: Tree)(fun: Tree, args: List[Tree])(implicit ctx: Context): TypeApply =
       ta.assignType(untpd.cpy.TypeApply(tree)(fun, args), fun, args)
       // Same remark as for Apply
-    
+
     override def Closure(tree: Tree)(env: List[Tree], meth: Tree, tpt: Tree)(implicit ctx: Context): Closure =
             ta.assignType(untpd.cpy.Closure(tree)(env, meth, tpt), meth, tpt)
 
@@ -770,6 +770,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
       else if (!ctx.erasedTypes) asInstance(tp)
       else Erasure.Boxing.adaptToType(tree, tp)
 
+    /** `tree ne null` (might need a cast to be type correct) */
+     def testNotNull(implicit ctx: Context): Tree =
+      tree.ensureConforms(defn.ObjectType)
+        .select(defn.Object_ne).appliedTo(Literal(Constant(null)))
+
     /** If inititializer tree is `_', the default value of its type,
      *  otherwise the tree itself.
      */

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -251,6 +251,8 @@ class Definitions {
     lazy val Any_getClass = enterMethod(AnyClass, nme.getClass_, MethodType(Nil, ClassClass.typeRef.appliedTo(TypeBounds.empty)), Final)
     lazy val Any_isInstanceOf = enterT1ParameterlessMethod(AnyClass, nme.isInstanceOf_, _ => BooleanType, Final)
     lazy val Any_asInstanceOf = enterT1ParameterlessMethod(AnyClass, nme.asInstanceOf_, TypeParamRef(_, 0), Final)
+    lazy val Any_typeTest = enterT1ParameterlessMethod(AnyClass, nme.isInstanceOfPM, _ => BooleanType, Final)
+      // generated by pattern matcher, eliminated by erasure
 
     def AnyMethods = List(Any_==, Any_!=, Any_equals, Any_hashCode,
       Any_toString, Any_##, Any_getClass, Any_isInstanceOf, Any_asInstanceOf)

compiler/src/dotty/tools/dotc/core/StdNames.scala

@@ -438,6 +438,7 @@ object StdNames {
     val isDefined: N            = "isDefined"
     val isEmpty: N              = "isEmpty"
     val isInstanceOf_ : N       = "isInstanceOf"
+    val isInstanceOfPM: N       = "$isInstanceOf$"
     val java: N                 = "java"
     val key: N                  = "key"
     val lang: N                 = "lang"

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

@@ -172,7 +172,8 @@ object Erasure extends TypeTestsCasts{
     }
 
     def constant(tree: Tree, const: Tree)(implicit ctx: Context) =
-      if (isPureExpr(tree)) const else Block(tree :: Nil, const)
+      (if (isPureExpr(tree)) const else Block(tree :: Nil, const))
+        .withPos(tree.pos)
 
     final def box(tree: Tree, target: => String = "")(implicit ctx: Context): Tree = ctx.traceIndented(i"boxing ${tree.showSummary}: ${tree.tpe} into $target") {
       tree.tpe.widen match {

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

@@ -1,16 +1,15 @@
 package dotty.tools.dotc
 package transform
 
-import core.Contexts._
-import core.Symbols._
-import core.Types._
-import core.Constants._
-import core.StdNames._
-import core.TypeErasure.isUnboundedGeneric
+import core._
+import Contexts._, Symbols._, Types._, Constants._, StdNames._, Decorators._
 import ast.Trees._
 import Erasure.Boxing._
-import core.TypeErasure._
+import TypeErasure._
 import ValueClasses._
+import core.Flags._
+import util.Positions._
+
 
 /** This transform normalizes type tests and type casts,
  *  also replacing type tests with singleton argument type with reference equality check
@@ -26,8 +25,6 @@ import ValueClasses._
 trait TypeTestsCasts {
   import ast.tpd._
 
-  // override def phaseName: String = "typeTestsCasts"
-
   def interceptTypeApply(tree: TypeApply)(implicit ctx: Context): Tree = ctx.traceIndented(s"transforming ${tree.show}", show = true) {
     tree.fun match {
       case fun @ Select(qual, selector) =>
@@ -38,88 +35,122 @@ trait TypeTestsCasts {
         def derivedTree(qual1: Tree, sym: Symbol, tp: Type) =
           cpy.TypeApply(tree)(qual1.select(sym).withPos(qual.pos), List(TypeTree(tp)))
 
-        def qualCls = qual.tpe.widen.classSymbol
+        def foundCls = qual.tpe.widen.classSymbol
+        // println(i"ta $tree, found = $foundCls")
 
-        def transformIsInstanceOf(expr:Tree, argType: Type): Tree = {
-          def argCls = argType.classSymbol
-          if ((expr.tpe <:< argType) && isPureExpr(expr))
-            Literal(Constant(true)) withPos tree.pos
-          else if (argCls.isPrimitiveValueClass)
-            if (qualCls.isPrimitiveValueClass) Literal(Constant(qualCls == argCls)) withPos tree.pos
-            else transformIsInstanceOf(expr, defn.boxedType(argCls.typeRef))
-          else argType.dealias match {
-            case _: SingletonType =>
-              val cmpOp = if (argType derivesFrom defn.AnyValClass) defn.Any_equals else defn.Object_eq
-              expr.select(cmpOp).appliedTo(singleton(argType))
-            case AndType(tp1, tp2) =>
-              evalOnce(expr) { fun =>
-                val erased1 = transformIsInstanceOf(fun, tp1)
-                val erased2 = transformIsInstanceOf(fun, tp2)
-                erased1 match {
-                  case Literal(Constant(true)) => erased2
-                  case _ =>
-                    erased2 match {
-                      case Literal(Constant(true)) => erased1
-                      case _ => erased1 and erased2
-                    }
-                }
+        def inMatch =
+          fun.symbol == defn.Any_typeTest ||  // new scheme
+          qual.symbol.is(Case)                // old scheme
+
+        def transformIsInstanceOf(expr:Tree, testType: Type, flagUnrelated: Boolean): Tree = {
+          def testCls = testType.classSymbol
+
+          def unreachable(why: => String) =
+            if (flagUnrelated)
+              if (inMatch) ctx.error(em"this case is unreachable since $why", expr.pos)
+              else ctx.warning(em"this will always yield false since $why", expr.pos)
+
+          def checkSensical: Boolean = {
+            val foundCls = erasure(qual.tpe.widen).typeSymbol
+            val testCls = testType.typeSymbol
+            !foundCls.isClass ||
+            !testCls.isClass ||
+            testCls.isPrimitiveValueClass ||
+            ValueClasses.isDerivedValueClass(foundCls) ||
+            ValueClasses.isDerivedValueClass(testCls) ||
+            foundCls == defn.ObjectClass ||
+            foundCls.derivesFrom(testCls) || {
+              if (foundCls.is(Final)) {
+                unreachable(i"$foundCls is not a subclass of $testCls")
+                false
               }
-            case defn.MultiArrayOf(elem, ndims) if isUnboundedGeneric(elem) =>
-              def isArrayTest(arg: Tree) =
-                ref(defn.runtimeMethodRef(nme.isArray)).appliedTo(arg, Literal(Constant(ndims)))
-              if (ndims == 1) isArrayTest(qual)
-              else evalOnce(qual) { qual1 =>
-                derivedTree(qual1, defn.Any_isInstanceOf, qual1.tpe) and isArrayTest(qual1)
+              else if (!testCls.derivesFrom(foundCls) &&
+                       (testCls.is(Final) || !testCls.is(Trait) && !foundCls.is(Trait))) {
+                unreachable(i"$foundCls and $testCls are unrelated")
+                false
               }
+              else true
+            }
+          }
+          println(i"test ii $tree ${expr.tpe} $testType")
+
+          if (expr.tpe <:< testType)
+            if (expr.tpe.isNotNull) {
+              ctx.warning(
+                em"this will always yield true, since `$foundCls` is a subclass of `$testCls`",
+                expr.pos)
+              constant(expr, Literal(Constant(true)))
+            }
+            else expr.testNotNull
+          else if (!checkSensical)
+            constant(expr, Literal(Constant(false)))
+          else if (testCls.isPrimitiveValueClass)
+            if (foundCls.isPrimitiveValueClass)
+              constant(expr, Literal(Constant(foundCls == testCls)))
+            else
+              transformIsInstanceOf(expr, defn.boxedType(testCls.typeRef), flagUnrelated)
+          else testType.dealias match {
+            case _: SingletonType =>
+              val cmpOp =
+                if (testType derivesFrom defn.AnyValClass) defn.Any_equals
+                else defn.Object_eq
+              expr.select(cmpOp).appliedTo(singleton(testType))
             case _ =>
-              derivedTree(expr, defn.Any_isInstanceOf, argType)
+              derivedTree(expr, defn.Any_isInstanceOf, testType)
           }
         }
 
-        def transformAsInstanceOf(argType: Type): Tree = {
-          def argCls = argType.widen.classSymbol
-          if (qual.tpe <:< argType)
+        def transformAsInstanceOf(testType: Type): Tree = {
+          def testCls = testType.widen.classSymbol
+          if (qual.tpe <:< testType)
             Typed(qual, tree.args.head)
-          else if (qualCls.isPrimitiveValueClass) {
-            if (argCls.isPrimitiveValueClass) primitiveConversion(qual, argCls)
-            else derivedTree(box(qual), defn.Any_asInstanceOf, argType)
+          else if (foundCls.isPrimitiveValueClass) {
+            if (testCls.isPrimitiveValueClass) primitiveConversion(qual, testCls)
+            else derivedTree(box(qual), defn.Any_asInstanceOf, testType)
           }
-          else if (argCls.isPrimitiveValueClass)
-            unbox(qual.ensureConforms(defn.ObjectType), argType)
-          else if (isDerivedValueClass(argCls)) {
+          else if (testCls.isPrimitiveValueClass)
+            unbox(qual.ensureConforms(defn.ObjectType), testType)
+          else if (isDerivedValueClass(testCls)) {
             qual // adaptToType in Erasure will do the necessary type adaptation
           }
           else
-            derivedTree(qual, defn.Any_asInstanceOf, argType)
+            derivedTree(qual, defn.Any_asInstanceOf, testType)
         }
 
         /** Transform isInstanceOf OrType
          *
          *    expr.isInstanceOf[A | B]  ~~>  expr.isInstanceOf[A] | expr.isInstanceOf[B]
          *    expr.isInstanceOf[A & B]  ~~>  expr.isInstanceOf[A] & expr.isInstanceOf[B]
          *
-         *  The transform happens before erasure of `argType`, thus cannot be merged
-         *  with `transformIsInstanceOf`, which depends on erased type of `argType`.
+         *  The transform happens before erasure of `testType`, thus cannot be merged
+         *  with `transformIsInstanceOf`, which depends on erased type of `testType`.
          */
-        def transformTypeTest(qual: Tree, argType: Type): Tree = argType.dealias match {
+        def transformTypeTest(qual: Tree, testType: Type, flagUnrelated: Boolean): Tree = testType.dealias match {
           case OrType(tp1, tp2) =>
             evalOnce(qual) { fun =>
-              transformTypeTest(fun, tp1)
+              transformTypeTest(fun, tp1, flagUnrelated = false)
                 .select(defn.Boolean_||)
-                .appliedTo(transformTypeTest(fun, tp2))
+                .appliedTo(transformTypeTest(fun, tp2, flagUnrelated = false))
             }
           case AndType(tp1, tp2) =>
             evalOnce(qual) { fun =>
-              transformTypeTest(fun, tp1)
+              transformTypeTest(fun, tp1, flagUnrelated)
                 .select(defn.Boolean_&&)
-                .appliedTo(transformTypeTest(fun, tp2))
+                .appliedTo(transformTypeTest(fun, tp2, flagUnrelated))
+            }
+          case defn.MultiArrayOf(elem, ndims) if isUnboundedGeneric(elem) =>
+            def isArrayTest(arg: Tree) =
+              ref(defn.runtimeMethodRef(nme.isArray)).appliedTo(arg, Literal(Constant(ndims)))
+            if (ndims == 1) isArrayTest(qual)
+            else evalOnce(qual) { qual1 =>
+              derivedTree(qual1, defn.Any_isInstanceOf, qual1.tpe) and isArrayTest(qual1)
             }
           case _ =>
-            transformIsInstanceOf(qual, erasure(argType))
+            transformIsInstanceOf(qual, erasure(testType), flagUnrelated)
         }
 
-        if (sym eq defn.Any_isInstanceOf)
-          transformTypeTest(qual, tree.args.head.tpe)
+        if ((sym eq defn.Any_isInstanceOf) || (sym eq defn.Any_typeTest))
+          transformTypeTest(qual, tree.args.head.tpe, flagUnrelated = true)
         else if (sym eq defn.Any_asInstanceOf)
           transformAsInstanceOf(erasure(tree.args.head.tpe))
         else tree

tests/neg/tryPatternMatchError.scala

@@ -25,7 +25,7 @@ object Test {
       case _: ExceptionTrait =>
       case _: NoSuchElementException if a <= 1 =>
       case _: NullPointerException | _:IOException =>
-      case e: Int => // error
+      case e: Int => // error: unrelated
       case EX =>
       case IAE(msg) =>
       case e: IllegalArgumentException =>