Skip to content
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.

Sign up for GitHub

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

Fix #22051: only trust the type application part for case class unapplies #22099

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix #22051: only trust the type application part for case class unapplies #22099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67024a9
Only trust the type application part for case class unapplies
noti0na1 Dec 3, 2024
a7b23d6
Move test to warn
noti0na1 Dec 3, 2024
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
11 changes: 5 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ object TypeTestsCasts {
* 9. if `X` is `T1 | T2`, checkable(T1, P) && checkable(T2, P).
* 10. otherwise, ""
*/
def whyUncheckable(X: Type, P: Type, span: Span)(using Context): String = atPhase(Phases.refchecksPhase.next) {
def whyUncheckable(X: Type, P: Type, span: Span, trustTypeApplication: Boolean)(using Context): String = atPhase(Phases.refchecksPhase.next) {
extension (inline s1: String) inline def &&(inline s2: String): String = if s1 == "" then s2 else s1
extension (inline b: Boolean) inline def |||(inline s: String): String = if b then "" else s

Expand Down Expand Up @@ -143,7 +143,7 @@ object TypeTestsCasts {
case defn.ArrayOf(tpE) => recur(tpE, tpT)
case _ => recur(defn.AnyType, tpT)
}
case tpe @ AppliedType(tycon, targs) =>
case tpe @ AppliedType(tycon, targs) if !trustTypeApplication =>
X.widenDealias match {
case OrType(tp1, tp2) =>
// This case is required to retrofit type inference,
Expand Down Expand Up @@ -366,8 +366,7 @@ object TypeTestsCasts {
if (sym.isTypeTest) {
val argType = tree.args.head.tpe
val isTrusted = tree.hasAttachment(PatternMatcher.TrustedTypeTestKey)
if !isTrusted then
checkTypePattern(expr.tpe, argType, expr.srcPos)
checkTypePattern(expr.tpe, argType, expr.srcPos, isTrusted)
transformTypeTest(expr, argType,
flagUnrelated = enclosingInlineds.isEmpty) // if test comes from inlined code, dont't flag it even if it always false
}
Expand All @@ -392,10 +391,10 @@ object TypeTestsCasts {
def checkBind(tree: Bind)(using Context) =
checkTypePattern(defn.ThrowableType, tree.body.tpe, tree.srcPos)

private def checkTypePattern(exprTpe: Type, castTpe: Type, pos: SrcPos)(using Context) =
private def checkTypePattern(exprTpe: Type, castTpe: Type, pos: SrcPos, trustTypeApplication: Boolean = false)(using Context) =
val isUnchecked = exprTpe.widenTermRefExpr.hasAnnotation(defn.UncheckedAnnot)
if !isUnchecked then
val whyNot = whyUncheckable(exprTpe, castTpe, pos.span)
val whyNot = whyUncheckable(exprTpe, castTpe, pos.span, trustTypeApplication)
if whyNot.nonEmpty then
report.uncheckedWarning(UncheckedTypePattern(castTpe, whyNot), pos)

Expand Down
17 changes: 17 additions & 0 deletions tests/warn/i22051.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
def boundary[T](body: (T => RuntimeException) => T): T =
case class Break(value: T) extends RuntimeException
try body(Break.apply)
catch case Break(t) => t // warn: pattern matching on local classes is unsound currently

def test =
boundary[Int]: EInt =>
val v: String = boundary[String]: EString =>
throw EInt(3)
v.length // a runtime error: java.lang.ClassCastException

def boundaryCorrectBehaviour[T](body: (T => RuntimeException) => T): T =
object local:
// A correct implementation, but is still treated as a local class right now
case class Break(value: T) extends RuntimeException
try body(local.Break.apply)
catch case local.Break(t) => t // warn
Loading