From ded5d25eb13bbac90c955aa4d9ae99d343e51aaf Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 1 May 2023 18:46:20 +0200 Subject: [PATCH 1/2] 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. --- .../src/dotty/tools/dotc/core/Types.scala | 36 +++++++------------ tests/pos/i17380.scala | 3 ++ 2 files changed, 16 insertions(+), 23 deletions(-) create mode 100644 tests/pos/i17380.scala diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index fe0fc8a6dc2d..f000fe53f239 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -735,33 +735,24 @@ object Types { // TODO: change tp.parent to nullable or other values if ((tp.parent: Type | Null) == null) NoDenotation else if (tp eq pre) go(tp.parent) - else { + else //println(s"find member $pre . $name in $tp") // We have to be careful because we might open the same (wrt eq) recursive type - // twice during findMember which risks picking the wrong prefix in the `substRecThis(rt, pre)` - // call below. To avoid this problem we do a defensive copy of the recursive - // type first. But if we do this always we risk being inefficient and we ran into - // stackoverflows when compiling pos/hk.scala under the refinement encoding - // of hk-types. So we only do a copy if the type - // is visited again in a recursive call to `findMember`, as tracked by `tp.opened`. - // Furthermore, if this happens we mark the original recursive type with `openedTwice` - // which means that we always defensively copy the type in the future. This second - // measure is necessary because findMember calls might be cached, so do not - // necessarily appear in nested order. - // Without the `openedTwice` trick, Typer.scala fails to Ycheck - // at phase resolveSuper. + // twice during findMember with two different prefixes, which risks picking the wrong prefix + // in the `substRecThis(rt, pre)` call below. To avoid this problem we do a defensive copy + // of the recursive type if the new prefix `pre` is neq the prefix with which the + // type was previously opened. + + val openedPre = tp.openedWithPrefix val rt = - if (tp.opened) { // defensive copy - tp.openedTwice = true + if openedPre.exists && (openedPre ne pre) then // defensive copy RecType(rt => tp.parent.substRecThis(tp, rt.recThis)) - } else tp - rt.opened = true + rt.openedWithPrefix = pre try go(rt.parent).mapInfo(_.substRecThis(rt, pre)) - finally - if (!rt.openedTwice) rt.opened = false - } + finally rt.openedWithPrefix = NoType + end goRec def goRefined(tp: RefinedType) = { val pdenot = go(tp.parent) @@ -3191,9 +3182,8 @@ object Types { */ class RecType(parentExp: RecType => Type) extends RefinedOrRecType with BindingType { - // See discussion in findMember#goRec why these vars are needed - private[Types] var opened: Boolean = false - private[Types] var openedTwice: Boolean = false + // See discussion in findMember#goRec why this field is needed + private[Types] var openedWithPrefix: Type = NoType val parent: Type = parentExp(this: @unchecked) diff --git a/tests/pos/i17380.scala b/tests/pos/i17380.scala new file mode 100644 index 000000000000..8963db3058b3 --- /dev/null +++ b/tests/pos/i17380.scala @@ -0,0 +1,3 @@ +class C { type T; type U } + +type X = C { type U = T; def u: U } { type T = String } \ No newline at end of file From 90f4253bf871cce9b79073759757696b001f5634 Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 5 May 2023 12:49:59 +0200 Subject: [PATCH 2/2] Add test --- tests/pos/i17381.scala | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/pos/i17381.scala diff --git a/tests/pos/i17381.scala b/tests/pos/i17381.scala new file mode 100644 index 000000000000..3d8aac8e9f67 --- /dev/null +++ b/tests/pos/i17381.scala @@ -0,0 +1,9 @@ +import reflect.Selectable.reflectiveSelectable + +type Y = { type T = String; def u(): T } + +trait Test { + + val y1: Y + val y2 = y1.u() +} \ No newline at end of file