Skip to content

Fix #8681: handle scrutinee of union types #8722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2062,12 +2062,12 @@ object messages {
def explain = ""
}

class TypeTestAlwaysSucceeds(foundCls: Symbol, testCls: Symbol)(implicit ctx: Context) extends SyntaxMsg(TypeTestAlwaysSucceedsID) {
class TypeTestAlwaysSucceeds(scrutTp: Type, testTp: Type)(implicit ctx: Context) extends SyntaxMsg(TypeTestAlwaysSucceedsID) {
def msg = {
val addendum =
if (foundCls != testCls) s" is a subtype of $testCls"
if (scrutTp != testTp) s" is a subtype of ${testTp.show}"
else " is the same as the tested type"
s"The highlighted type test will always succeed since the scrutinee type ($foundCls)" + addendum
s"The highlighted type test will always succeed since the scrutinee type ($scrutTp.show)" + addendum
}
def explain = ""
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ object Erasure {
}

override def typedTypeApply(tree: untpd.TypeApply, pt: Type)(using Context): Tree = {
val ntree = interceptTypeApply(tree.asInstanceOf[TypeApply])(ctx.withPhase(ctx.erasurePhase)).withSpan(tree.span)
val ntree = interceptTypeApply(tree.asInstanceOf[TypeApply])(using ctx.withPhase(ctx.erasurePhase)).withSpan(tree.span)

ntree match {
case TypeApply(fun, args) =>
Expand Down
53 changes: 33 additions & 20 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ object TypeTestsCasts {
* 7. if `P` is a refinement type, FALSE
* 8. otherwise, TRUE
*/
def checkable(X: Type, P: Type, span: Span)(implicit ctx: Context): Boolean = {
def checkable(X: Type, P: Type, span: Span)(using Context): Boolean = {
def isAbstract(P: Type) = !P.dealias.typeSymbol.isClass
def isPatternTypeSymbol(sym: Symbol) = !sym.isClass && sym.is(Case)

Expand Down Expand Up @@ -155,7 +155,7 @@ object TypeTestsCasts {
res
}

def interceptTypeApply(tree: TypeApply)(implicit ctx: Context): Tree = trace(s"transforming ${tree.show}", show = true) {
def interceptTypeApply(tree: TypeApply)(using Context): Tree = trace(s"transforming ${tree.show}", show = true) {
/** Intercept `expr.xyz[XYZ]` */
def interceptWith(expr: Tree): Tree =
if (expr.isEmpty) tree
Expand All @@ -172,7 +172,9 @@ object TypeTestsCasts {
else if tp.isRef(defn.AnyValClass) then defn.AnyClass
else tp.classSymbol

def foundCls = effectiveClass(expr.tpe.widen)
def foundClasses(tp: Type, acc: List[Symbol]): List[Symbol] = tp match
case OrType(tp1, tp2) => foundClasses(tp2, foundClasses(tp1, acc))
case _ => effectiveClass(tp) :: acc

def inMatch =
tree.fun.symbol == defn.Any_typeTest || // new scheme
Expand All @@ -181,7 +183,7 @@ object TypeTestsCasts {
def transformIsInstanceOf(expr: Tree, testType: Type, flagUnrelated: Boolean): Tree = {
def testCls = effectiveClass(testType.widen)

def unreachable(why: => String): Boolean = {
def unreachable(why: => String)(using ctx: Context): Boolean = {
if (flagUnrelated)
if (inMatch) ctx.error(em"this case is unreachable since $why", expr.sourcePos)
else ctx.warning(em"this will always yield false since $why", expr.sourcePos)
Expand All @@ -191,7 +193,7 @@ object TypeTestsCasts {
/** Are `foundCls` and `testCls` classes that allow checks
* whether a test would be always false?
*/
def isCheckable =
def isCheckable(foundCls: Symbol) =
foundCls.isClass && testCls.isClass &&
!(testCls.isPrimitiveValueClass && !foundCls.isPrimitiveValueClass) &&
// if `test` is primitive but `found` is not, we might have a case like
Expand All @@ -203,8 +205,8 @@ object TypeTestsCasts {
/** Check whether a runtime test that a value of `foundCls` can be a `testCls`
* can be true in some cases. Issues a warning or an error otherwise.
*/
def checkSensical: Boolean =
if (!isCheckable) true
def checkSensical(foundCls: Symbol)(using Context): Boolean =
if (!isCheckable(foundCls)) true
else if (foundCls.isPrimitiveValueClass && !testCls.isPrimitiveValueClass) {
ctx.error("cannot test if value types are references", tree.sourcePos)
false
Expand All @@ -214,38 +216,49 @@ object TypeTestsCasts {
testCls.is(Final) || !testCls.is(Trait) && !foundCls.is(Trait)
)
if (foundCls.is(Final))
unreachable(i"$foundCls is not a subclass of $testCls")
unreachable(i"type ${expr.tpe.widen} is not a subclass of $testCls")
else if (unrelated)
unreachable(i"$foundCls and $testCls are unrelated")
unreachable(i"type ${expr.tpe.widen} and $testCls are unrelated")
else true
}
else true

if (expr.tpe <:< testType)
if (expr.tpe.isNotNull) {
if (!inMatch) ctx.warning(TypeTestAlwaysSucceeds(foundCls, testCls), tree.sourcePos)
if (!inMatch) ctx.warning(TypeTestAlwaysSucceeds(expr.tpe, testType), tree.sourcePos)
constant(expr, Literal(Constant(true)))
}
else expr.testNotNull
else if (!checkSensical)
constant(expr, Literal(Constant(false)))
else if (testCls.isPrimitiveValueClass)
if (foundCls.isPrimitiveValueClass)
constant(expr, Literal(Constant(foundCls == testCls)))
else {
val nestedCtx = ctx.fresh.setNewTyperState()
val foundClsSyms = foundClasses(expr.tpe.widen, Nil)
val sensical = foundClsSyms.exists(sym => checkSensical(sym)(using nestedCtx))
if (!sensical) {
nestedCtx.typerState.commit()
constant(expr, Literal(Constant(false)))
}
else if (testCls.isPrimitiveValueClass )
if (foundClsSyms.size == 1 && foundClsSyms.head.isPrimitiveValueClass)
constant(expr, Literal(Constant(foundClsSyms.head == testCls)))
else
transformIsInstanceOf(expr, defn.boxedType(testCls.typeRef), flagUnrelated)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be nicer as a pattern match foundClsSyms match { case List(foundCls) => ... case _ => ... }

else
transformIsInstanceOf(expr, defn.boxedType(testCls.typeRef), flagUnrelated)
else
derivedTree(expr, defn.Any_isInstanceOf, testType)
derivedTree(expr, defn.Any_isInstanceOf, testType)
}
}

def transformAsInstanceOf(testType: Type): Tree = {
def testCls = testType.widen.classSymbol
def testCls = effectiveClass(testType.widen)
def foundClsSymPrimitive = {
val foundClsSyms = foundClasses(expr.tpe.widen, Nil)
foundClsSyms.size == 1 && foundClsSyms.head.isPrimitiveValueClass
}
if (erasure(expr.tpe) <:< testType)
Typed(expr, tree.args.head) // Replace cast by type ascription (which does not generate any bytecode)
else if (testCls eq defn.BoxedUnitClass)
// as a special case, casting to Unit always successfully returns Unit
Block(expr :: Nil, Literal(Constant(()))).withSpan(expr.span)
else if (foundCls.isPrimitiveValueClass)
else if (foundClsSymPrimitive)
if (testCls.isPrimitiveValueClass) primitiveConversion(expr, testCls)
else derivedTree(box(expr), defn.Any_asInstanceOf, testType)
else if (testCls.isPrimitiveValueClass)
Expand Down
14 changes: 14 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i8681.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
case class A(a: Int)
case class B(b: Int)
case class C(c: Int)

val a = (A(1): A) match {
case A(_) => "OK"
case B(_) => "NOT OK" // error: this case is unreachable since class A and class B are unrelated
}

val b = (A(1): A | B) match {
case A(_) => "OK"
case B(_) => "OK"
case C(_) => "NOT OK" // error
}
8 changes: 8 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i8711.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Error: tests/neg-custom-args/fatal-warnings/i8711.scala:7:9 ---------------------------------------------------------
7 | case x: B => x // error: this case is unreachable since class A is not a subclass of class B
| ^
| this case is unreachable since type A and class B are unrelated
-- Error: tests/neg-custom-args/fatal-warnings/i8711.scala:12:9 --------------------------------------------------------
12 | case x: C => x // error
| ^
| this case is unreachable since type A | B and class C are unrelated
15 changes: 15 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i8711.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class A
class B
class C

object Test {
def foo(x: A) = x match {
case x: B => x // error: this case is unreachable since class A is not a subclass of class B
case _ =>
}

def bar(x: A | B) = x match {
case x: C => x // error
case _ =>
}
}