Merge pull request #4045 from dotty-staging/fix-3324

Fix #3324: add `isInstanceOf` check

Author: liufengyun

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

@@ -81,6 +81,7 @@ class Compiler {
          new CrossCastAnd,           // Normalize selections involving intersection types.
          new Splitter) ::            // Expand selections involving union types into conditionals
     List(new ErasedDecls,            // Removes all erased defs and vals decls (except for parameters)
+         new IsInstanceOfChecker,    // check runtime realisability for `isInstanceOf`
          new VCInlineMethods,        // Inlines calls to value class methods
          new SeqLiterals,            // Express vararg arguments as arrays
          new InterceptedMethods,     // Special handling of `==`, `|=`, `getClass` methods

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

@@ -805,7 +805,7 @@ object Trees {
     def unforced: AnyRef
     protected def force(x: AnyRef): Unit
     def forceIfLazy(implicit ctx: Context): T = unforced match {
-      case lzy: Lazy[T] =>
+      case lzy: Lazy[T @unchecked] =>
         val x = lzy.complete
         force(x)
         x

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

@@ -0,0 +1,133 @@
+package dotty.tools.dotc
+package transform
+
+import util.Positions._
+import MegaPhase.MiniPhase
+import core._
+import Contexts.Context, Types._, Decorators._, Symbols._, typer._, ast._, NameKinds._
+import TypeUtils._, Flags._
+import config.Printers.{ transforms => debug }
+
+/** Check runtime realizability of type test, see the documentation for `Checkable`.
+ */
+class IsInstanceOfChecker extends MiniPhase {
+
+  import ast.tpd._
+
+  val phaseName = "isInstanceOfChecker"
+
+  override def transformTypeApply(tree: TypeApply)(implicit ctx: Context): Tree = {
+    def ensureCheckable(qual: Tree, pt: Tree): Tree = {
+      if (!Checkable.checkable(qual.tpe, pt.tpe, tree.pos))
+         ctx.warning(
+           s"the type test for ${pt.show} cannot be checked at runtime",
+           tree.pos
+         )
+
+      tree
+    }
+
+    tree.fun match {
+      case fn: Select
+      if fn.symbol == defn.Any_typeTest || fn.symbol == defn.Any_isInstanceOf =>
+        ensureCheckable(fn.qualifier, tree.args.head)
+      case _ => tree
+    }
+  }
+}
+
+object Checkable {
+  import Inferencing._
+  import ProtoTypes._
+
+  /** Whether `(x:X).isInstanceOf[P]` can be checked at runtime?
+   *
+   *  First do the following substitution:
+   *  (a) replace `T @unchecked` and pattern binder types (e.g., `_$1`) in P with WildcardType
+   *  (b) replace pattern binder types (e.g., `_$1`) in X:
+   *      - variance = 1  : hiBound
+   *      - variance = -1 : loBound
+   *      - variance = 0  : OrType(Any, Nothing) // TODO: use original type param bounds
+   *
+   *  Then check:
+   *
+   *  1. if `X <:< P`, TRUE
+   *  2. if `P` is a singleton type, TRUE
+   *  3. if `P` refers to an abstract type member or type parameter, FALSE
+   *  4. if `P = Array[T]`, checkable(E, T) where `E` is the element type of `X`, defaults to `Any`.
+   *  5. if `P` is `pre.F[Ts]` and `pre.F` refers to a class which is not `Array`:
+   *     (a) replace `Ts` with fresh type variables `Xs`
+   *     (b) constrain `Xs` with `pre.F[Xs] <:< X`
+   *     (c) instantiate Xs and check `pre.F[Xs] <:< P`
+   *  6. if `P = T1 | T2` or `P = T1 & T2`, checkable(X, T1) && checkable(X, T2).
+   *  7. if `P` is a refinement type, FALSE
+   *  8. otherwise, TRUE
+   */
+  def checkable(X: Type, P: Type, pos: Position)(implicit ctx: Context): Boolean = {
+    def isAbstract(P: Type) = !P.dealias.typeSymbol.isClass
+    def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case)
+
+    def replaceP(implicit ctx: Context) = new TypeMap {
+      def apply(tp: Type) = tp match {
+        case tref: TypeRef
+        if isPatternTypeSymbol(tref.typeSymbol) => WildcardType
+        case AnnotatedType(_, annot)
+        if annot.symbol == defn.UncheckedAnnot => WildcardType
+        case _ => mapOver(tp)
+      }
+    }
+
+    def replaceX(implicit ctx: Context) = new TypeMap {
+      def apply(tp: Type) = tp match {
+        case tref: TypeRef
+        if isPatternTypeSymbol(tref.typeSymbol) =>
+          if (variance == 1) tref.info.hiBound
+          else if (variance == -1) tref.info.loBound
+          else OrType(defn.AnyType, defn.NothingType)
+        case _ => mapOver(tp)
+      }
+    }
+
+    def isClassDetermined(X: Type, P: AppliedType)(implicit ctx: Context) = {
+      val AppliedType(tycon, _) = P
+      val typeLambda = tycon.ensureLambdaSub.asInstanceOf[TypeLambda]
+      val tvars = constrained(typeLambda, untpd.EmptyTree, alwaysAddTypeVars = true)._2.map(_.tpe)
+      val P1 = tycon.appliedTo(tvars)
+
+      debug.println("P : " + P.show)
+      debug.println("P1 : " + P1.show)
+      debug.println("X : " + X.show)
+
+      P1 <:< X  // may fail, ignore
+
+      val res = isFullyDefined(P1, ForceDegree.noBottom) &&  P1 <:< P
+      debug.println("P1 : " + P1)
+      debug.println("P1 <:< P = " + res)
+
+      res
+    }
+
+    def recur(X: Type, P: Type): Boolean = (X <:< P) || (P match {
+      case _: SingletonType     => true
+      case _: TypeProxy
+      if isAbstract(P)          => false
+      case defn.ArrayOf(tpT)    =>
+        X match {
+          case defn.ArrayOf(tpE)   => recur(tpE, tpT)
+          case _                   => recur(defn.AnyType, tpT)
+        }
+      case tpe: AppliedType     => isClassDetermined(X, tpe)(ctx.fresh.setNewTyperState())
+      case AndType(tp1, tp2)    => recur(X, tp1) && recur(X, tp2)
+      case OrType(tp1, tp2)     => recur(X, tp1) && recur(X, tp2)
+      case AnnotatedType(t, _)  => recur(X, t)
+      case _: RefinedType       => false
+      case _                    => true
+    })
+
+    val res = recur(replaceX.apply(X.widen), replaceP.apply(P))
+
+    debug.println(i"checking  ${X.show} isInstanceOf ${P} = $res")
+
+    res
+  }
+}

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

@@ -11,6 +11,7 @@ import ast.Trees._
 import ast.untpd
 import Decorators._
 import NameOps._
+import Annotations.Annotation
 import ValueClasses.isDerivedValueClass
 import scala.collection.mutable.ListBuffer
 import scala.language.postfixOps
@@ -152,17 +153,20 @@ class SyntheticMethods(thisPhase: DenotTransformer) {
      *  def equals(that: Any): Boolean =
      *    (this eq that) || {
      *      that match {
-     *        case x$0 @ (_: C) => this.x == this$0.x && this.y == x$0.y
+     *        case x$0 @ (_: C @unchecked) => this.x == this$0.x && this.y == x$0.y
      *        case _ => false
      *     }
      *  ```
      *
      *  If `C` is a value class the initial `eq` test is omitted.
+     *
+     *  `@unchecked` is needed for parametric case classes.
+     *
      */
     def equalsBody(that: Tree)(implicit ctx: Context): Tree = {
       val thatAsClazz = ctx.newSymbol(ctx.owner, nme.x_0, Synthetic, clazzType, coord = ctx.owner.pos) // x$0
       def wildcardAscription(tp: Type) = Typed(Underscore(tp), TypeTree(tp))
-      val pattern = Bind(thatAsClazz, wildcardAscription(clazzType)) // x$0 @ (_: C)
+      val pattern = Bind(thatAsClazz, wildcardAscription(AnnotatedType(clazzType, Annotation(defn.UncheckedAnnot)))) // x$0 @ (_: C @unchecked)
       val comparisons = accessors map { accessor =>
         This(clazz).select(accessor).equal(ref(thatAsClazz).select(accessor)) }
       val rhs = // this.x == this$0.x && this.y == x$0.y
@@ -250,10 +254,12 @@ class SyntheticMethods(thisPhase: DenotTransformer) {
      *  gets the `canEqual` method
      *
      *  ```
-     *  def canEqual(that: Any) = that.isInstanceOf[C]
+     *  def canEqual(that: Any) = that.isInstanceOf[C @unchecked]
      *  ```
+     *
+     *  `@unchecked` is needed for parametric case classes.
      */
-    def canEqualBody(that: Tree): Tree = that.isInstance(clazzType)
+    def canEqualBody(that: Tree): Tree = that.isInstance(AnnotatedType(clazzType, Annotation(defn.UncheckedAnnot)))
 
     symbolsToSynthesize flatMap syntheticDefIfMissing
   }

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -78,6 +78,7 @@ class CompilationTests extends ParallelTesting {
     ) +
     compileFilesInDir("tests/pos-special/spec-t5545", defaultOptions) +
     compileFilesInDir("tests/pos-special/strawman-collections", defaultOptions) +
+    compileFilesInDir("tests/pos-special/isInstanceOf", allowDeepSubtypes.and("-Xfatal-warnings")) +
     compileFile("scala2-library/src/library/scala/collection/immutable/IndexedSeq.scala", defaultOptions) +
     compileFile("scala2-library/src/library/scala/collection/parallel/mutable/ParSetLike.scala", defaultOptions) +
     compileList(
@@ -190,6 +191,7 @@ class CompilationTests extends ParallelTesting {
     compileFile("tests/neg-custom-args/noimports2.scala", defaultOptions.and("-Yno-imports")) +
     compileFile("tests/neg-custom-args/i3882.scala", allowDeepSubtypes) +
     compileFile("tests/neg-custom-args/i1754.scala", allowDeepSubtypes) +
+    compileFilesInDir("tests/neg-custom-args/isInstanceOf", allowDeepSubtypes and "-Xfatal-warnings") +
     compileFile("tests/neg-custom-args/i3627.scala", allowDeepSubtypes)
   }.checkExpectedErrors()
 

tests/neg-custom-args/isInstanceOf/1828.scala

@@ -0,0 +1,9 @@
+class Test {
+  def remove[S](a: S | Int, f: Int => S):S = a match {
+    case a: S => a             // error
+    case a: Int => f(a)
+  }
+
+  val t: Int | String = 5
+  val t1 = remove[String](t, _.toString)
+}

tests/neg-custom-args/isInstanceOf/3324b.scala

@@ -0,0 +1,9 @@
+class C[T] {
+  val x: Any = ???
+  if (x.isInstanceOf[List[String]]) // error: unchecked
+    if (x.isInstanceOf[T])          // error: unchecked
+      x match {
+        case x: List[String] =>     // error: unchecked
+        case x: T =>                // error: unchecked
+      }
+}

tests/neg-custom-args/isInstanceOf/3324f.scala

@@ -0,0 +1,8 @@
+trait C[T]
+class D[T]
+
+class Test {
+  def foo[T](x: C[T]) = x match {
+    case _: D[T] =>  // error
+  }
+}

tests/neg-custom-args/isInstanceOf/3324g.scala

@@ -0,0 +1,19 @@
+class Test {
+  trait A[+T]
+  class B[T] extends A[T]
+  class C[T] extends B[Any] with A[T]
+
+  def foo[T](c: C[T]): Unit = c match {
+    case _: B[T] => // error
+  }
+
+  def bar[T](b: B[T]): Unit = b match {
+    case _: A[T] =>
+  }
+
+  def quux[T](a: A[T]): Unit = a match {
+    case _: B[T] => // should be an error!!
+  }
+
+  quux(new C[Int])
+}

tests/neg-custom-args/isInstanceOf/4075.scala.ignore

@@ -0,0 +1,22 @@
+object Test {
+  trait Foo
+  case class One[+T](fst: T)
+
+  def bad[T <: Foo](e: One[T])(x: T) = e match {
+    case foo: One[a] =>
+      x.isInstanceOf[a]  // error
+      val y: Any = ???
+      y.isInstanceOf[a]  // error
+  }
+}
+
+object Test2 {
+  case class One[T](fst: T)
+
+  def bad[T](e: One[T])(x: T) = e match {
+    case foo: One[a] =>
+      x.isInstanceOf[a] // error
+      val y: Any = ???
+      y.isInstanceOf[a] // error
+  }
+}

tests/neg-custom-args/isInstanceOf/enum-approx2.scala

@@ -0,0 +1,9 @@
+sealed trait Exp[T]
+case class Fun[A, B](f: Exp[A => B]) extends Exp[A => B]
+
+class Test {
+  def eval[T](e: Exp[T]) = e match {
+    case Fun(x: Fun[Int, Double]) => ???          // error
+    case Fun(x: Exp[Int => String]) => ???        // error
+  }
+}

tests/neg-custom-args/isInstanceOf/i3324.scala

@@ -0,0 +1,4 @@
+class Foo {
+  def foo(x: Any): Boolean =
+    x.isInstanceOf[List[String]]  // error
+}

tests/pos-special/isInstanceOf/3324c.scala

@@ -0,0 +1,8 @@
+sealed trait A[T]
+class B[T] extends A[T]
+
+class Test {
+  def f(x: B[Int]) = x match { case _: A[Int] if true => }
+
+  def g(x: A[Int]) = x match { case _: B[Int] => }
+}

tests/pos-special/isInstanceOf/3324d.scala

@@ -0,0 +1,8 @@
+class Test {
+  val x: Any = ???
+
+  x match {
+    case _: List[Int @unchecked] => 5
+    case _: List[Int] @unchecked => 5
+  }
+}

tests/pos-special/isInstanceOf/3324e.scala

@@ -0,0 +1,17 @@
+class C[T] {
+  val x: T = ???
+  x.isInstanceOf[T]
+
+  val y: Array[T] = ???
+
+  y match {
+    case x: Array[T] =>
+  }
+
+  type F[X]
+
+  val z: F[T] = ???
+  z match {
+    case x: F[T] =>
+  }
+}

tests/pos-special/isInstanceOf/3324h.scala

@@ -0,0 +1,8 @@
+object Test {
+  trait Marker
+  def foo[T](x: T) = x match {
+    case _: (T & Marker)       => // no warning
+    case _: T with Marker      => // scalac emits a warning
+    case _ =>
+  }
+}

tests/pos-special/isInstanceOf/Result.scala

@@ -0,0 +1,8 @@
+object p {
+
+  // test parametric case classes, which synthesis `canEqual` and `equals`
+  enum Result[+T, +E] {
+    case OK [T](x: T) extends Result[T, Nothing]
+    case Err[E](e: E) extends Result[Nothing, E]
+  }
+}

tests/pos-special/isInstanceOf/classTag.scala

@@ -0,0 +1,10 @@
+import scala.reflect.ClassTag
+
+object IsInstanceOfClassTag {
+  def safeCast[T: ClassTag](x: Any): Option[T] = {
+    x match {
+      case x: T => Some(x)
+      case _ => None
+    }
+  }
+}