Skip to content

Commit ded5d25

Browse files
committed
Change logic to find members of recursive types
The previous logic used two boolean variables to selectively create defensive copies. It was quite complicated. The new logic is simpler and copies less. - It copies only if the same recursive type is accessed with two different prefixes. As long as the prefix stays the same,no confusion in the `substRecThis` is possible. - It avoids the openedTwice logic that causes defensive copies at all points in the future. It seems that trick is no longer necessary. Fixes #17380 by avoiding infinite recursion due to defensive copies.
1 parent 2a1e3de commit ded5d25

File tree

2 files changed

+16
-23
lines changed

2 files changed

+16
-23
lines changed

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

+13-23
Original file line numberDiff line numberDiff line change
@@ -735,33 +735,24 @@ object Types {
735735
// TODO: change tp.parent to nullable or other values
736736
if ((tp.parent: Type | Null) == null) NoDenotation
737737
else if (tp eq pre) go(tp.parent)
738-
else {
738+
else
739739
//println(s"find member $pre . $name in $tp")
740740

741741
// We have to be careful because we might open the same (wrt eq) recursive type
742-
// twice during findMember which risks picking the wrong prefix in the `substRecThis(rt, pre)`
743-
// call below. To avoid this problem we do a defensive copy of the recursive
744-
// type first. But if we do this always we risk being inefficient and we ran into
745-
// stackoverflows when compiling pos/hk.scala under the refinement encoding
746-
// of hk-types. So we only do a copy if the type
747-
// is visited again in a recursive call to `findMember`, as tracked by `tp.opened`.
748-
// Furthermore, if this happens we mark the original recursive type with `openedTwice`
749-
// which means that we always defensively copy the type in the future. This second
750-
// measure is necessary because findMember calls might be cached, so do not
751-
// necessarily appear in nested order.
752-
// Without the `openedTwice` trick, Typer.scala fails to Ycheck
753-
// at phase resolveSuper.
742+
// twice during findMember with two different prefixes, which risks picking the wrong prefix
743+
// in the `substRecThis(rt, pre)` call below. To avoid this problem we do a defensive copy
744+
// of the recursive type if the new prefix `pre` is neq the prefix with which the
745+
// type was previously opened.
746+
747+
val openedPre = tp.openedWithPrefix
754748
val rt =
755-
if (tp.opened) { // defensive copy
756-
tp.openedTwice = true
749+
if openedPre.exists && (openedPre ne pre) then // defensive copy
757750
RecType(rt => tp.parent.substRecThis(tp, rt.recThis))
758-
}
759751
else tp
760-
rt.opened = true
752+
rt.openedWithPrefix = pre
761753
try go(rt.parent).mapInfo(_.substRecThis(rt, pre))
762-
finally
763-
if (!rt.openedTwice) rt.opened = false
764-
}
754+
finally rt.openedWithPrefix = NoType
755+
end goRec
765756

766757
def goRefined(tp: RefinedType) = {
767758
val pdenot = go(tp.parent)
@@ -3191,9 +3182,8 @@ object Types {
31913182
*/
31923183
class RecType(parentExp: RecType => Type) extends RefinedOrRecType with BindingType {
31933184

3194-
// See discussion in findMember#goRec why these vars are needed
3195-
private[Types] var opened: Boolean = false
3196-
private[Types] var openedTwice: Boolean = false
3185+
// See discussion in findMember#goRec why this field is needed
3186+
private[Types] var openedWithPrefix: Type = NoType
31973187

31983188
val parent: Type = parentExp(this: @unchecked)
31993189

tests/pos/i17380.scala

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class C { type T; type U }
2+
3+
type X = C { type U = T; def u: U } { type T = String }

0 commit comments

Comments
 (0)