From bee809ddbaac86a915613506035ec2d013b1292a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Tue, 22 Jun 2021 20:04:36 +0200 Subject: [PATCH 1/2] Restricts `isInstanceOf[Null]` checks Fixes #4004 `isInstanceOf[Nothing]` checks are always prohibited `isInstanceOf[Null]` checks are probihibited unless they can be proven at compiletime, then thay are simplified to `true` --- .../src/dotty/tools/dotc/core/Definitions.scala | 2 ++ .../dotty/tools/dotc/transform/TypeTestsCasts.scala | 6 ++++-- tests/neg/i4004.scala | 13 +++++++++++++ tests/run/i4004.check | 2 ++ tests/run/i4004.scala | 6 ++++++ 5 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 tests/neg/i4004.scala create mode 100644 tests/run/i4004.check create mode 100644 tests/run/i4004.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 547ceb292055..8d71a1718c81 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -1196,6 +1196,8 @@ class Definitions { @tu lazy val topClasses: Set[Symbol] = Set(AnyClass, MatchableClass, ObjectClass, AnyValClass) + @tu lazy val untestableClasses: Set[Symbol] = Set(NothingClass, NullClass) + @tu lazy val AbstractFunctionType: Array[TypeRef] = mkArityArray("scala.runtime.AbstractFunction", MaxImplementedFunctionArity, 0) val AbstractFunctionClassPerRun: PerRun[Array[Symbol]] = new PerRun(AbstractFunctionType.map(_.symbol.asClass)) def AbstractFunctionClass(n: Int)(using Context): Symbol = AbstractFunctionClassPerRun()(using ctx)(n) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 5d03d5381eed..ae0e61ea65e9 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -236,14 +236,16 @@ object TypeTestsCasts { val foundEffectiveClass = effectiveClass(expr.tpe.widen) - if foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass then + val isUntestable = defn.untestableClasses.contains(testCls) + val isIllegalPrimitiveTest = foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass + if isIllegalPrimitiveTest || isUntestable then report.error(i"cannot test if value of $exprType is a reference of $testCls", tree.srcPos) false else foundClasses.exists(check) end checkSensical if (expr.tpe <:< testType) - if (expr.tpe.isNotNull) { + if (expr.tpe.isNotNull || testType.widen.isRef(defn.NullClass)) { if (!inMatch) report.warning(TypeTestAlwaysSucceeds(expr.tpe, testType), tree.srcPos) constant(expr, Literal(Constant(true))) } diff --git a/tests/neg/i4004.scala b/tests/neg/i4004.scala new file mode 100644 index 000000000000..9833038823be --- /dev/null +++ b/tests/neg/i4004.scala @@ -0,0 +1,13 @@ +@main def Test = + "a".isInstanceOf[Null] // error + null.isInstanceOf[Null] + "a".isInstanceOf[Nothing] // error + + "a" match + case _: Null => () // error + case _ => () + + null match + case _: Null => () + case _ => () + diff --git a/tests/run/i4004.check b/tests/run/i4004.check new file mode 100644 index 000000000000..3324d4040ec0 --- /dev/null +++ b/tests/run/i4004.check @@ -0,0 +1,2 @@ +true +null \ No newline at end of file diff --git a/tests/run/i4004.scala b/tests/run/i4004.scala new file mode 100644 index 000000000000..77f38f7e1e33 --- /dev/null +++ b/tests/run/i4004.scala @@ -0,0 +1,6 @@ +@main def Test = + println(null.isInstanceOf[Null]) + + null match + case _: Null => println("null") + case _ => println("not null") \ No newline at end of file From ca4aa168df9cca01e5ffcfd33ef46760a2df6b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Marks?= Date: Wed, 23 Jun 2021 14:45:38 +0200 Subject: [PATCH 2/2] Make `isInstanceOf[Null]` illegal everywhere also add `scala.Singleton` to untestable types --- .../src/dotty/tools/dotc/core/Definitions.scala | 2 +- .../tools/dotc/transform/TypeTestsCasts.scala | 16 ++++++++++------ tests/explicit-nulls/pos/matchable.scala | 2 -- tests/neg/i4004.scala | 7 +++++-- tests/run/i4004.check | 2 -- tests/run/i4004.scala | 6 ------ 6 files changed, 16 insertions(+), 19 deletions(-) delete mode 100644 tests/run/i4004.check delete mode 100644 tests/run/i4004.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 8d71a1718c81..2fec0683c410 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -1196,7 +1196,7 @@ class Definitions { @tu lazy val topClasses: Set[Symbol] = Set(AnyClass, MatchableClass, ObjectClass, AnyValClass) - @tu lazy val untestableClasses: Set[Symbol] = Set(NothingClass, NullClass) + @tu lazy val untestableClasses: Set[Symbol] = Set(NothingClass, NullClass, SingletonClass) @tu lazy val AbstractFunctionType: Array[TypeRef] = mkArityArray("scala.runtime.AbstractFunction", MaxImplementedFunctionArity, 0) val AbstractFunctionClassPerRun: PerRun[Array[Symbol]] = new PerRun(AbstractFunctionType.map(_.symbol.asClass)) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index ae0e61ea65e9..a4951b99f599 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -236,16 +236,14 @@ object TypeTestsCasts { val foundEffectiveClass = effectiveClass(expr.tpe.widen) - val isUntestable = defn.untestableClasses.contains(testCls) - val isIllegalPrimitiveTest = foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass - if isIllegalPrimitiveTest || isUntestable then + if foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass then report.error(i"cannot test if value of $exprType is a reference of $testCls", tree.srcPos) false else foundClasses.exists(check) end checkSensical if (expr.tpe <:< testType) - if (expr.tpe.isNotNull || testType.widen.isRef(defn.NullClass)) { + if (expr.tpe.isNotNull) { if (!inMatch) report.warning(TypeTestAlwaysSucceeds(expr.tpe, testType), tree.srcPos) constant(expr, Literal(Constant(true))) } @@ -341,8 +339,14 @@ object TypeTestsCasts { case AppliedType(tref: TypeRef, _) if tref.symbol == defn.PairClass => ref(defn.RuntimeTuples_isInstanceOfNonEmptyTuple).appliedTo(expr) case _ => - val erasedTestType = erasure(testType) - transformIsInstanceOf(expr, erasedTestType, erasedTestType, flagUnrelated) + val testWidened = testType.widen + defn.untestableClasses.find(testWidened.isRef(_)) match + case Some(untestable) => + report.error(i"$untestable cannot be used in runtime type tests", tree.srcPos) + constant(expr, Literal(Constant(false))) + case _ => + val erasedTestType = erasure(testType) + transformIsInstanceOf(expr, erasedTestType, erasedTestType, flagUnrelated) } if (sym.isTypeTest) { diff --git a/tests/explicit-nulls/pos/matchable.scala b/tests/explicit-nulls/pos/matchable.scala index 928f8e33934f..ef9c5e7eae07 100644 --- a/tests/explicit-nulls/pos/matchable.scala +++ b/tests/explicit-nulls/pos/matchable.scala @@ -1,3 +1 @@ -def foo1[T <: Matchable](t: T) = t match { case t: Null => () } - def foo2[T <: Matchable](t: T) = t match { case null => () } \ No newline at end of file diff --git a/tests/neg/i4004.scala b/tests/neg/i4004.scala index 9833038823be..bf757a0863a7 100644 --- a/tests/neg/i4004.scala +++ b/tests/neg/i4004.scala @@ -1,13 +1,16 @@ @main def Test = "a".isInstanceOf[Null] // error - null.isInstanceOf[Null] + null.isInstanceOf[Null] // error "a".isInstanceOf[Nothing] // error + "a".isInstanceOf[Singleton] // error "a" match case _: Null => () // error + case _: Nothing => () // error + case _: Singleton => () // error case _ => () null match - case _: Null => () + case _: Null => () // error case _ => () diff --git a/tests/run/i4004.check b/tests/run/i4004.check deleted file mode 100644 index 3324d4040ec0..000000000000 --- a/tests/run/i4004.check +++ /dev/null @@ -1,2 +0,0 @@ -true -null \ No newline at end of file diff --git a/tests/run/i4004.scala b/tests/run/i4004.scala deleted file mode 100644 index 77f38f7e1e33..000000000000 --- a/tests/run/i4004.scala +++ /dev/null @@ -1,6 +0,0 @@ -@main def Test = - println(null.isInstanceOf[Null]) - - null match - case _: Null => println("null") - case _ => println("not null") \ No newline at end of file