From ea87c0895c258419218938bf4ac9d811dc051cae Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 13 Feb 2023 11:29:12 +0100 Subject: [PATCH 1/5] Tweak withDenot for overloaded denotations When creating a NamedType with a given overloaded denotation, make sure that the type has a Name as designator. This prevents accidentally overwriting a more precise symbolic TermRef that refers to one specific alternative of the denotation. This might be enough to fix #16884. It would be great if we could maintain the general invariant that NamedTypes with overloaded denotations always have names as designators. But that looks very hard when we take into account that we need to update named types to new runs. A type might have a single denotation in one round and an overloaded one in the next. --- .../src/dotty/tools/dotc/core/Types.scala | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index d2fc225ff19d..0908574b0308 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2644,8 +2644,12 @@ object Types { } /** A reference like this one, but with the given symbol, if it exists */ - final def withSym(sym: Symbol)(using Context): ThisType = - if ((designator ne sym) && sym.exists) NamedType(prefix, sym).asInstanceOf[ThisType] + private def withSym(sym: Symbol)(using Context): ThisType = + if designator ne sym then NamedType(prefix, sym).asInstanceOf[ThisType] + else this + + private def withName(name: Name)(using Context): ThisType = + if designator ne name then NamedType(prefix, name).asInstanceOf[ThisType] else this /** A reference like this one, but with the given denotation, if it exists. @@ -2656,6 +2660,8 @@ object Types { * does not have a currently known denotation. * 3. The current designator is a name and the new symbolic named type * has the same info as the current info + * Returns a new named type with a name as designator if the denotation is + * overloaded and the name is different from the current designator. * Otherwise the current denotation is overwritten with the given one. * * Note: (2) and (3) are a "lock in mechanism" where a reference with a name as @@ -2669,13 +2675,16 @@ object Types { */ final def withDenot(denot: Denotation)(using Context): ThisType = if denot.exists then - val adapted = withSym(denot.symbol) + val adapted = + if denot.symbol.exists then withSym(denot.symbol) + else if denot.isOverloaded then withName(denot.name) + else this val result = - if (adapted.eq(this) + if adapted.eq(this) || designator.isInstanceOf[Symbol] || !adapted.denotationIsCurrent - || adapted.info.eq(denot.info)) - adapted + || adapted.info.eq(denot.info) + then adapted else this val lastDenot = result.lastDenotation denot match From 46e82dd4bb32faa65c5d85c1f10b91eaca9256eb Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 13 Feb 2023 17:37:56 +0100 Subject: [PATCH 2/5] Refine infoDependsOnPrefix Make infoDependsOnPrefix return true even if the prefix is the ThisType of the enclosing class, in case that class has a selftype which refines the binding of an abstract type. This allows to implement withDenot without overwriting a NamedType's state. --- .../src/dotty/tools/dotc/core/Types.scala | 69 ++++++++++--------- .../dotty/tools/dotc/reporting/messages.scala | 2 +- .../test/dotc/pos-test-pickling.blacklist | 1 + 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 0908574b0308..5c16ef05f48f 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2501,10 +2501,36 @@ object Types { /** A reference with the initial symbol in `symd` has an info that * might depend on the given prefix. + * Note: If T is an abstract type in trait or class C, its info depends + * even on C.this if class C has a self type that refines the info of T. + * Currently, "refines" means an actual refinement type that constrains the + * name `T`. We should try to extend that also to other classes that introduce + * a new bining for `T`. Furthermore, we should also treat term members + * in this way. */ private def infoDependsOnPrefix(symd: SymDenotation, prefix: Type)(using Context): Boolean = + + def refines(tp: Type, name: Name): Boolean = tp match + case AndType(tp1, tp2) => refines(tp1, name) || refines(tp2, name) + case RefinedType(parent, rname, _) => rname == name || refines(parent, name) + case tp: RecType => refines(tp.parent, name) + case _ => false + + def givenSelfTypeOrCompleter(cls: Symbol) = cls.infoOrCompleter match + case cinfo: ClassInfo => + cinfo.selfInfo match + case sym: Symbol => sym.infoOrCompleter + case tpe: Type => tpe + case _ => NoType + symd.maybeOwner.membersNeedAsSeenFrom(prefix) && !symd.is(NonMember) - || prefix.isInstanceOf[Types.ThisType] && symd.is(Opaque) // see pos/i11277.scala for a test where this matters + || prefix.match + case prefix: Types.ThisType => + symd.isAbstractType + && prefix.sameThis(symd.maybeOwner.thisType) + && refines(givenSelfTypeOrCompleter(prefix.cls), symd.name) + case _ => false + end infoDependsOnPrefix /** Is this a reference to a class or object member with an info that might depend * on the prefix? @@ -2653,25 +2679,11 @@ object Types { else this /** A reference like this one, but with the given denotation, if it exists. - * Returns a new named type with the denotation's symbol if that symbol exists, and - * one of the following alternatives applies: - * 1. The current designator is a symbol and the symbols differ, or - * 2. The current designator is a name and the new symbolic named type - * does not have a currently known denotation. - * 3. The current designator is a name and the new symbolic named type - * has the same info as the current info - * Returns a new named type with a name as designator if the denotation is - * overloaded and the name is different from the current designator. - * Otherwise the current denotation is overwritten with the given one. - * - * Note: (2) and (3) are a "lock in mechanism" where a reference with a name as - * designator can turn into a symbolic reference. - * - * Note: This is a subtle dance to keep the balance between going to symbolic - * references as much as we can (since otherwise we'd risk getting cycles) - * and to still not lose any type info in the denotation (since symbolic - * references often recompute their info directly from the symbol's info). - * A test case is neg/opaque-self-encoding.scala. + * Returns a new named type with the denotation's symbol as designator + * if that symbol exists and it is different from the current designator. + * Returns a new named type with the denotations's name as designator + * if the denotation is overloaded and its name is different from the + * current designator. */ final def withDenot(denot: Denotation)(using Context): ThisType = if denot.exists then @@ -2679,14 +2691,7 @@ object Types { if denot.symbol.exists then withSym(denot.symbol) else if denot.isOverloaded then withName(denot.name) else this - val result = - if adapted.eq(this) - || designator.isInstanceOf[Symbol] - || !adapted.denotationIsCurrent - || adapted.info.eq(denot.info) - then adapted - else this - val lastDenot = result.lastDenotation + val lastDenot = adapted.lastDenotation denot match case denot: SymDenotation if denot.validFor.firstPhaseId < ctx.phase.id @@ -2696,15 +2701,15 @@ object Types { // In this case the new SymDenotation might be valid for all phases, which means // we would not recompute the denotation when travelling to an earlier phase, maybe // in the next run. We fix that problem by creating a UniqueRefDenotation instead. - core.println(i"overwrite ${result.toString} / ${result.lastDenotation}, ${result.lastDenotation.getClass} with $denot at ${ctx.phaseId}") - result.setDenot( + core.println(i"overwrite ${adapted.toString} / ${adapted.lastDenotation}, ${adapted.lastDenotation.getClass} with $denot at ${ctx.phaseId}") + adapted.setDenot( UniqueRefDenotation( denot.symbol, denot.info, Period(ctx.runId, ctx.phaseId, denot.validFor.lastPhaseId), this.prefix)) case _ => - result.setDenot(denot) - result.asInstanceOf[ThisType] + adapted.setDenot(denot) + adapted.asInstanceOf[ThisType] else // don't assign NoDenotation, we might need to recover later. Test case is pos/avoid.scala. this diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index f41d34b8c17c..d7920e1f1b36 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -245,7 +245,7 @@ extends NotFoundMsg(MissingIdentID) { } } -class TypeMismatch(found: Type, expected: Type, inTree: Option[untpd.Tree], addenda: => String*)(using Context) +class TypeMismatch(found: Type, expected: Type, inTree: Option[untpd.Tree], addenda: => String*)(using Context) extends TypeMismatchMsg(found, expected)(TypeMismatchID): def msg(using Context) = diff --git a/compiler/test/dotc/pos-test-pickling.blacklist b/compiler/test/dotc/pos-test-pickling.blacklist index cdbad2160f2a..5f7221ee0ce9 100644 --- a/compiler/test/dotc/pos-test-pickling.blacklist +++ b/compiler/test/dotc/pos-test-pickling.blacklist @@ -19,6 +19,7 @@ i12299a.scala i13871.scala i15181.scala i15922.scala +t5031_2.scala # Tree is huge and blows stack for printing Text i7034.scala From c830ad26733a633a6d58b93e9bd66ff7b67b65ed Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 13 Feb 2023 17:51:53 +0100 Subject: [PATCH 3/5] Also consider bindings in classes when computing "refines" --- compiler/src/dotty/tools/dotc/core/Types.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 5c16ef05f48f..dc5165e7ad7c 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2503,17 +2503,17 @@ object Types { * might depend on the given prefix. * Note: If T is an abstract type in trait or class C, its info depends * even on C.this if class C has a self type that refines the info of T. - * Currently, "refines" means an actual refinement type that constrains the - * name `T`. We should try to extend that also to other classes that introduce - * a new bining for `T`. Furthermore, we should also treat term members - * in this way. + * We should also treat term members in this way. */ private def infoDependsOnPrefix(symd: SymDenotation, prefix: Type)(using Context): Boolean = def refines(tp: Type, name: Name): Boolean = tp match case AndType(tp1, tp2) => refines(tp1, name) || refines(tp2, name) case RefinedType(parent, rname, _) => rname == name || refines(parent, name) - case tp: RecType => refines(tp.parent, name) + case tp: ClassInfo => + val other = tp.cls.infoOrCompleter.nonPrivateMember(name) + other.exists && other.symbol != symd.symbol + case tp: TypeProxy => refines(tp.underlying, name) case _ => false def givenSelfTypeOrCompleter(cls: Symbol) = cls.infoOrCompleter match From 8d65f197de88e46bcfe7627c06e5d06792decb8e Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 13 Feb 2023 19:25:25 +0100 Subject: [PATCH 4/5] Also consider non-final term members with self type refinements Also consider non-final term members with self type refinements for infoDependsOnPrefix --- .../src/dotty/tools/dotc/core/Types.scala | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index dc5165e7ad7c..5ef6dbd39335 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2501,20 +2501,24 @@ object Types { /** A reference with the initial symbol in `symd` has an info that * might depend on the given prefix. - * Note: If T is an abstract type in trait or class C, its info depends - * even on C.this if class C has a self type that refines the info of T. - * We should also treat term members in this way. + * Note: If M is an abstract type or non-final term member in trait or class C, + * its info depends even on C.this if class C has a self type that refines + * the info of M. */ private def infoDependsOnPrefix(symd: SymDenotation, prefix: Type)(using Context): Boolean = def refines(tp: Type, name: Name): Boolean = tp match - case AndType(tp1, tp2) => refines(tp1, name) || refines(tp2, name) - case RefinedType(parent, rname, _) => rname == name || refines(parent, name) + case AndType(tp1, tp2) => + refines(tp1, name) || refines(tp2, name) + case RefinedType(parent, rname, _) => + rname == name || refines(parent, name) case tp: ClassInfo => - val other = tp.cls.infoOrCompleter.nonPrivateMember(name) - other.exists && other.symbol != symd.symbol - case tp: TypeProxy => refines(tp.underlying, name) - case _ => false + val otherd = tp.cls.nonPrivateMembersNamed(name) + otherd.exists && !otherd.containsSym(symd.symbol) + case tp: TypeProxy => + refines(tp.underlying, name) + case _ => + false def givenSelfTypeOrCompleter(cls: Symbol) = cls.infoOrCompleter match case cinfo: ClassInfo => @@ -2526,7 +2530,10 @@ object Types { symd.maybeOwner.membersNeedAsSeenFrom(prefix) && !symd.is(NonMember) || prefix.match case prefix: Types.ThisType => - symd.isAbstractType + (symd.isAbstractType + || symd.isTerm + && !symd.flagsUNSAFE.isOneOf(Module | Final | Param) + && !symd.maybeOwner.isEffectivelyFinal) && prefix.sameThis(symd.maybeOwner.thisType) && refines(givenSelfTypeOrCompleter(prefix.cls), symd.name) case _ => false From 994282177170ca1fab4a0fe3197c0efe4729e06a Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 14 Feb 2023 09:24:15 +0100 Subject: [PATCH 5/5] Avoid infinite recursion in infoDependsOnPrefix --- compiler/src/dotty/tools/dotc/core/Types.scala | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 5ef6dbd39335..d4fa6a819dc4 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2508,15 +2508,20 @@ object Types { private def infoDependsOnPrefix(symd: SymDenotation, prefix: Type)(using Context): Boolean = def refines(tp: Type, name: Name): Boolean = tp match - case AndType(tp1, tp2) => - refines(tp1, name) || refines(tp2, name) + case tp: TypeRef => + tp.symbol match + case cls: ClassSymbol => + val otherd = cls.nonPrivateMembersNamed(name) + otherd.exists && !otherd.containsSym(symd.symbol) + case tsym => + refines(tsym.info.hiBound, name) + // avoid going through tp.denot, since that might call infoDependsOnPrefix again case RefinedType(parent, rname, _) => rname == name || refines(parent, name) - case tp: ClassInfo => - val otherd = tp.cls.nonPrivateMembersNamed(name) - otherd.exists && !otherd.containsSym(symd.symbol) case tp: TypeProxy => refines(tp.underlying, name) + case AndType(tp1, tp2) => + refines(tp1, name) || refines(tp2, name) case _ => false