Skip to content

Commit

Permalink
Merge pull request #14715 from dotty-staging/fix-14705-v2
Browse files Browse the repository at this point in the history
Don't optimize explicitly written isInstanceOf tests away.
  • Loading branch information
odersky authored Mar 21, 2022
2 parents 0aafca1 + 9648c63 commit 0b4e96c
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID]:
DoubleDefinitionID,
MatchCaseOnlyNullWarningID,
ImportRenamedTwiceID,
TypeTestAlwaysSucceedsID,
TypeTestAlwaysDivergesID,
TermMemberNeedsNeedsResultTypeForImplicitSearchID,
ClassCannotExtendEnumID,
ValueClassParameterMayNotBeCallByNameID,
Expand Down
10 changes: 3 additions & 7 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2177,13 +2177,9 @@ import transform.SymUtils._
def explain = ""
}

class TypeTestAlwaysSucceeds(scrutTp: Type, testTp: Type)(using Context) extends SyntaxMsg(TypeTestAlwaysSucceedsID) {
def msg = {
val addendum =
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 ${scrutTp.show}" + addendum
}
class TypeTestAlwaysDiverges(scrutTp: Type, testTp: Type)(using Context) extends SyntaxMsg(TypeTestAlwaysDivergesID) {
def msg =
s"This type test will never return a result since the scrutinee type ${scrutTp.show} does not contain any value."
def explain = ""
}

Expand Down
31 changes: 20 additions & 11 deletions compiler/src/dotty/tools/dotc/transform/TailRec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,16 @@ package transform

import ast.{TreeTypeMap, tpd}
import config.Printers.tailrec
import core.Contexts._
import core.Constants.Constant
import core.Flags._
import core.NameKinds.{TailLabelName, TailLocalName, TailTempName}
import core.StdNames.nme
import core.Symbols._
import reporting._
import core.*
import Contexts.*, Flags.*, Symbols.*
import Constants.Constant
import NameKinds.{TailLabelName, TailLocalName, TailTempName}
import StdNames.nme
import reporting.*
import transform.MegaPhase.MiniPhase
import util.LinearSet
import dotty.tools.uncheckedNN


/** A Tail Rec Transformer.
*
* What it does:
Expand Down Expand Up @@ -161,15 +159,26 @@ class TailRec extends MiniPhase {
val rhsFullyTransformed = varForRewrittenThis match {
case Some(localThisSym) =>
val thisRef = localThisSym.termRef
new TreeTypeMap(
val substitute = new TreeTypeMap(
typeMap = _.substThisUnlessStatic(enclosingClass, thisRef)
.subst(rewrittenParamSyms, varsForRewrittenParamSyms.map(_.termRef)),
treeMap = {
case tree: This if tree.symbol == enclosingClass => Ident(thisRef)
case tree => tree
}
).transform(rhsSemiTransformed)

)
// The previous map will map `This` references to `Ident`s even under `Super`.
// This violates super's contract. We fix this by cleaning up `Ident`s under
// super, mapping them back to the original `This` reference. This is not
// very elegant, but I did not manage to find a cleaner way to handle this.
// See pos/tailrec-super.scala for a test case.
val cleanup = new TreeMap:
override def transform(t: Tree)(using Context) = t match
case Super(qual: Ident, mix) if !qual.tpe.isInstanceOf[Types.ThisType] =>
cpy.Super(t)(This(enclosingClass), mix)
case _ =>
super.transform(t)
cleanup.transform(substitute.transform(rhsSemiTransformed))
case none =>
new TreeTypeMap(
typeMap = _.subst(rewrittenParamSyms, varsForRewrittenParamSyms.map(_.termRef))
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ class TreeChecker extends Phase with SymTransformer {
}

override def typedSuper(tree: untpd.Super, pt: Type)(using Context): Tree =
assert(tree.qual.tpe.isInstanceOf[ThisType], i"expect prefix of Super to be This, actual = ${tree.qual}")
assert(tree.qual.typeOpt.isInstanceOf[ThisType], i"expect prefix of Super to be This, actual = ${tree.qual}")
super.typedSuper(tree, pt)

override def typedTyped(tree: untpd.Typed, pt: Type)(using Context): Tree =
Expand Down
9 changes: 4 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,12 @@ object TypeTestsCasts {
else foundClasses.exists(check)
end checkSensical

if (expr.tpe <:< testType)
if (expr.tpe.isNotNull) {
if (!inMatch) report.warning(TypeTestAlwaysSucceeds(expr.tpe, testType), tree.srcPos)
constant(expr, Literal(Constant(true)))
}
if (expr.tpe <:< testType) && inMatch then
if expr.tpe.isNotNull then constant(expr, Literal(Constant(true)))
else expr.testNotNull
else {
if expr.tpe.isBottomType then
report.warning(TypeTestAlwaysDiverges(expr.tpe, testType), tree.srcPos)
val nestedCtx = ctx.fresh.setNewTyperState()
val foundClsSyms = foundClasses(expr.tpe.widen, Nil)
val sensical = checkSensical(foundClsSyms)(using nestedCtx)
Expand Down
3 changes: 3 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i14705.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
val n = Nil
val b = n.head.isInstanceOf[String] // error

17 changes: 17 additions & 0 deletions tests/pos/tailrec-super.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
class Tree
case class Inlined(call: Tree, bindings: List[String], expr: Tree) extends Tree
case object EmptyTree extends Tree
class Context

class Transform:
def transform(tree: Tree)(using Context): Tree = tree

class Inliner:
var enclosingInlineds: List[String] = Nil
private def expandMacro(using Context) =
val inlinedNormalizer = new Transform:
override def transform(tree: Tree)(using Context) = tree match
case Inlined(EmptyTree, Nil, expr) if enclosingInlineds.isEmpty => transform(expr)
case _ => super.transform(tree)

object Inliner
19 changes: 19 additions & 0 deletions tests/run/i14705.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
trait Fruit
case class Apple() extends Fruit
case class Orange() extends Fruit

case class Box[C](fruit: C) extends Fruit

val apple = Box(fruit = Apple())
val orange = Box(fruit = Orange())


val result = List(apple, orange).map {
case appleBox: Box[Apple] @unchecked if appleBox.fruit.isInstanceOf[Apple] => //contains apple
"apple"
case _ =>
"orange"
}

@main def Test =
assert(result == List("apple", "orange"))

0 comments on commit 0b4e96c

Please sign in to comment.