Skip to content

Commit 23e64a3

Browse files
authored
Merge pull request #2820 from dotty-staging/fix-isInstanceOf
Merge isInstanceOfEvaluator with TypeTestCasts
2 parents bae3df2 + c9e36fb commit 23e64a3

File tree

10 files changed

+135
-96
lines changed

10 files changed

+135
-96
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ class Compiler {
7070
new CrossCastAnd, // Normalize selections involving intersection types.
7171
new Splitter), // Expand selections involving union types into conditionals
7272
List(new VCInlineMethods, // Inlines calls to value class methods
73-
new IsInstanceOfEvaluator, // Issues warnings when unreachable statements are present in match/if expressions
7473
new SeqLiterals, // Express vararg arguments as arrays
7574
new InterceptedMethods, // Special handling of `==`, `|=`, `getClass` methods
7675
new Getters, // Replace non-private vals and vars with getter defs (fields are added later)

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
601601
override def TypeApply(tree: Tree)(fun: Tree, args: List[Tree])(implicit ctx: Context): TypeApply =
602602
ta.assignType(untpd.cpy.TypeApply(tree)(fun, args), fun, args)
603603
// Same remark as for Apply
604-
604+
605605
override def Closure(tree: Tree)(env: List[Tree], meth: Tree, tpt: Tree)(implicit ctx: Context): Closure =
606606
ta.assignType(untpd.cpy.Closure(tree)(env, meth, tpt), meth, tpt)
607607

@@ -770,6 +770,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
770770
else if (!ctx.erasedTypes) asInstance(tp)
771771
else Erasure.Boxing.adaptToType(tree, tp)
772772

773+
/** `tree ne null` (might need a cast to be type correct) */
774+
def testNotNull(implicit ctx: Context): Tree =
775+
tree.ensureConforms(defn.ObjectType)
776+
.select(defn.Object_ne).appliedTo(Literal(Constant(null)))
777+
773778
/** If inititializer tree is `_', the default value of its type,
774779
* otherwise the tree itself.
775780
*/

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,8 @@ class Definitions {
251251
lazy val Any_getClass = enterMethod(AnyClass, nme.getClass_, MethodType(Nil, ClassClass.typeRef.appliedTo(TypeBounds.empty)), Final)
252252
lazy val Any_isInstanceOf = enterT1ParameterlessMethod(AnyClass, nme.isInstanceOf_, _ => BooleanType, Final)
253253
lazy val Any_asInstanceOf = enterT1ParameterlessMethod(AnyClass, nme.asInstanceOf_, TypeParamRef(_, 0), Final)
254+
lazy val Any_typeTest = enterT1ParameterlessMethod(AnyClass, nme.isInstanceOfPM, _ => BooleanType, Final)
255+
// generated by pattern matcher, eliminated by erasure
254256

255257
def AnyMethods = List(Any_==, Any_!=, Any_equals, Any_hashCode,
256258
Any_toString, Any_##, Any_getClass, Any_isInstanceOf, Any_asInstanceOf)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,7 @@ object StdNames {
438438
val isDefined: N = "isDefined"
439439
val isEmpty: N = "isEmpty"
440440
val isInstanceOf_ : N = "isInstanceOf"
441+
val isInstanceOfPM: N = "$isInstanceOf$"
441442
val java: N = "java"
442443
val key: N = "key"
443444
val lang: N = "lang"

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,15 @@ object Types {
218218
* For the moment this is only true for modules, but it could
219219
* be refined later.
220220
*/
221-
final def isNotNull(implicit ctx: Context): Boolean =
222-
classSymbol is ModuleClass
221+
final def isNotNull(implicit ctx: Context): Boolean = this match {
222+
case tp: ConstantType => tp.value.value != null
223+
case tp: ClassInfo => !tp.cls.isNullableClass && tp.cls != defn.NothingClass
224+
case tp: TypeBounds => tp.lo.isNotNull
225+
case tp: TypeProxy => tp.underlying.isNotNull
226+
case AndType(tp1, tp2) => tp1.isNotNull || tp2.isNotNull
227+
case OrType(tp1, tp2) => tp1.isNotNull && tp2.isNotNull
228+
case _ => false
229+
}
223230

224231
/** Is this type produced as a repair for an error? */
225232
final def isError(implicit ctx: Context): Boolean = stripTypeVar match {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ object Erasure extends TypeTestsCasts{
172172
}
173173

174174
def constant(tree: Tree, const: Tree)(implicit ctx: Context) =
175-
if (isPureExpr(tree)) const else Block(tree :: Nil, const)
175+
(if (isPureExpr(tree)) const else Block(tree :: Nil, const))
176+
.withPos(tree.pos)
176177

177178
final def box(tree: Tree, target: => String = "")(implicit ctx: Context): Tree = ctx.traceIndented(i"boxing ${tree.showSummary}: ${tree.tpe} into $target") {
178179
tree.tpe.widen match {

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

Lines changed: 112 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
package dotty.tools.dotc
22
package transform
33

4-
import core.Contexts._
5-
import core.Symbols._
6-
import core.Types._
7-
import core.Constants._
8-
import core.StdNames._
9-
import core.TypeErasure.isUnboundedGeneric
4+
import core._
5+
import Contexts._, Symbols._, Types._, Constants._, StdNames._, Decorators._
106
import ast.Trees._
117
import Erasure.Boxing._
12-
import core.TypeErasure._
8+
import TypeErasure._
139
import ValueClasses._
10+
import core.Flags._
11+
import util.Positions._
12+
1413

1514
/** This transform normalizes type tests and type casts,
1615
* also replacing type tests with singleton argument type with reference equality check
@@ -26,100 +25,140 @@ import ValueClasses._
2625
trait TypeTestsCasts {
2726
import ast.tpd._
2827

29-
// override def phaseName: String = "typeTestsCasts"
30-
3128
def interceptTypeApply(tree: TypeApply)(implicit ctx: Context): Tree = ctx.traceIndented(s"transforming ${tree.show}", show = true) {
3229
tree.fun match {
33-
case fun @ Select(qual, selector) =>
30+
case fun @ Select(expr, selector) =>
3431
val sym = tree.symbol
3532

3633
def isPrimitive(tp: Type) = tp.classSymbol.isPrimitiveValueClass
3734

38-
def derivedTree(qual1: Tree, sym: Symbol, tp: Type) =
39-
cpy.TypeApply(tree)(qual1.select(sym).withPos(qual.pos), List(TypeTree(tp)))
40-
41-
def qualCls = qual.tpe.widen.classSymbol
42-
43-
def transformIsInstanceOf(expr:Tree, argType: Type): Tree = {
44-
def argCls = argType.classSymbol
45-
if ((expr.tpe <:< argType) && isPureExpr(expr))
46-
Literal(Constant(true)) withPos tree.pos
47-
else if (argCls.isPrimitiveValueClass)
48-
if (qualCls.isPrimitiveValueClass) Literal(Constant(qualCls == argCls)) withPos tree.pos
49-
else transformIsInstanceOf(expr, defn.boxedType(argCls.typeRef))
50-
else argType.dealias match {
51-
case _: SingletonType =>
52-
val cmpOp = if (argType derivesFrom defn.AnyValClass) defn.Any_equals else defn.Object_eq
53-
expr.select(cmpOp).appliedTo(singleton(argType))
54-
case AndType(tp1, tp2) =>
55-
evalOnce(expr) { fun =>
56-
val erased1 = transformIsInstanceOf(fun, tp1)
57-
val erased2 = transformIsInstanceOf(fun, tp2)
58-
erased1 match {
59-
case Literal(Constant(true)) => erased2
60-
case _ =>
61-
erased2 match {
62-
case Literal(Constant(true)) => erased1
63-
case _ => erased1 and erased2
64-
}
65-
}
35+
def derivedTree(expr1: Tree, sym: Symbol, tp: Type) =
36+
cpy.TypeApply(tree)(expr1.select(sym).withPos(expr.pos), List(TypeTree(tp)))
37+
38+
def foundCls = expr.tpe.widen.classSymbol
39+
// println(i"ta $tree, found = $foundCls")
40+
41+
def inMatch =
42+
fun.symbol == defn.Any_typeTest || // new scheme
43+
expr.symbol.is(Case) // old scheme
44+
45+
def transformIsInstanceOf(expr:Tree, testType: Type, flagUnrelated: Boolean): Tree = {
46+
def testCls = testType.classSymbol
47+
48+
def unreachable(why: => String) =
49+
if (flagUnrelated)
50+
if (inMatch) ctx.error(em"this case is unreachable since $why", expr.pos)
51+
else ctx.warning(em"this will always yield false since $why", expr.pos)
52+
53+
/** Are `foundCls` and `testCls` classes that allow checks
54+
* whether a test would be always false?
55+
*/
56+
def isCheckable =
57+
foundCls.isClass && testCls.isClass &&
58+
!(testCls.isPrimitiveValueClass && !foundCls.isPrimitiveValueClass) &&
59+
// if `test` is primitive but `found` is not, we might have a case like
60+
// found = java.lang.Integer, test = Int, which could be true
61+
// (not sure why that is so, but scalac behaves the same way)
62+
!isDerivedValueClass(foundCls) && !isDerivedValueClass(testCls)
63+
// we don't have the logic to handle derived value classes
64+
65+
/** Check whether a runtime test that a value of `foundCls` can be a `testCls`
66+
* can be true in some cases. Issure a warning or an error if that's not the case.
67+
*/
68+
def checkSensical: Boolean =
69+
if (!isCheckable) true
70+
else if (foundCls.isPrimitiveValueClass && !testCls.isPrimitiveValueClass) {
71+
ctx.error("cannot test if value types are references", tree.pos)
72+
false
6673
}
67-
case defn.MultiArrayOf(elem, ndims) if isUnboundedGeneric(elem) =>
68-
def isArrayTest(arg: Tree) =
69-
ref(defn.runtimeMethodRef(nme.isArray)).appliedTo(arg, Literal(Constant(ndims)))
70-
if (ndims == 1) isArrayTest(qual)
71-
else evalOnce(qual) { qual1 =>
72-
derivedTree(qual1, defn.Any_isInstanceOf, qual1.tpe) and isArrayTest(qual1)
74+
else if (!foundCls.derivesFrom(testCls)) {
75+
if (foundCls.is(Final)) {
76+
unreachable(i"$foundCls is not a subclass of $testCls")
77+
false
7378
}
74-
case _ =>
75-
derivedTree(expr, defn.Any_isInstanceOf, argType)
76-
}
79+
else if (!testCls.derivesFrom(foundCls) &&
80+
(testCls.is(Final) ||
81+
!testCls.is(Trait) && !foundCls.is(Trait))) {
82+
unreachable(i"$foundCls and $testCls are unrelated")
83+
false
84+
}
85+
else true
86+
}
87+
else true
88+
89+
if (expr.tpe <:< testType)
90+
if (expr.tpe.isNotNull) {
91+
ctx.warning(
92+
em"this will always yield true, since `$foundCls` is a subclass of `$testCls`",
93+
expr.pos)
94+
constant(expr, Literal(Constant(true)))
95+
}
96+
else expr.testNotNull
97+
else if (!checkSensical)
98+
constant(expr, Literal(Constant(false)))
99+
else if (testCls.isPrimitiveValueClass)
100+
if (foundCls.isPrimitiveValueClass)
101+
constant(expr, Literal(Constant(foundCls == testCls)))
102+
else
103+
transformIsInstanceOf(expr, defn.boxedType(testCls.typeRef), flagUnrelated)
104+
else
105+
derivedTree(expr, defn.Any_isInstanceOf, testType)
77106
}
78107

79-
def transformAsInstanceOf(argType: Type): Tree = {
80-
def argCls = argType.widen.classSymbol
81-
if (qual.tpe <:< argType)
82-
Typed(qual, tree.args.head)
83-
else if (qualCls.isPrimitiveValueClass) {
84-
if (argCls.isPrimitiveValueClass) primitiveConversion(qual, argCls)
85-
else derivedTree(box(qual), defn.Any_asInstanceOf, argType)
108+
def transformAsInstanceOf(testType: Type): Tree = {
109+
def testCls = testType.widen.classSymbol
110+
if (expr.tpe <:< testType)
111+
Typed(expr, tree.args.head)
112+
else if (foundCls.isPrimitiveValueClass) {
113+
if (testCls.isPrimitiveValueClass) primitiveConversion(expr, testCls)
114+
else derivedTree(box(expr), defn.Any_asInstanceOf, testType)
86115
}
87-
else if (argCls.isPrimitiveValueClass)
88-
unbox(qual.ensureConforms(defn.ObjectType), argType)
89-
else if (isDerivedValueClass(argCls)) {
90-
qual // adaptToType in Erasure will do the necessary type adaptation
116+
else if (testCls.isPrimitiveValueClass)
117+
unbox(expr.ensureConforms(defn.ObjectType), testType)
118+
else if (isDerivedValueClass(testCls)) {
119+
expr // adaptToType in Erasure will do the necessary type adaptation
91120
}
92121
else
93-
derivedTree(qual, defn.Any_asInstanceOf, argType)
122+
derivedTree(expr, defn.Any_asInstanceOf, testType)
94123
}
95124

96125
/** Transform isInstanceOf OrType
97126
*
98127
* expr.isInstanceOf[A | B] ~~> expr.isInstanceOf[A] | expr.isInstanceOf[B]
99128
* expr.isInstanceOf[A & B] ~~> expr.isInstanceOf[A] & expr.isInstanceOf[B]
100129
*
101-
* The transform happens before erasure of `argType`, thus cannot be merged
102-
* with `transformIsInstanceOf`, which depends on erased type of `argType`.
130+
* The transform happens before erasure of `testType`, thus cannot be merged
131+
* with `transformIsInstanceOf`, which depends on erased type of `testType`.
103132
*/
104-
def transformTypeTest(qual: Tree, argType: Type): Tree = argType.dealias match {
133+
def transformTypeTest(expr: Tree, testType: Type, flagUnrelated: Boolean): Tree = testType.dealias match {
134+
case _: SingletonType =>
135+
val cmpOp =
136+
if (testType derivesFrom defn.AnyValClass) defn.Any_equals else defn.Object_eq
137+
expr.select(cmpOp).appliedTo(singleton(testType))
105138
case OrType(tp1, tp2) =>
106-
evalOnce(qual) { fun =>
107-
transformTypeTest(fun, tp1)
108-
.select(defn.Boolean_||)
109-
.appliedTo(transformTypeTest(fun, tp2))
139+
evalOnce(expr) { e =>
140+
transformTypeTest(e, tp1, flagUnrelated = false)
141+
.or(transformTypeTest(e, tp2, flagUnrelated = false))
110142
}
111143
case AndType(tp1, tp2) =>
112-
evalOnce(qual) { fun =>
113-
transformTypeTest(fun, tp1)
114-
.select(defn.Boolean_&&)
115-
.appliedTo(transformTypeTest(fun, tp2))
144+
evalOnce(expr) { e =>
145+
transformTypeTest(e, tp1, flagUnrelated)
146+
.and(transformTypeTest(e, tp2, flagUnrelated))
147+
}
148+
case defn.MultiArrayOf(elem, ndims) if isUnboundedGeneric(elem) =>
149+
def isArrayTest(arg: Tree) =
150+
ref(defn.runtimeMethodRef(nme.isArray)).appliedTo(arg, Literal(Constant(ndims)))
151+
if (ndims == 1) isArrayTest(expr)
152+
else evalOnce(expr) { e =>
153+
derivedTree(e, defn.Any_isInstanceOf, e.tpe)
154+
.and(isArrayTest(e))
116155
}
117156
case _ =>
118-
transformIsInstanceOf(qual, erasure(argType))
157+
transformIsInstanceOf(expr, erasure(testType), flagUnrelated)
119158
}
120159

121-
if (sym eq defn.Any_isInstanceOf)
122-
transformTypeTest(qual, tree.args.head.tpe)
160+
if ((sym eq defn.Any_isInstanceOf) || (sym eq defn.Any_typeTest))
161+
transformTypeTest(expr, tree.args.head.tpe, flagUnrelated = true)
123162
else if (sym eq defn.Any_asInstanceOf)
124163
transformAsInstanceOf(erasure(tree.args.head.tpe))
125164
else tree

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -872,22 +872,6 @@ class RefChecks extends MiniPhase { thisTransformer =>
872872
currentLevel.enterReference(tree.tpe.typeSymbol, tree.pos)
873873
tree
874874
}
875-
876-
override def transformTypeApply(tree: tpd.TypeApply)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = {
877-
tree.fun match {
878-
case fun@Select(qual, selector) =>
879-
val sym = tree.symbol
880-
881-
if (sym == defn.Any_isInstanceOf) {
882-
val argType = tree.args.head.tpe
883-
val qualCls = qual.tpe.widen.classSymbol
884-
val argCls = argType.classSymbol
885-
if (qualCls.isPrimitiveValueClass && !argCls.isPrimitiveValueClass) ctx.error("isInstanceOf cannot test if value types are references", tree.pos)
886-
}
887-
case _ =>
888-
}
889-
tree
890-
}
891875
}
892876
}
893877

compiler/test/dotty/tools/vulpix/TestConfiguration.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ object TestConfiguration {
5353
private val yCheckOptions = Array("-Ycheck:tailrec,resolveSuper,mixin,arrayConstructors,labelDef")
5454

5555
val defaultUnoptimised = noCheckOptions ++ checkOptions ++ yCheckOptions ++ classPath
56-
val defaultOptions = defaultUnoptimised :+ "-optimise"
56+
val defaultOptimised = defaultUnoptimised :+ "-optimise"
57+
val defaultOptions = defaultUnoptimised
5758
val allowDeepSubtypes = defaultOptions diff Array("-Yno-deep-subtypes")
5859
val allowDoubleBindings = defaultOptions diff Array("-Yno-double-bindings")
5960
val picklingOptions = defaultUnoptimised ++ Array(

tests/neg/tryPatternMatchError.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ object Test {
2525
case _: ExceptionTrait =>
2626
case _: NoSuchElementException if a <= 1 =>
2727
case _: NullPointerException | _:IOException =>
28-
case e: Int => // error
28+
case e: Int => // error: unrelated
2929
case EX =>
3030
case IAE(msg) =>
3131
case e: IllegalArgumentException =>

0 commit comments

Comments
 (0)