Skip to content

Commit de789c3

Browse files
committed
Fix #9000: Avoid spurious cyclic errors for typelevel matches
I still don't know why the match type ``` type A2B[Xs <: Tuple] <: Tuple = Xs match case Unit => Unit case a *: as => B *: A2B[as] ``` compiled OK if it appeared in a user defined object, but failed with a cyclic reference when written on the top level. But the logic for cyclic detection was clearly very fragile, and unadapted to the case at hand. Essentially, it's a type map that inserts LazyRefs when it detects legal cycles. The problem is that these insertions cause recomputations of applied types in the TypeMap. And these fail since the type constructor (A2B in this case) has not yet been completed. So after a LazyRef was inserted, the cycle checker fails again in an unrecoverable way because it tries to get the type parameters of an uncompleted type constructor. So the question would be rather - why did this work if the match type appears in a user-defined object? I could not answer that question, but I could fix the logic to handle these cases.
1 parent aa1c363 commit de789c3

File tree

4 files changed

+64
-16
lines changed

4 files changed

+64
-16
lines changed

compiler/src/dotty/tools/dotc/core/Substituters.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,11 @@ trait Substituters { this: Context =>
157157
.mapOver(tp)
158158
}
159159

160-
final class SubstBindingMap(from: BindingType, to: BindingType) extends DeepTypeMap {
160+
/** SubstBinding is called from checkNotCyclic at a point where the
161+
* type constructor of an applied type might not be completed yet.
162+
* Therefore it needs to be a SimpleMap. Test case is toplevel-match.scala
163+
*/
164+
final class SubstBindingMap(from: BindingType, to: BindingType) extends DeepTypeMap, SimpleTypeMap {
161165
def apply(tp: Type): Type = subst(tp, from, to, this)
162166
}
163167

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

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4888,6 +4888,19 @@ object Types {
48884888
protected def derivedLambdaType(tp: LambdaType)(formals: List[tp.PInfo], restpe: Type): Type =
48894889
tp.derivedLambdaType(tp.paramNames, formals, restpe)
48904890

4891+
protected def mapAppliedType(tp: AppliedType): Type =
4892+
def mapArgs(args: List[Type], tparams: List[ParamInfo]): List[Type] = args match
4893+
case arg :: otherArgs if tparams.nonEmpty =>
4894+
val arg1 = arg match
4895+
case arg: TypeBounds => this(arg)
4896+
case arg => atVariance(variance * tparams.head.paramVarianceSign)(this(arg))
4897+
val otherArgs1 = mapArgs(otherArgs, tparams.tail)
4898+
if ((arg1 eq arg) && (otherArgs1 eq otherArgs)) args
4899+
else arg1 :: otherArgs1
4900+
case nil =>
4901+
nil
4902+
derivedAppliedType(tp, this(tp.tycon), mapArgs(tp.args, tp.tyconTypeParams))
4903+
48914904
/** Map this function over given type */
48924905
def mapOver(tp: Type): Type = {
48934906
record(s"mapOver ${getClass}")
@@ -4911,19 +4924,7 @@ object Types {
49114924
| NoPrefix => tp
49124925

49134926
case tp: AppliedType =>
4914-
def mapArgs(args: List[Type], tparams: List[ParamInfo]): List[Type] = args match {
4915-
case arg :: otherArgs if tparams.nonEmpty =>
4916-
val arg1 = arg match {
4917-
case arg: TypeBounds => this(arg)
4918-
case arg => atVariance(variance * tparams.head.paramVarianceSign)(this(arg))
4919-
}
4920-
val otherArgs1 = mapArgs(otherArgs, tparams.tail)
4921-
if ((arg1 eq arg) && (otherArgs1 eq otherArgs)) args
4922-
else arg1 :: otherArgs1
4923-
case nil =>
4924-
nil
4925-
}
4926-
derivedAppliedType(tp, this(tp.tycon), mapArgs(tp.args, tp.tyconTypeParams))
4927+
mapAppliedType(tp)
49274928

49284929
case tp: RefinedType =>
49294930
derivedRefinedType(tp, this(tp.parent), this(tp.refinedInfo))
@@ -5037,6 +5038,31 @@ object Types {
50375038
}
50385039
}
50395040

5041+
/** A type map with simplified handling of applied types. Arguments are mapped
5042+
* without regard to variance and the applied type is reconsitituted without
5043+
* going through derivedAppliedType. This could have performance benefits, and
5044+
* it also does not force the type constructor's info. The second aspect is
5045+
* needed when checking for cycles in Namer, since the type constructor is
5046+
* not completed yet at this point.
5047+
*/
5048+
trait SimpleTypeMap extends TypeMap:
5049+
5050+
override protected def mapAppliedType(tp: AppliedType): Type =
5051+
def mapArgs(args: List[Type]): List[Type] = args match
5052+
case arg :: otherArgs =>
5053+
val arg1 = this(arg)
5054+
val otherArgs1 = mapArgs(otherArgs)
5055+
if ((arg1 eq arg) && (otherArgs1 eq otherArgs)) args
5056+
else arg1 :: otherArgs1
5057+
case nil =>
5058+
nil
5059+
val tycon1 = this(tp.tycon)
5060+
val args1 = mapArgs(tp.args)
5061+
if (tycon1 eq tp.tycon) && (args1 eq tp.args) then tp
5062+
else AppliedType(tycon1, args1)
5063+
5064+
end SimpleTypeMap
5065+
50405066
/** A type map that maps also parents and self type of a ClassInfo */
50415067
abstract class DeepTypeMap(implicit ctx: Context) extends TypeMap {
50425068
override def mapClassInfo(tp: ClassInfo): ClassInfo = {

compiler/src/dotty/tools/dotc/typer/Checking.scala

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,11 @@ object Checking {
266266
this(tp.info)
267267
mapOver(tp)
268268
case tp @ AppliedType(tycon, args) =>
269-
tp.derivedAppliedType(this(tycon), args.mapConserve(this(_, nestedCycleOK, nestedCycleOK)))
269+
val tycon1 = this(tycon)
270+
val args1 = args.mapConserve(this(_, nestedCycleOK, nestedCycleOK))
271+
if (tycon1 eq tycon) && (args1 eq args) then tp
272+
else AppliedType(tycon1, args1)
273+
// don't go through derivedAppliedType sine tycon might not be completed yet
270274
case tp @ RefinedType(parent, name, rinfo) =>
271275
tp.derivedRefinedType(this(parent), name, this(rinfo, nestedCycleOK, nestedCycleOK))
272276
case tp: RecType =>
@@ -334,7 +338,7 @@ object Checking {
334338

335339
/** Check that `info` of symbol `sym` is not cyclic.
336340
* @pre sym is not yet initialized (i.e. its type is a Completer).
337-
* @return `info` where every legal F-bounded reference is proctected
341+
* @return `info` where every legal F-bounded reference is protected
338342
* by a `LazyRef`, or `ErrorType` if a cycle was detected and reported.
339343
*/
340344
def checkNonCyclic(sym: Symbol, info: Type, reportErrors: Boolean)(using Context): Type = {

tests/pos/toplevel-match.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
class A
2+
class B
3+
4+
trait HList
5+
class HCons[A, As <: HList] extends HList
6+
class HNil[A] extends HList
7+
8+
type AtoB[Xs <: HList] <: HList = Xs match
9+
case HNil[a] => HNil[B]
10+
case HCons[a, as] => HCons[B, AtoB[as]]
11+
12+
type A2B[Xs <: Tuple] <: Tuple = Xs match
13+
case Unit => Unit
14+
case a *: as => B *: A2B[as]

0 commit comments

Comments
 (0)