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.

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

Expose reason for unchecked local class warning #16086

Merged
merged 2 commits into from
Oct 17, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
50 changes: 30 additions & 20 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,32 @@ object TypeTestsCasts {
import typer.Inferencing.maximizeType
import typer.ProtoTypes.constrained

/** Whether `(x: X).isInstanceOf[P]` can be checked at runtime?
/** Tests whether `(x: X).isInstanceOf[P]` is uncheckable at runtime, returning the reason,
* or the empty string if it is checkable.
*
* First do the following substitution:
* (a) replace `T @unchecked` and pattern binder types (e.g., `_$1`) in P with WildcardType
*
* Then check:
*
* 1. if `X <:< P`, TRUE
* 2. if `P` is a singleton type, TRUE
* 3. if `P` refers to an abstract type member or type parameter, FALSE
* 1. if `X <:< P`, ""
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefix this with a comment that the method returns a string explaining why the type is not checkable, or the empty string if it is checkable.

* 2. if `P` is a singleton type, ""
* 3. if `P` refers to an abstract type member or type parameter, "it refers to an abstract type member or type parameter"
* 4. if `P = Array[T]`, checkable(E, T) where `E` is the element type of `X`, defaults to `Any`.
* 5. if `P` is `pre.F[Ts]` and `pre.F` refers to a class which is not `Array`:
* (a) replace `Ts` with fresh type variables `Xs`
* (b) constrain `Xs` with `pre.F[Xs] <:< X`
* (c) maximize `pre.F[Xs]` and check `pre.F[Xs] <:< P`
* (c) maximize `pre.F[Xs]`
* (d) if !`pre.F[Xs] <:< P`, "its type arguments can't be determined from $X"
* 6. if `P = T1 | T2` or `P = T1 & T2`, checkable(X, T1) && checkable(X, T2).
* 7. if `P` is a refinement type, FALSE
* 8. if `P` is a local class which is not statically reachable from the scope where `X` is defined, FALSE
* 9. otherwise, TRUE
* 7. if `P` is a refinement type, "it's a refinement type"
* 8. if `P` is a local class which is not statically reachable from the scope where `X` is defined, "it's a local class"
* 9. otherwise, ""
*/
def checkable(X: Type, P: Type, span: Span)(using Context): Boolean = atPhase(Phases.refchecksPhase.next) {
def whyUncheckable(X: Type, P: Type, span: Span)(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

// Run just before ElimOpaque transform (which follows RefChecks)
def isAbstract(P: Type) = !P.dealias.typeSymbol.isClass

Expand Down Expand Up @@ -124,10 +129,10 @@ object TypeTestsCasts {

}

def recur(X: Type, P: Type): Boolean = (X <:< P) || (P.dealias match {
case _: SingletonType => true
def recur(X: Type, P: Type): String = (X <:< P) ||| (P.dealias match {
case _: SingletonType => ""
case _: TypeProxy
if isAbstract(P) => false
if isAbstract(P) => i"it refers to an abstract type member or type parameter"
case defn.ArrayOf(tpT) =>
X match {
case defn.ArrayOf(tpE) => recur(tpE, tpT)
Expand All @@ -147,21 +152,23 @@ object TypeTestsCasts {
X.classSymbol.exists && P.classSymbol.exists &&
!X.classSymbol.asClass.mayHaveCommonChild(P.classSymbol.asClass)
|| typeArgsTrivial(X, tpe)
||| i"its type arguments can't be determined from $X"
}
case AndType(tp1, tp2) => recur(X, tp1) && recur(X, tp2)
case OrType(tp1, tp2) => recur(X, tp1) && recur(X, tp2)
case AnnotatedType(t, _) => recur(X, t)
case tp2: RefinedType => recur(X, tp2.parent) && TypeComparer.hasMatchingMember(tp2.refinedName, X, tp2)
case tp2: RefinedType => recur(X, tp2.parent)
&& (TypeComparer.hasMatchingMember(tp2.refinedName, X, tp2) ||| i"it's a refinement type")
case tp2: RecType => recur(X, tp2.parent)
case _
if P.classSymbol.isLocal && foundClasses(X).exists(P.classSymbol.isInaccessibleChildOf) => // 8
false
case _ => true
i"it's a local class"
case _ => ""
})

val res = X.widenTermRefExpr.hasAnnotation(defn.UncheckedAnnot) || recur(X.widen, replaceP(P))
val res = recur(X.widen, replaceP(P))

debug.println(i"checking ${X.show} isInstanceOf ${P} = $res")
debug.println(i"checking $X isInstanceOf $P = $res")

res
}
Expand Down Expand Up @@ -348,9 +355,12 @@ object TypeTestsCasts {
if (sym.isTypeTest) {
val argType = tree.args.head.tpe
val isTrusted = tree.hasAttachment(PatternMatcher.TrustedTypeTestKey)
if (!isTrusted && !checkable(expr.tpe, argType, tree.span))
report.uncheckedWarning(i"the type test for $argType cannot be checked at runtime", expr.srcPos)
transformTypeTest(expr, tree.args.head.tpe,
val isUnchecked = expr.tpe.widenTermRefExpr.hasAnnotation(defn.UncheckedAnnot)
if !isTrusted && !isUnchecked then
val whyNot = whyUncheckable(expr.tpe, argType, tree.span)
if whyNot.nonEmpty then
report.uncheckedWarning(i"the type test for $argType cannot be checked at runtime because $whyNot", expr.srcPos)
transformTypeTest(expr, argType,
flagUnrelated = enclosingInlineds.isEmpty) // if test comes from inlined code, dont't flag it even if it always false
}
else if (sym.isTypeCast)
Expand Down
4 changes: 2 additions & 2 deletions tests/neg-custom-args/fatal-warnings/i12253.check
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
-- Error: tests/neg-custom-args/fatal-warnings/i12253.scala:11:10 ------------------------------------------------------
11 | case extractors.InlinedLambda(_, Select(_, name)) => Expr(name) // error // error
| ^
| the type test for extractors.q2.reflect.Term cannot be checked at runtime
|the type test for extractors.q2.reflect.Term cannot be checked at runtime because it refers to an abstract type member or type parameter
-- Error: tests/neg-custom-args/fatal-warnings/i12253.scala:11:38 ------------------------------------------------------
11 | case extractors.InlinedLambda(_, Select(_, name)) => Expr(name) // error // error
| ^
| the type test for q1.reflect.Select cannot be checked at runtime
|the type test for q1.reflect.Select cannot be checked at runtime because it refers to an abstract type member or type parameter
there was 1 deprecation warning; re-run with -deprecation for details
2 changes: 1 addition & 1 deletion tests/neg-custom-args/nowarn/nowarn.check
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Matching filters for @nowarn or -Wconf:
-- Unchecked Warning: tests/neg-custom-args/nowarn/nowarn.scala:53:7 ---------------------------------------------------
53 | case _: List[Int] => 0 // warning (patmat, unchecked)
| ^
| the type test for List[Int] cannot be checked at runtime
|the type test for List[Int] cannot be checked at runtime because its type arguments can't be determined from Any
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:31:1 ---------------------------------------------------------------
31 |@nowarn("id=1") def t4d = try 1 // error and warning (unused nowarn, wrong id)
|^^^^^^^^^^^^^^^
Expand Down
4 changes: 4 additions & 0 deletions tests/neg/15981.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- Error: tests/neg/15981.scala:4:45 -----------------------------------------------------------------------------------
4 | override def equals(any: Any): Boolean = any.isInstanceOf[PosInt] // error
| ^^^
| the type test for PosInt cannot be checked at runtime because it's a local class
6 changes: 6 additions & 0 deletions tests/neg/15981.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// scalac: -Werror
val _ = locally{
sealed abstract class PosInt(val value: Int) {
override def equals(any: Any): Boolean = any.isInstanceOf[PosInt] // error
}
}
14 changes: 7 additions & 7 deletions tests/neg/i4812.check
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
-- Error: tests/neg/i4812.scala:8:11 -----------------------------------------------------------------------------------
8 | case prev: A => // error: the type test for A cannot be checked at runtime
| ^
| the type test for A cannot be checked at runtime
| the type test for A cannot be checked at runtime because it's a local class
-- Error: tests/neg/i4812.scala:18:11 ----------------------------------------------------------------------------------
18 | case prev: A => // error: the type test for A cannot be checked at runtime
| ^
| the type test for A cannot be checked at runtime
| the type test for A cannot be checked at runtime because it's a local class
-- Error: tests/neg/i4812.scala:28:11 ----------------------------------------------------------------------------------
28 | case prev: A => // error: the type test for A cannot be checked at runtime
| ^
| the type test for A cannot be checked at runtime
| the type test for A cannot be checked at runtime because it's a local class
-- Error: tests/neg/i4812.scala:38:11 ----------------------------------------------------------------------------------
38 | case prev: A => // error: the type test for A cannot be checked at runtime
| ^
| the type test for A cannot be checked at runtime
| the type test for A cannot be checked at runtime because it's a local class
-- Error: tests/neg/i4812.scala:50:13 ----------------------------------------------------------------------------------
50 | case prev: A => // error: the type test for A cannot be checked at runtime
| ^
| the type test for A cannot be checked at runtime
| the type test for A cannot be checked at runtime because it's a local class
-- Error: tests/neg/i4812.scala:60:11 ----------------------------------------------------------------------------------
60 | case prev: A => // error: the type test for A cannot be checked at runtime
| ^
| the type test for A cannot be checked at runtime
| the type test for A cannot be checked at runtime because it's a local class
-- Error: tests/neg/i4812.scala:96:11 ----------------------------------------------------------------------------------
96 | case x: B => // error: the type test for B cannot be checked at runtime
| ^
| the type test for B cannot be checked at runtime
| the type test for B cannot be checked at runtime because it's a local class