Skip to content

Commit

Permalink
Merge pull request #4045 from dotty-staging/fix-3324
Browse files Browse the repository at this point in the history
Fix #3324: add `isInstanceOf` check
  • Loading branch information
liufengyun committed Apr 5, 2018
2 parents 62be430 + 2b00b7a commit b1d9162
Show file tree
Hide file tree
Showing 21 changed files with 384 additions and 7 deletions.
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
133 changes: 133 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/IsInstanceOfChecker.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
14 changes: 10 additions & 4 deletions compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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()

Expand Down
9 changes: 9 additions & 0 deletions tests/neg-custom-args/isInstanceOf/1828.scala
Original file line number Diff line number Diff line change
@@ -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)
}
9 changes: 9 additions & 0 deletions tests/neg-custom-args/isInstanceOf/3324b.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
8 changes: 8 additions & 0 deletions tests/neg-custom-args/isInstanceOf/3324f.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
trait C[T]
class D[T]

class Test {
def foo[T](x: C[T]) = x match {
case _: D[T] => // error
}
}
19 changes: 19 additions & 0 deletions tests/neg-custom-args/isInstanceOf/3324g.scala
Original file line number Diff line number Diff line change
@@ -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])
}
22 changes: 22 additions & 0 deletions tests/neg-custom-args/isInstanceOf/4075.scala.ignore
Original file line number Diff line number Diff line change
@@ -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
}
}
9 changes: 9 additions & 0 deletions tests/neg-custom-args/isInstanceOf/enum-approx2.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
4 changes: 4 additions & 0 deletions tests/neg-custom-args/isInstanceOf/i3324.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class Foo {
def foo(x: Any): Boolean =
x.isInstanceOf[List[String]] // error
}
8 changes: 8 additions & 0 deletions tests/pos-special/isInstanceOf/3324c.scala
Original file line number Diff line number Diff line change
@@ -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] => }
}
8 changes: 8 additions & 0 deletions tests/pos-special/isInstanceOf/3324d.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class Test {
val x: Any = ???

x match {
case _: List[Int @unchecked] => 5
case _: List[Int] @unchecked => 5
}
}
17 changes: 17 additions & 0 deletions tests/pos-special/isInstanceOf/3324e.scala
Original file line number Diff line number Diff line change
@@ -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] =>
}
}
8 changes: 8 additions & 0 deletions tests/pos-special/isInstanceOf/3324h.scala
Original file line number Diff line number Diff line change
@@ -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 _ =>
}
}
8 changes: 8 additions & 0 deletions tests/pos-special/isInstanceOf/Result.scala
Original file line number Diff line number Diff line change
@@ -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]
}
}
10 changes: 10 additions & 0 deletions tests/pos-special/isInstanceOf/classTag.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
}
Loading

0 comments on commit b1d9162

Please sign in to comment.