diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index b70fcb093ff3..467e1f27ffec 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -357,6 +357,9 @@ class Definitions { List(AnyClass.typeRef), EmptyScope) def SingletonType = SingletonClass.typeRef + lazy val ListType: TypeRef = ctx.requiredClassRef("scala.collection.immutable.List") + def ListClass(implicit ctx: Context) = ListType.symbol.asClass + lazy val SeqType: TypeRef = ctx.requiredClassRef("scala.collection.Seq") def SeqClass(implicit ctx: Context) = SeqType.symbol.asClass diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index 6ea168b99241..82eddb9fd8c6 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -56,6 +56,7 @@ public enum ErrorMessageID { CyclicReferenceInvolvingID, CyclicReferenceInvolvingImplicitID, SuperQualMustBeParentID, + ErasedTypeID, ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 87837fd82c0b..e85d6e692e01 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1221,4 +1221,11 @@ object messages { |Attempting to define a field in a method signature after a varargs field is an error. |""".stripMargin } + + case class ErasedType(val explanation: String = "")(implicit ctx: Context) + extends Message(ErasedTypeID) { + val kind = "Erased Type" + val msg = + i"abstract type pattern is unchecked since it is eliminated by erasure" + } } diff --git a/compiler/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/compiler/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala index ebd2ae43634a..d8733fd1cba3 100644 --- a/compiler/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala +++ b/compiler/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -5,7 +5,8 @@ import dotty.tools.dotc.util.Positions._ import TreeTransforms.{MiniPhaseTransform, TransformerInfo} import core._ import Contexts.Context, Types._, Constants._, Decorators._, Symbols._ -import TypeUtils._, TypeErasure._, Flags._ +import TypeUtils._, TypeErasure._, Flags._, TypeApplications._ +import reporting.diagnostic.messages._ /** Implements partial evaluation of `sc.isInstanceOf[Sel]` according to: * @@ -18,6 +19,11 @@ import TypeUtils._, TypeErasure._, Flags._ * This is a generalized solution to raising an error on unreachable match * cases and warnings on other statically known results of `isInstanceOf`. * + * This phase also warns if the erased type parameter of a parameterized type + * is used in a match where it would be erased to `Object` or if the + * typeparameters are removed. Both of these cases could cause surprising + * behavior for the users. + * * Steps taken: * * 1. `evalTypeApply` will establish the matrix and choose the appropriate @@ -128,6 +134,67 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => val selClassNonFinal = selClass && !(selector.typeSymbol is Final) val selFinalClass = selClass && (selector.typeSymbol is Final) + // Check if the selector's potential type parameters will be erased, and if so warn + val selTypeParam = tree.args.head.tpe.widen match { + case tp @ AppliedType(_, args @ (arg :: _)) => + // If the type is `Array[X]` where `X` is a primitive value + // class. In the future, when we have a solid implementation of + // Arrays of value classes, we might be able to relax this check. + val anyValArray = tp.isRef(defn.ArrayClass) && arg.typeSymbol.isPrimitiveValueClass + // param is: Any | AnyRef | java.lang.Object + val topType = defn.ObjectType <:< arg + // has @unchecked annotation to suppress warnings + val hasUncheckedAnnot = arg.hasAnnotation(defn.UncheckedAnnot) + + // Shouldn't warn when matching on a subclass with underscore + // params or type binding + val matchingUnderscoresOrTypeBindings = args.forall(_ match { + case tr: TypeRef => + tr.symbol.is(BindDefinedType) + case TypeBounds(lo, hi) => + (lo eq defn.NothingType) && (hi eq defn.AnyType) + case _ => false + }) && selector <:< scrutinee + + // we don't want to warn when matching on `List` from `Seq` e.g: + // (xs: Seq[Int]) match { case xs: List[Int] => ??? } + val matchingSeqToList = { + val hasSameTypeArgs = s.qualifier.tpe.widen match { + case AppliedType(_, scrutArg :: Nil) => + (scrutArg eq arg) || arg <:< scrutArg + case _ => false + } + + scrutinee.isRef(defn.SeqClass) && + tp.isRef(defn.ListClass) && + hasSameTypeArgs + } + + val shouldWarn = + !topType && !hasUncheckedAnnot && + !matchingUnderscoresOrTypeBindings && !matchingSeqToList && + !anyValArray + + if (shouldWarn) ctx.uncheckedWarning( + ErasedType(hl"""|Since type parameters are erased, you should not match on them in + |${"match"} expressions."""), + tree.pos + ) + true + case _ => + if (tree.args.head.symbol.is(TypeParam)) { + ctx.uncheckedWarning( + ErasedType( + hl"""|`${tree.args.head.tpe}` will be erased to `${selector}`. Which means that the specified + |behavior could be different during runtime.""" + ), + tree.pos + ) + true + } + else false + } + // Cases --------------------------------- val valueClassesOrAny = ValueClasses.isDerivedValueClass(scrutinee.typeSymbol) || @@ -148,7 +215,7 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => val inMatch = s.qualifier.symbol is Case - if (valueClassesOrAny) tree + if (selTypeParam || valueClassesOrAny) tree else if (knownStatically) handleStaticallyKnown(s, scrutinee, selector, inMatch, tree.pos) else if (falseIfUnrelated && scrutinee <:< selector) diff --git a/compiler/test/dotty/tools/dotc/repl/TestREPL.scala b/compiler/test/dotty/tools/dotc/repl/TestREPL.scala index c5557b86e985..2a3cfa568a82 100644 --- a/compiler/test/dotty/tools/dotc/repl/TestREPL.scala +++ b/compiler/test/dotty/tools/dotc/repl/TestREPL.scala @@ -10,7 +10,7 @@ import org.junit.Test /** A subclass of REPL used for testing. * It takes a transcript of a REPL session in `script`. The transcript - * starts with the first input prompt `scala> ` and ends with `scala> :quit` and a newline. + * starts with the first input prompt `scala> ` and ends with `scala> :quit`. * Invoking `process()` on the `TestREPL` runs all input lines and * collects then interleaved with REPL output in a string writer `out`. * Invoking `check()` checks that the collected output matches the original @@ -26,6 +26,7 @@ class TestREPL(script: String) extends REPL { override def context(ctx: Context) = { val fresh = ctx.fresh fresh.setSetting(ctx.settings.color, "never") + fresh.setSetting(ctx.settings.unchecked, true) fresh.setSetting(ctx.settings.classpath, Jars.dottyReplDeps.mkString(":")) fresh.initialize()(fresh) fresh diff --git a/tests/repl/erasure.check b/tests/repl/erasure.check new file mode 100644 index 000000000000..828aec884e25 --- /dev/null +++ b/tests/repl/erasure.check @@ -0,0 +1,64 @@ +scala> def unsafeCast[S](a: Any) = a match { case s: S => s; case _ => ??? } +-- [E048] Erased Type Unchecked Warning: ----------------------------- +4 |def unsafeCast[S](a: Any) = a match { case s: S => s; case _ => ??? } + | ^ + | abstract type pattern is unchecked since it is eliminated by erasure + +longer explanation available when compiling with `-explain` +def unsafeCast[S](a: Any): [S](a: Any)S +scala> unsafeCast[String](1) +java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String + at .(:6) + at .() + at RequestResult$.(:3) + at RequestResult$.() + at RequestResult$result() + at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) + at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.jav... +scala> class A; def unsafeCast2[S <: A](a: Any) = a match { case s: S => s; case _ => ??? } +-- [E048] Erased Type Unchecked Warning: ----------------------------- +4 |class A; def unsafeCast2[S <: A](a: Any) = a match { case s: S => s; case _ => ??? } + | ^ + | abstract type pattern is unchecked since it is eliminated by erasure + +longer explanation available when compiling with `-explain` +defined class A +def unsafeCast2[S <: A](a: Any): [S <: A](a: Any)S +scala> def matchArray1[A](xs: Array[A]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ??? } +def matchArray1[A](xs: Array[A]): [A](xs: Array[A])Array[Int] +scala> def matchArray2[A](xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ??? } +-- [E048] Erased Type Unchecked Warning: ----------------------------- +5 |def matchArray2[A](xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ??? } + | ^ + | abstract type pattern is unchecked since it is eliminated by erasure + +longer explanation available when compiling with `-explain` +def matchArray2[A](xs: Array[Any]): [A](xs: Array[Any])Array[Int] +scala> def matchArray3[A](xs: Array[A]) = xs match { case xs: Array[Int] => xs; case xs: Array[AnyRef] => ???; case xs: Array[Any] => ??? } +def matchArray3[A](xs: Array[A]): [A](xs: Array[A])Array[Int] +scala> def matchArray4(xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ???; case xs: Array[Any] => ??? } +-- [E048] Erased Type Unchecked Warning: ----------------------------- +5 |def matchArray4(xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A] => ???; case xs: Array[Any] => ??? } + | ^ + | abstract type pattern is unchecked since it is eliminated by erasure + +longer explanation available when compiling with `-explain` +def matchArray4(xs: Array[Any]): Array[Int] +scala> def matchArray5[A](xs: Array[Any]) = xs match { case xs: Array[Int] => xs; case xs: Array[A @unchecked] => ??? } +def matchArray5[A](xs: Array[Any]): [A](xs: Array[Any])Array[Int] +scala> def matchList1(xs: List[Any]) = xs match { case xs: List[Int @unchecked] => ??? } +def matchList1(xs: List[Any]): Nothing +scala> def matchList2(xs: List[Any]) = xs match { case List() => ???; case _ => ??? } +def matchList2(xs: List[Any]): Nothing +scala> def matchList3(xs: Seq[_]) = xs match { case List() => ???; case _ => ??? } +def matchList3(xs: Seq[_]): Nothing +scala> val xs: Seq[Int] = Seq(1,2,3) +val xs: scala.collection.Seq[Int] = List(1, 2, 3) +scala> val xsMatch = xs match { case ss: List[Int] => 1 } +val xsMatch: Int = 1 +scala> trait Foo[X]; trait Bar[X,Y] +defined trait Foo +defined trait Bar +scala> val underScoreMatch = (Nil: Any) match { case _: Foo[_] => ???; case _: Bar[_,_] => ???; case _ => 0 } +val underScoreMatch: Int = 0 +scala> :quit