From 7910be06114f2447cdfa1e85f0d75f7ec3a46e9b Mon Sep 17 00:00:00 2001 From: Oron Port Date: Tue, 4 Aug 2026 18:14:26 +0000 Subject: [PATCH 1/3] Fix #26681: keep one skolem for a selection on an unstable prefix `TypeAssigner.selectionType` skolemizes an unstable qualifier with a *fresh* `QualSkolemType` on every call, and stores the resulting info in the denotation of the (shared, hash-consed) `NamedType` it returns. `Inliner.InlineTyper.typedSelect` re-ran `selectionType` on trees that were already typed, so the same selection could end up with a different skolem each time, and the types derived from the previous denotation silently changed meaning. For `f(unstable.b)` in the issue this produced three skolems for `Owner#b`: one at the call site, one when the inline match selector was retyped, and one when the reduced `val $scrutinee1 = unstable.b` was retyped. The reduction instantiated the pattern-bound type `t` from the second while the scrutinee's type read back through the third, so the reduced body failed to typecheck with `Found: Box[(?1.w : Int)] / Required: Box[(?2.w : Int)]`. Skip the recomputation when it provably cannot select anything different from what the tree already has: the denotation is current, the prefix is unchanged, and selecting on that prefix skolemizes it, so the only thing a recomputation could change is the skolem. The refresh that i22070 and i23134 rely on, where the prefix does change while inlining, still happens. `skolemizesPrefix` names that condition rather than spelling it out separately from `maybeSkolemizePrefix`. Note that a fresh skolem per call is not itself the bug: distinct occurrences of a selection on an unstable prefix must not share one, or two evaluations of `unstable` get conflated. Making `maybeSkolemizePrefix` idempotent fixes this issue but is unsound, so the fix is at the point that recomputes needlessly. `NamedType.memberDenot` also recomputed a member as seen from the raw prefix, while the typer computes it as seen from a skolem of the prefix. A recomputed denotation was therefore more approximate than the pickled one -- `Box[(Owner#w : Int)]` came back as `Box[? <: (Owner#w : Int)]` -- and trees did not survive a pickling round-trip. Skolemize there too, so that a selection on an unstable prefix means the same thing compiled from source and from TASTy. --- .../src/dotty/tools/dotc/core/Types.scala | 15 +++++++++++- .../dotty/tools/dotc/inlines/Inliner.scala | 13 ++++++++++- .../dotty/tools/dotc/typer/TypeAssigner.scala | 11 ++++++++- tests/run/i26681.check | 2 ++ tests/run/i26681.scala | 23 +++++++++++++++++++ 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 tests/run/i26681.check create mode 100644 tests/run/i26681.scala diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 7e0867f087eb..73a26d6c7c85 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2572,7 +2572,20 @@ object Types extends TypeUtils { } private def memberDenot(prefix: Type, name: Name, allowPrivate: Boolean)(using Context): Denotation = - if (allowPrivate) prefix.member(name) else prefix.nonPrivateMember(name) + // We need a valid prefix for `asSeenFrom`, as in `memberBasedOnFlags`. + val pre0 = prefix match + case prefix: ClassInfo => prefix.appliedRef + case prefix => prefix.widenIfUnstable + // When the prefix of a term selection is not a legal prefix, the typer computes the + // member as seen from a skolem of the prefix, so that a member type that mentions the + // prefix stays precise (see `TypeAssigner.maybeSkolemizePrefix`). Recomputing the + // denotation here -- after unpickling, most importantly -- has to skolemize the same + // way, or the reference silently acquires a more approximate info than the one it was + // pickled with, and the tree no longer survives a pickling round-trip. + val pre = + if name.isTermName && !TypeOps.isLegalPrefix(pre0) then QualSkolemType(pre0) + else pre0 + prefix.findMember(name, pre, excluded = if allowPrivate then EmptyFlags else Private) private def argDenot(param: TypeSymbol)(using Context): Denotation = { val cls = param.owner diff --git a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala index 998cb0b945e4..8b8132e24923 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala @@ -963,7 +963,18 @@ class Inliner(val call: tpd.Tree)(using Context): // For instance in tests/pos/i22070 when we type `Featureful[?]#toFeatures`, // `selectionType` will skolemize the prefix, find the denotation, // and then set that denotation for the `TermRef(Featureful[?], symbol toFeatures)`. - val reselectedType = selectionType(tree, qual1) + // But skip it when it cannot select anything else than what we already have: for an + // unstable prefix `selectionType` creates a *fresh* skolem on every call (see + // `maybeSkolemizePrefix`), and since the `NamedType` it returns is shared, computing + // it again silently changes the meaning of the types that were derived from the + // previous denotation. That is how the same selection ended up carrying two different + // skolems in i26681. + val reselectedType = tree.typeOpt match + case tpe: NamedType + if tpe.denotationIsCurrent + && skolemizesPrefix(tpe.prefix, tree.name) + && tpe.prefix == qual1.tpe.widenIfUnstable => tpe + case _ => selectionType(tree, qual1) def isConcreteImplementationOf(reselected: Symbol, overridden: Symbol)(using Context): Boolean = reselected.isTerm diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index d8f7dd32c9bd..1fe8ae1a0a77 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -120,11 +120,20 @@ trait TypeAssigner { * @see QualSkolemType, TypeOps#asSeenFrom */ def maybeSkolemizePrefix(qualType: Type, name: Name)(using Context): Type = - if (name.isTermName && !TypeOps.isLegalPrefix(qualType)) + if skolemizesPrefix(qualType, name) then QualSkolemType(qualType) else qualType + /** Does selecting `name` on `qualType` skolemize the prefix? + * + * Callers that may compute a selection type more than once need this: each call to + * `maybeSkolemizePrefix` makes a *fresh* skolem, so for such a selection recomputing + * the type is not a no-op even when it finds the very same member. + */ + def skolemizesPrefix(qualType: Type, name: Name)(using Context): Boolean = + name.isTermName && !TypeOps.isLegalPrefix(qualType) + /** The type of the selection `tree`, where `qual1` is the typed qualifier part. */ def selectionType(tree: untpd.RefTree, qual1: Tree)(using Context): Type = val qualType = diff --git a/tests/run/i26681.check b/tests/run/i26681.check new file mode 100644 index 000000000000..79ebd0860f49 --- /dev/null +++ b/tests/run/i26681.check @@ -0,0 +1,2 @@ +ok +ok diff --git a/tests/run/i26681.scala b/tests/run/i26681.scala new file mode 100644 index 000000000000..f409aef89978 --- /dev/null +++ b/tests/run/i26681.scala @@ -0,0 +1,23 @@ +// https://github.com/scala/scala3/issues/26681 +class Box[T] + +class Owner: + val w: Int = 8 + val b: Box[w.type] = new Box[w.type] + +def use[T](b: Box[T]): String = "ok" + +def unstable: Owner = new Owner + +transparent inline def f(inline rhs: Any): Any = + inline rhs match + case r: Box[t] => use[t](r) + +@main def Test = + // `unstable.b` has an unstable prefix, so its type is `Owner#b`, whose info is + // computed by skolemizing the prefix. Retyping the selection while inlining used + // to create a second skolem and overwrite the denotation of `Owner#b`, so the type + // `t` inferred when reducing the inline match no longer matched the scrutinee. + println(f(unstable.b)) + val o = unstable + println(f(o.b)) From 638cf5448c1ca391ca19e9b9d9d4ab6d6ee9dbec Mon Sep 17 00:00:00 2001 From: Oron Port Date: Tue, 4 Aug 2026 18:14:27 +0000 Subject: [PATCH 2/3] Pin that selections on an unstable prefix get distinct skolems Each selection on an unstable prefix must skolemize that prefix afresh: two evaluations of `unstable` return two different `Owner`s, so their `w` are unrelated and `Box[w.type]` must not unify across occurrences. Nothing in the test suite covered this. Memoizing the `QualSkolemType` per prefix type -- which makes `maybeSkolemizePrefix` idempotent, and is the natural response to the same selection carrying two different skolems in #26681 -- makes all three cases here compile, with the whole suite still green. This test makes that regression visible. --- tests/neg/i26681b.scala | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/neg/i26681b.scala diff --git a/tests/neg/i26681b.scala b/tests/neg/i26681b.scala new file mode 100644 index 000000000000..845cb82410e2 --- /dev/null +++ b/tests/neg/i26681b.scala @@ -0,0 +1,25 @@ +// https://github.com/scala/scala3/issues/26681 +// +// Selecting on an unstable prefix skolemizes that prefix, and each selection must +// get its *own* skolem: two evaluations of `unstable` return two different `Owner`s, +// so their `w` are unrelated and `Box[w.type]` must not unify across occurrences. +// +// Memoizing the `QualSkolemType` per prefix type would make `maybeSkolemizePrefix` +// idempotent -- which is tempting, since a fresh skolem per call is what made the +// same selection carry two different skolems in i26681 -- but it also makes every +// case below compile. This test pins the property so that stays visible. +class Box[T](val x: T) + +class Owner: + val w: Int = 8 + val b: Box[w.type] = new Box(w) + def get: Box[w.type] = new Box(w) + def set(bb: Box[w.type]): Unit = () + +def unstable: Owner = new Owner + +def same[T](a: Box[T], b: Box[T]): Unit = () + +def t1 = same(unstable.b, unstable.b) // error +def t2 = same(unstable.get, unstable.get) // error +def t3 = unstable.set(unstable.get) // error From b530b9145883b15452364d7a65552cdcf9a9cc46 Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Wed, 5 Aug 2026 11:39:14 +0900 Subject: [PATCH 3/3] WIP: make `maybeSkolemizePrefix` idempotent for same tree --- .../src/dotty/tools/dotc/core/Types.scala | 15 +--------- .../dotty/tools/dotc/inlines/Inliner.scala | 13 +-------- .../dotty/tools/dotc/typer/TypeAssigner.scala | 28 ++++++++++++------- .../test/dotc/run-test-pickling.excludelist | 4 +++ tests/neg/i26681b.scala | 9 ------ tests/run/i26681.scala | 4 --- 6 files changed, 24 insertions(+), 49 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 73a26d6c7c85..7e0867f087eb 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2572,20 +2572,7 @@ object Types extends TypeUtils { } private def memberDenot(prefix: Type, name: Name, allowPrivate: Boolean)(using Context): Denotation = - // We need a valid prefix for `asSeenFrom`, as in `memberBasedOnFlags`. - val pre0 = prefix match - case prefix: ClassInfo => prefix.appliedRef - case prefix => prefix.widenIfUnstable - // When the prefix of a term selection is not a legal prefix, the typer computes the - // member as seen from a skolem of the prefix, so that a member type that mentions the - // prefix stays precise (see `TypeAssigner.maybeSkolemizePrefix`). Recomputing the - // denotation here -- after unpickling, most importantly -- has to skolemize the same - // way, or the reference silently acquires a more approximate info than the one it was - // pickled with, and the tree no longer survives a pickling round-trip. - val pre = - if name.isTermName && !TypeOps.isLegalPrefix(pre0) then QualSkolemType(pre0) - else pre0 - prefix.findMember(name, pre, excluded = if allowPrivate then EmptyFlags else Private) + if (allowPrivate) prefix.member(name) else prefix.nonPrivateMember(name) private def argDenot(param: TypeSymbol)(using Context): Denotation = { val cls = param.owner diff --git a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala index 8b8132e24923..998cb0b945e4 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala @@ -963,18 +963,7 @@ class Inliner(val call: tpd.Tree)(using Context): // For instance in tests/pos/i22070 when we type `Featureful[?]#toFeatures`, // `selectionType` will skolemize the prefix, find the denotation, // and then set that denotation for the `TermRef(Featureful[?], symbol toFeatures)`. - // But skip it when it cannot select anything else than what we already have: for an - // unstable prefix `selectionType` creates a *fresh* skolem on every call (see - // `maybeSkolemizePrefix`), and since the `NamedType` it returns is shared, computing - // it again silently changes the meaning of the types that were derived from the - // previous denotation. That is how the same selection ended up carrying two different - // skolems in i26681. - val reselectedType = tree.typeOpt match - case tpe: NamedType - if tpe.denotationIsCurrent - && skolemizesPrefix(tpe.prefix, tree.name) - && tpe.prefix == qual1.tpe.widenIfUnstable => tpe - case _ => selectionType(tree, qual1) + val reselectedType = selectionType(tree, qual1) def isConcreteImplementationOf(reselected: Symbol, overridden: Symbol)(using Context): Boolean = reselected.isTerm diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 1fe8ae1a0a77..2e49e7ffce72 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -120,19 +120,22 @@ trait TypeAssigner { * @see QualSkolemType, TypeOps#asSeenFrom */ def maybeSkolemizePrefix(qualType: Type, name: Name)(using Context): Type = - if skolemizesPrefix(qualType, name) then + if (name.isTermName && !TypeOps.isLegalPrefix(qualType)) QualSkolemType(qualType) else qualType - /** Does selecting `name` on `qualType` skolemize the prefix? - * - * Callers that may compute a selection type more than once need this: each call to - * `maybeSkolemizePrefix` makes a *fresh* skolem, so for such a selection recomputing - * the type is not a no-op even when it finds the very same member. - */ - def skolemizesPrefix(qualType: Type, name: Name)(using Context): Boolean = - name.isTermName && !TypeOps.isLegalPrefix(qualType) + def maybeSkolemizePrefix(tree: untpd.Tree, qualType: Type, name: Name)(using Context): Type = + if name.isTermName && !TypeOps.isLegalPrefix(qualType) then + tree.getAttachment(QualSkolem) match + case Some(skolem) if skolem.info == qualType => + skolem + case _ => + val skolem = QualSkolemType(qualType) + tree.putAttachment(QualSkolem, skolem) + skolem + else + qualType /** The type of the selection `tree`, where `qual1` is the typed qualifier part. */ def selectionType(tree: untpd.RefTree, qual1: Tree)(using Context): Type = @@ -164,7 +167,7 @@ trait TypeAssigner { // is casted to T[] by javac. Since the return type of Array[T]#clone() is Array[T], // this is exactly what Erasure will do. case _ => - val pre = maybeSkolemizePrefix(qualType, name) + val pre = maybeSkolemizePrefix(tree, qualType, name) val mbr = if ctx.isJava then // don't look in the companion class here if qual is a module, @@ -630,6 +633,11 @@ object TypeAssigner extends TypeAssigner: */ private[dotc] val SkolemizedArgs = new Property.StickyKey[Map[tpd.Tree, SkolemType]] + /** An attachment on a selection holding the `QualSkolemType` used to type it. + * Sticky so that copies of the tree (e.g. while inlining) keep the same skolem. + */ + private[dotc] val QualSkolem = new Property.StickyKey[QualSkolemType] + def seqLitType(tree: untpd.SeqLiteral, elemType: Type)(using Context) = tree match case tree: untpd.JavaSeqLiteral => defn.ArrayOf(elemType) case _ => if ctx.erasedTypes then defn.SeqType else defn.SeqType.appliedTo(elemType) diff --git a/compiler/test/dotc/run-test-pickling.excludelist b/compiler/test/dotc/run-test-pickling.excludelist index 6aa8a67982b3..05f772a17cc2 100644 --- a/compiler/test/dotc/run-test-pickling.excludelist +++ b/compiler/test/dotc/run-test-pickling.excludelist @@ -52,5 +52,9 @@ typeCheckErrors.scala i18150.scala i22968.scala +# Unstable-prefix selection: typer skolemizes, unpickling recomputes without skolem +# so info goes from Box[(Owner#w : Int)] to Box[? <: (Owner#w : Int)]. Separate fix later. +i26681.scala + # Pickling differences with local parameters export forwarders of methods with into parameters. But their external type is the same Parser.scala diff --git a/tests/neg/i26681b.scala b/tests/neg/i26681b.scala index 845cb82410e2..2e3aa19f22f5 100644 --- a/tests/neg/i26681b.scala +++ b/tests/neg/i26681b.scala @@ -1,13 +1,4 @@ // https://github.com/scala/scala3/issues/26681 -// -// Selecting on an unstable prefix skolemizes that prefix, and each selection must -// get its *own* skolem: two evaluations of `unstable` return two different `Owner`s, -// so their `w` are unrelated and `Box[w.type]` must not unify across occurrences. -// -// Memoizing the `QualSkolemType` per prefix type would make `maybeSkolemizePrefix` -// idempotent -- which is tempting, since a fresh skolem per call is what made the -// same selection carry two different skolems in i26681 -- but it also makes every -// case below compile. This test pins the property so that stays visible. class Box[T](val x: T) class Owner: diff --git a/tests/run/i26681.scala b/tests/run/i26681.scala index f409aef89978..5f299659f94b 100644 --- a/tests/run/i26681.scala +++ b/tests/run/i26681.scala @@ -14,10 +14,6 @@ transparent inline def f(inline rhs: Any): Any = case r: Box[t] => use[t](r) @main def Test = - // `unstable.b` has an unstable prefix, so its type is `Owner#b`, whose info is - // computed by skolemizing the prefix. Retyping the selection while inlining used - // to create a second skolem and overwrite the denotation of `Owner#b`, so the type - // `t` inferred when reducing the inline match no longer matched the scrutinee. println(f(unstable.b)) val o = unstable println(f(o.b))