Skip to content

Commit ca400bd

Browse files
EugeneFlesselleEugeneFlesselle
authored
Fix derivesFrom false negative in provablyDisjointClasses (#23834)
Fix derivesFrom false negative in provablyDisjointClasses Before the addition of `SymDenotation#mayDeriveFrom`, tests/neg/i17132.min.scala was unsoundly accepted without a type error because `R[T]` is reduced to `Any` where `T <: P[Any]` by testing `provablyDisjointClasses` before `P` is added as a baseClass of `Q`. Now, a recursion overflows occurs because: - reducing `R[T] := T match case Q[t] => R[t]; case ...` requires - proving `T` disjoint from `Q[t]` where `T <: P[Any]`, which asks - whether existsCommonBaseTypeWithDisjointArguments, which requires - normalizing the type arg to the base class P for the class Q, i.e. R[t] - ... In short, despite the use of the pending set in provablyDisjoint, diverging is still possible when there is "cycle" in the type arguments in the base classes, (and where the pending set is thus reset for a separate normalization problem). One could attempt to add some more logic to detect these loops s.t. they are considered "stuck", as opposed to reporting an error. But it isn't clear that there are any concrete motivations for this.
2 parents a78ae51 + 546fbd1 commit ca400bd

File tree

7 files changed

+73
-8
lines changed

7 files changed

+73
-8
lines changed

‎compiler/src/dotty/tools/backend/jvm/CodeGen.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ class CodeGen(val int: DottyBackendInterface, val primitives: DottyPrimitives)(
8585
case ex: InterruptedException => throw ex
8686
case ex: CompilationUnit.SuspendException => throw ex
8787
case ex: Throwable =>
88-
ex.printStackTrace()
89-
report.error(s"Error while emitting ${unit.source}\n${ex.getMessage}", NoSourcePosition)
88+
if !ex.isInstanceOf[TypeError] then ex.printStackTrace()
89+
report.error(s"Error while emitting ${unit.source}\n${ex.getMessage}", cd.sourcePos)
9090

9191

9292
def genTastyAndSetAttributes(claszSymbol: Symbol, store: ClassNode): Unit =

‎compiler/src/dotty/tools/dotc/core/SymDenotations.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,9 +865,23 @@ object SymDenotations {
865865
* and is the denoting symbol also different from `Null` or `Nothing`?
866866
* @note erroneous classes are assumed to derive from all other classes
867867
* and all classes derive from them.
868+
* @note may return a false negative when `this.info.isInstanceOf[TempClassInfo]`.
868869
*/
869870
def derivesFrom(base: Symbol)(using Context): Boolean = false
870871

872+
/** Could `this` derive from `base` now or in the future.
873+
* For concistency with derivesFrom, the info is only forced when this is a ClassDenotation.
874+
* If the info is a TempClassInfo then the baseClassSet may be temporarily approximated as empty.
875+
* This is problematic when stability of `!derivesFrom(base)` is assumed for soundness,
876+
* e.g., in `TypeComparer#provablyDisjointClasses`.
877+
* @note may return a false positive when `this.info.isInstanceOf[TempClassInfo]`.
878+
*/
879+
final def mayDeriveFrom(base: Symbol)(using Context): Boolean =
880+
this.isInstanceOf[ClassDenotation] && (info.isInstanceOf[TempClassInfo] || derivesFrom(base))
881+
882+
final def derivesFrom(base: Symbol, defaultIfUnknown: Boolean)(using Context): Boolean =
883+
if defaultIfUnknown/*== true*/ then mayDeriveFrom(base) else derivesFrom(base)
884+
871885
/** Is this a Scala or Java annotation ? */
872886
def isAnnotation(using Context): Boolean =
873887
isClass && (derivesFrom(defn.AnnotationClass) || is(JavaAnnotation))

‎compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3131,9 +3131,9 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
31313131
* unique value derives from the class.
31323132
*/
31333133
case (tp1: SingletonType, tp2) =>
3134-
!tp1.derivesFrom(tp2.classSymbol)
3134+
!tp1.derivesFrom(tp2.classSymbol, defaultIfUnknown = true)
31353135
case (tp1, tp2: SingletonType) =>
3136-
!tp2.derivesFrom(tp1.classSymbol)
3136+
!tp2.derivesFrom(tp1.classSymbol, defaultIfUnknown = true)
31373137

31383138
/* Now both sides are possibly-parameterized class types `p.C[Ts]` and `q.D[Us]`.
31393139
*
@@ -3189,7 +3189,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
31893189
val cls2BaseClassSet = SymDenotations.BaseClassSet(cls2.classDenot.baseClasses)
31903190
val commonBaseClasses = cls1.classDenot.baseClasses.filter(cls2BaseClassSet.contains(_))
31913191
def isAncestorOfOtherBaseClass(cls: ClassSymbol): Boolean =
3192-
commonBaseClasses.exists(other => (other ne cls) && other.derivesFrom(cls))
3192+
commonBaseClasses.exists(other => (other ne cls) && other.mayDeriveFrom(cls))
31933193
val result = commonBaseClasses.exists { baseClass =>
31943194
!isAncestorOfOtherBaseClass(baseClass) && isBaseTypeWithDisjointArguments(baseClass, innerPending)
31953195
}
@@ -3230,7 +3230,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
32303230
.filter(child => child.exists && child != cls)
32313231

32323232
def eitherDerivesFromOther(cls1: Symbol, cls2: Symbol): Boolean =
3233-
cls1.derivesFrom(cls2) || cls2.derivesFrom(cls1)
3233+
cls1.mayDeriveFrom(cls2) || cls2.mayDeriveFrom(cls1)
32343234

32353235
def smallestNonTraitBase(cls: Symbol): Symbol =
32363236
val classes = if cls.isClass then cls.asClass.baseClasses else cls.info.classSymbols

‎compiler/src/dotty/tools/dotc/core/Types.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,15 +270,15 @@ object Types extends TypeUtils {
270270
/** True if this type is an instance of the given `cls` or an instance of
271271
* a non-bottom subclass of `cls`.
272272
*/
273-
final def derivesFrom(cls: Symbol)(using Context): Boolean = {
273+
final def derivesFrom(cls: Symbol, defaultIfUnknown: Boolean = false)(using Context): Boolean = {
274274
def isLowerBottomType(tp: Type) =
275275
tp.isBottomType
276276
&& (tp.hasClassSymbol(defn.NothingClass)
277277
|| cls != defn.NothingClass && !cls.isValueClass)
278278
def loop(tp: Type): Boolean = try tp match
279279
case tp: TypeRef =>
280280
val sym = tp.symbol
281-
if (sym.isClass) sym.derivesFrom(cls) else loop(tp.superType)
281+
if (sym.isClass) sym.derivesFrom(cls, defaultIfUnknown) else loop(tp.superType)
282282
case tp: AppliedType =>
283283
tp.superType.derivesFrom(cls)
284284
case tp: MatchType =>

‎tests/neg/i17132.min.check

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
-- Error: tests/neg/i17132.min.scala:4:7 -------------------------------------------------------------------------------
2+
4 |class Q[T <: P[Any]] extends P[R[T]] // error
3+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4+
| Recursion limit exceeded.
5+
| Maybe there is an illegal cyclic reference?
6+
| If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
7+
| For the unprocessed stack trace, compile with -Xno-enrich-error-messages.
8+
| A recurring operation is (inner to outer):
9+
|
10+
| reduce type t match ...
11+
| reduce type t match ...
12+
| reduce type t match ...
13+
| reduce type t match ...
14+
| reduce type t match ...
15+
| reduce type t match ...
16+
| reduce type t match ...
17+
| reduce type t match ...
18+
| reduce type t match ...
19+
| reduce type t match ...
20+
| ...
21+
|
22+
| reduce type t match ...
23+
| reduce type t match ...
24+
| reduce type t match ...
25+
| reduce type t match ...
26+
| reduce type t match ...
27+
| reduce type t match ...
28+
| reduce type t match ...
29+
| reduce type t match ...
30+
| reduce type t match ...
31+
| reduce type T match ...

‎tests/neg/i17132.min.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
class P[T]
3+
//class Q[T] extends P[R[T]] // ok
4+
class Q[T <: P[Any]] extends P[R[T]] // error
5+
//type Q[T <: P[Any]] <: P[R[T]] // ok
6+
7+
type R[U] = U match
8+
case Q[t] => R[t]
9+
case P[t] => t

‎tests/neg/i17132.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
2+
sealed trait Transformation[T]
3+
4+
case object Count extends Transformation[Int]
5+
case class MultiTransformation[T1 <: Transformation[?], T2 <: Transformation[?]](t1: T1, t2: T2) // error cyclic
6+
extends Transformation[MultiTransformationResult[T1, T2]]
7+
8+
type MultiTransformationResult[T1 <: Transformation[?], T2 <: Transformation[?]] <: Tuple = (T1, T2) match {
9+
case (Transformation[t], MultiTransformation[t1, t2]) => t *: MultiTransformationResult[t1, t2]
10+
case (Transformation[t1], Transformation[t2]) => (t1, t2)
11+
}

0 commit comments

Comments
 (0)