diff options
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Checkable.scala | 48 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Symbols.scala | 4 | ||||
-rw-r--r-- | test/files/neg/t8597.check | 21 | ||||
-rw-r--r-- | test/files/neg/t8597.flags | 1 | ||||
-rw-r--r-- | test/files/neg/t8597.scala | 27 | ||||
-rw-r--r-- | test/files/neg/t8597b.check | 6 | ||||
-rw-r--r-- | test/files/neg/t8597b.flags | 1 | ||||
-rw-r--r-- | test/files/neg/t8597b.scala | 21 | ||||
-rw-r--r-- | test/files/neg/unchecked-abstract.check | 14 |
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 |