summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Checkable.scala48
-rw-r--r--src/reflect/scala/reflect/internal/Symbols.scala4
-rw-r--r--test/files/neg/t8597.check21
-rw-r--r--test/files/neg/t8597.flags1
-rw-r--r--test/files/neg/t8597.scala27
-rw-r--r--test/files/neg/t8597b.check6
-rw-r--r--test/files/neg/t8597b.flags1
-rw-r--r--test/files/neg/t8597b.scala21
-rw-r--r--test/files/neg/unchecked-abstract.check14
9 files changed, 137 insertions, 6 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
index 3a77cab919..fc632e0d0d 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
@@ -11,12 +11,28 @@ import scala.language.postfixOps
/** On pattern matcher checkability:
*
+ * The spec says that case _: List[Int] should be always issue
+ * an unchecked warning:
+ *
+ * > Types which are not of one of the forms described above are
+ * > also accepted as type patterns. However, such type patterns
+ * > will be translated to their erasure (§3.7). The Scala compiler
+ * > will issue an “unchecked” warning for these patterns to flag
+ * > the possible loss of type-safety.
+ *
+ * But the implementation goes a little further to omit warnings
+ * based on the static type of the scrutinee. As a trivial example:
+ *
+ * def foo(s: Seq[Int]) = s match { case _: List[Int] => }
+ *
+ * need not issue this warning.
+ *
* Consider a pattern match of this form: (x: X) match { case _: P => }
*
* There are four possibilities to consider:
* [P1] X will always conform to P
* [P2] x will never conform to P
- * [P3] X <: P if some runtime test is true
+ * [P3] X will conform to P if some runtime test is true
* [P4] X cannot be checked against P
*
* The first two cases correspond to those when there is enough
@@ -28,6 +44,11 @@ import scala.language.postfixOps
* which is essentially the intersection of X and |P|, where |P| is
* the erasure of P. If XR <: P, then no warning is emitted.
*
+ * We evaluate "X with conform to P" by checking `X <: P_wild, where
+ * P_wild is the result of substituting wildcard types in place of
+ * pattern type variables. This is intentionally stricter than
+ * (X matchesPattern P), see SI-8597 for motivating test cases.
+ *
* Examples of how this info is put to use:
* sealed trait A[T] ; class B[T] extends A[T]
* def f(x: B[Int]) = x match { case _: A[Int] if true => }
@@ -100,7 +121,7 @@ trait Checkable {
private def typeArgsInTopLevelType(tp: Type): List[Type] = {
val tps = tp match {
case RefinedType(parents, _) => parents flatMap typeArgsInTopLevelType
- case TypeRef(_, ArrayClass, arg :: Nil) => typeArgsInTopLevelType(arg)
+ case TypeRef(_, ArrayClass, arg :: Nil) => if (arg.typeSymbol.isAbstractType) arg :: Nil else typeArgsInTopLevelType(arg)
case TypeRef(pre, sym, args) => typeArgsInTopLevelType(pre) ++ args
case ExistentialType(tparams, underlying) => tparams.map(_.tpe) ++ typeArgsInTopLevelType(underlying)
case _ => Nil
@@ -108,14 +129,31 @@ trait Checkable {
tps filterNot isUnwarnableTypeArg
}
+ private def scrutConformsToPatternType(scrut: Type, pattTp: Type): Boolean = {
+ def typeVarToWildcard(tp: Type) = {
+ // The need for typeSymbolDirect is demonstrated in neg/t8597b.scala
+ if (tp.typeSymbolDirect.isPatternTypeVariable) WildcardType else tp
+ }
+ val pattTpWild = pattTp.map(typeVarToWildcard)
+ scrut <:< pattTpWild
+ }
+
private class CheckabilityChecker(val X: Type, val P: Type) {
def Xsym = X.typeSymbol
def Psym = P.typeSymbol
- def XR = if (Xsym == AnyClass) classExistentialType(Psym) else propagateKnownTypes(X, Psym)
+ def PErased = {
+ P match {
+ case erasure.GenericArray(n, core) => existentialAbstraction(core.typeSymbol :: Nil, P)
+ case _ => existentialAbstraction(Psym.typeParams, Psym.tpe_*)
+ }
+ }
+ def XR = if (Xsym == AnyClass) PErased else propagateKnownTypes(X, Psym)
+
+
// sadly the spec says (new java.lang.Boolean(true)).isInstanceOf[scala.Boolean]
- def P1 = X matchesPattern P
+ def P1 = scrutConformsToPatternType(X, P)
def P2 = !Psym.isPrimitiveValueClass && isNeverSubType(X, P)
- def P3 = isNonRefinementClassType(P) && (XR matchesPattern P)
+ def P3 = isNonRefinementClassType(P) && scrutConformsToPatternType(XR, P)
def P4 = !(P1 || P2 || P3)
def summaryString = f"""
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index 51f06b1d6d..b2f176e894 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -792,6 +792,10 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
final def isDefinedInPackage = effectiveOwner.isPackageClass
final def needsFlatClasses = phase.flatClasses && rawowner != NoSymbol && !rawowner.isPackageClass
+ // TODO introduce a flag for these?
+ final def isPatternTypeVariable: Boolean =
+ isAbstractType && !isExistential && !isTypeParameterOrSkolem && isLocalToBlock
+
/** change name by appending $$<fully-qualified-name-of-class `base`>
* Do the same for any accessed symbols or setters/getters.
* Implementation in TermSymbol.
diff --git a/test/files/neg/t8597.check b/test/files/neg/t8597.check
new file mode 100644
index 0000000000..bc945f9191
--- /dev/null
+++ b/test/files/neg/t8597.check
@@ -0,0 +1,21 @@
+t8597.scala:2: warning: abstract type T in type pattern Some[T] is unchecked since it is eliminated by erasure
+ def nowarn[T] = (null: Any) match { case _: Some[T] => } // warn (did not warn due to SI-8597)
+ ^
+t8597.scala:5: warning: abstract type pattern T is unchecked since it is eliminated by erasure
+ def warn1[T] = (null: Any) match { case _: T => } // warn
+ ^
+t8597.scala:6: warning: non-variable type argument String in type pattern Some[String] is unchecked since it is eliminated by erasure
+ def warn2 = (null: Any) match { case _: Some[String] => } // warn
+ ^
+t8597.scala:7: warning: non-variable type argument Unchecked.this.C in type pattern Some[Unchecked.this.C] is unchecked since it is eliminated by erasure
+ (null: Any) match { case _: Some[C] => } // warn
+ ^
+t8597.scala:18: warning: abstract type T in type pattern Array[T] is unchecked since it is eliminated by erasure
+ def warnArray[T] = (null: Any) match { case _: Array[T] => } // warn (did not warn due to SI-8597)
+ ^
+t8597.scala:26: warning: non-variable type argument String in type pattern Array[Array[List[String]]] is unchecked since it is eliminated by erasure
+ def warnArrayErasure2 = (null: Any) match {case Some(_: Array[Array[List[String]]]) => } // warn
+ ^
+error: No warnings can be incurred under -Xfatal-warnings.
+6 warnings found
+one error found
diff --git a/test/files/neg/t8597.flags b/test/files/neg/t8597.flags
new file mode 100644
index 0000000000..85d8eb2ba2
--- /dev/null
+++ b/test/files/neg/t8597.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
diff --git a/test/files/neg/t8597.scala b/test/files/neg/t8597.scala
new file mode 100644
index 0000000000..068e87d91a
--- /dev/null
+++ b/test/files/neg/t8597.scala
@@ -0,0 +1,27 @@
+class Unchecked[C] {
+ def nowarn[T] = (null: Any) match { case _: Some[T] => } // warn (did not warn due to SI-8597)
+
+ // These warned before.
+ def warn1[T] = (null: Any) match { case _: T => } // warn
+ def warn2 = (null: Any) match { case _: Some[String] => } // warn
+ (null: Any) match { case _: Some[C] => } // warn
+
+ // These must remain without warnings. These are excerpts from
+ // related tests that are more exhauative.
+ class C; class D extends C
+ def okay = (List(new D) : Seq[D]) match { case _: List[C] => case _ => } // nowarn
+ class B2[A, B]
+ class A2[X] extends B2[X, String]
+ def okay2(x: A2[Int]) = x match { case _: B2[Int, _] => true } // nowarn
+ def okay3(x: A2[Int]) = x match { case _: B2[Int, typeVar] => true } // nowarn
+
+ def warnArray[T] = (null: Any) match { case _: Array[T] => } // warn (did not warn due to SI-8597)
+ def nowarnArrayC = (null: Any) match { case _: Array[C] => } // nowarn
+
+ def nowarnArrayTypeVar[T] = (null: Any) match { case _: Array[t] => } // nowarn
+
+ def noWarnArrayErasure1 = (null: Any) match {case Some(_: Array[String]) => } // nowarn
+ def noWarnArrayErasure2 = (null: Any) match {case Some(_: Array[List[_]]) => } // nowarn
+ def noWarnArrayErasure3 = (null: Any) match {case Some(_: Array[Array[List[_]]]) => } // nowarn
+ def warnArrayErasure2 = (null: Any) match {case Some(_: Array[Array[List[String]]]) => } // warn
+}
diff --git a/test/files/neg/t8597b.check b/test/files/neg/t8597b.check
new file mode 100644
index 0000000000..3c45a31337
--- /dev/null
+++ b/test/files/neg/t8597b.check
@@ -0,0 +1,6 @@
+t8597b.scala:18: warning: non-variable type argument T in type pattern Some[T] is unchecked since it is eliminated by erasure
+ case _: Some[T] => // warn
+ ^
+error: No warnings can be incurred under -Xfatal-warnings.
+one warning found
+one error found
diff --git a/test/files/neg/t8597b.flags b/test/files/neg/t8597b.flags
new file mode 100644
index 0000000000..85d8eb2ba2
--- /dev/null
+++ b/test/files/neg/t8597b.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
diff --git a/test/files/neg/t8597b.scala b/test/files/neg/t8597b.scala
new file mode 100644
index 0000000000..b29d591cb1
--- /dev/null
+++ b/test/files/neg/t8597b.scala
@@ -0,0 +1,21 @@
+object Unchecked {
+ (null: Any) match {
+ case _: Some[t] =>
+
+ // t is a fresh pattern type variable, despite our attempts to
+ // backtick our way to the enclosing `t`. Under this interpretation,
+ // the absense of an unchecked warning is expected.
+ (null: Any) match {
+ case _: Some[t] => // no warn
+ }
+ (null: Any) match {
+ case _: Some[`t`] => // no warn
+ }
+
+ // here we correctly issue an unchecked warning
+ type T = t
+ (null: Any) match {
+ case _: Some[T] => // warn
+ }
+ }
+}
diff --git a/test/files/neg/unchecked-abstract.check b/test/files/neg/unchecked-abstract.check
index 72019082ac..703929dca8 100644
--- a/test/files/neg/unchecked-abstract.check
+++ b/test/files/neg/unchecked-abstract.check
@@ -4,6 +4,9 @@ unchecked-abstract.scala:16: warning: abstract type H in type Contravariant[M.th
unchecked-abstract.scala:21: warning: abstract type H in type Contravariant[M.this.H] is unchecked since it is eliminated by erasure
/* warn */ println(x.isInstanceOf[Contravariant[H]])
^
+unchecked-abstract.scala:22: warning: abstract type T in type Contravariant[M.this.T] is unchecked since it is eliminated by erasure
+ /* warn */ println(x.isInstanceOf[Contravariant[T]])
+ ^
unchecked-abstract.scala:27: warning: abstract type T in type Invariant[M.this.T] is unchecked since it is eliminated by erasure
/* warn */ println(x.isInstanceOf[Invariant[T]])
^
@@ -22,6 +25,15 @@ unchecked-abstract.scala:36: warning: abstract type H in type Invariant[M.this.H
unchecked-abstract.scala:37: warning: abstract type T in type Invariant[M.this.T] is unchecked since it is eliminated by erasure
/* warn */ println(x.isInstanceOf[Invariant[T]])
^
+unchecked-abstract.scala:42: warning: abstract type T in type Covariant[M.this.T] is unchecked since it is eliminated by erasure
+ /* warn */ println(x.isInstanceOf[Covariant[T]])
+ ^
+unchecked-abstract.scala:43: warning: abstract type L in type Covariant[M.this.L] is unchecked since it is eliminated by erasure
+ /* warn */ println(x.isInstanceOf[Covariant[L]])
+ ^
+unchecked-abstract.scala:48: warning: abstract type L in type Covariant[M.this.L] is unchecked since it is eliminated by erasure
+ /* warn */ println(x.isInstanceOf[Covariant[L]])
+ ^
error: No warnings can be incurred under -Xfatal-warnings.
-8 warnings found
+12 warnings found
one error found
generated by cgit v1.2.3 (git 2.39.1) at 2024-07-05 14:02:07 +0000