From a690de26f9c7d73375f6463544bfd549a8487388 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 May 2022 18:44:47 +0200 Subject: [PATCH 1/4] Invalidate some denotations in earlier phases This addresses the specific problem raised by #10044: A denotation of a NamedType goes from a UniqueRefDenotation to a SymDenotation after erasure and is then not reset to the original since the SymDenotation is valid at all phases. We remember in this case in the NamedType the first phase in which the SymDenotaton is valid and force a recompute in earlier phases. Fixes #10044 --- .../src/dotty/tools/dotc/core/Types.scala | 24 +++++++++++++++++++ compiler/test-resources/repl/i10044 | 12 ++++++++++ 2 files changed, 36 insertions(+) create mode 100644 compiler/test-resources/repl/i10044 diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 7d57b4eb2740..5927552b8585 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2097,6 +2097,7 @@ object Types { private var lastDenotation: Denotation | Null = null private var lastSymbol: Symbol | Null = null private var checkedPeriod: Period = Nowhere + private var firstValidPhaseId: Int = 0 private var myStableHash: Byte = 0 private var mySignature: Signature = _ private var mySignatureRunId: Int = NoRunId @@ -2212,6 +2213,8 @@ object Types { val now = ctx.period // Even if checkedPeriod == now we still need to recheck lastDenotation.validFor // as it may have been mutated by SymDenotation#installAfter + if firstValidPhaseId > now.phaseId then + revalidateDenot() if (checkedPeriod != Nowhere && lastDenotation.nn.validFor.contains(now)) { checkedPeriod = now lastDenotation.nn @@ -2340,6 +2343,18 @@ object Types { def recomputeDenot()(using Context): Unit = setDenot(memberDenot(name, allowPrivate = !symbol.exists || symbol.is(Private))) + /** Try to recompute denotation and reset `firstValidPhaseId`. + * @pre Current phase id < firstValidPhaseId + */ + def revalidateDenot()(using Context): Unit = + if (prefix ne NoPrefix) then + core.println(i"revalidate $prefix . $name, $firstValidPhaseId > ${ctx.phaseId}") + val newDenot = memberDenot(name, allowPrivate = + lastSymbol == null || !lastSymbol.nn.exists || lastSymbol.nn.is(Private)) + if newDenot.exists then + setDenot(newDenot) + firstValidPhaseId = ctx.phaseId + private def setDenot(denot: Denotation)(using Context): Unit = { if (Config.checkNoDoubleBindings) if (ctx.settings.YnoDoubleBindings.value) @@ -2570,6 +2585,15 @@ object Types { || adapted.info.eq(denot.info)) adapted else this + val lastDenot = result.lastDenotation + if denot.isInstanceOf[SymDenotation] && lastDenot != null && !lastDenot.isInstanceOf[SymDenotation] then + // 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 recording in this case in the NamedType + // the phase from which the denotation is valid. Taking the denotation at an earlier + // phase will then lead to a `revalidateDenot`. + core.println(i"overwrite ${result.toString} / ${result.lastDenotation} with $denot") + result.firstValidPhaseId = ctx.phaseId result.setDenot(denot) result.asInstanceOf[ThisType] } diff --git a/compiler/test-resources/repl/i10044 b/compiler/test-resources/repl/i10044 new file mode 100644 index 000000000000..3d0fe506ea32 --- /dev/null +++ b/compiler/test-resources/repl/i10044 @@ -0,0 +1,12 @@ +scala> object Foo { opaque type Bar = Int; object Bar { extension (b: Bar) def flip: Bar = -b; def apply(x: Int): Bar = x }} +// defined object Foo +scala> val a = Foo.Bar(42) +val a: Foo.Bar = 42 +scala> val b = a.flip +val b: Foo.Bar = -42 +scala> val c = b.flip +val c: Foo.Bar = 42 +scala> val d = c.flip +val d: Foo.Bar = -42 +scala> val e = d.flip +val e: Foo.Bar = 42 From e609851c3f505b597f95a70c21a76d895e08c4fe Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 6 May 2022 12:42:38 +0200 Subject: [PATCH 2/4] Alternative fix for 10044 --- .../src/dotty/tools/dotc/core/Types.scala | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 5927552b8585..8617dff102a3 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2097,7 +2097,6 @@ object Types { private var lastDenotation: Denotation | Null = null private var lastSymbol: Symbol | Null = null private var checkedPeriod: Period = Nowhere - private var firstValidPhaseId: Int = 0 private var myStableHash: Byte = 0 private var mySignature: Signature = _ private var mySignatureRunId: Int = NoRunId @@ -2213,8 +2212,6 @@ object Types { val now = ctx.period // Even if checkedPeriod == now we still need to recheck lastDenotation.validFor // as it may have been mutated by SymDenotation#installAfter - if firstValidPhaseId > now.phaseId then - revalidateDenot() if (checkedPeriod != Nowhere && lastDenotation.nn.validFor.contains(now)) { checkedPeriod = now lastDenotation.nn @@ -2343,18 +2340,6 @@ object Types { def recomputeDenot()(using Context): Unit = setDenot(memberDenot(name, allowPrivate = !symbol.exists || symbol.is(Private))) - /** Try to recompute denotation and reset `firstValidPhaseId`. - * @pre Current phase id < firstValidPhaseId - */ - def revalidateDenot()(using Context): Unit = - if (prefix ne NoPrefix) then - core.println(i"revalidate $prefix . $name, $firstValidPhaseId > ${ctx.phaseId}") - val newDenot = memberDenot(name, allowPrivate = - lastSymbol == null || !lastSymbol.nn.exists || lastSymbol.nn.is(Private)) - if newDenot.exists then - setDenot(newDenot) - firstValidPhaseId = ctx.phaseId - private def setDenot(denot: Denotation)(using Context): Unit = { if (Config.checkNoDoubleBindings) if (ctx.settings.YnoDoubleBindings.value) @@ -2576,7 +2561,7 @@ object Types { * A test case is neg/opaque-self-encoding.scala. */ final def withDenot(denot: Denotation)(using Context): ThisType = - if (denot.exists) { + if denot.exists then val adapted = withSym(denot.symbol) val result = if (adapted.eq(this) @@ -2586,17 +2571,24 @@ object Types { adapted else this val lastDenot = result.lastDenotation - if denot.isInstanceOf[SymDenotation] && lastDenot != null && !lastDenot.isInstanceOf[SymDenotation] then - // 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 recording in this case in the NamedType - // the phase from which the denotation is valid. Taking the denotation at an earlier - // phase will then lead to a `revalidateDenot`. - core.println(i"overwrite ${result.toString} / ${result.lastDenotation} with $denot") - result.firstValidPhaseId = ctx.phaseId - result.setDenot(denot) + denot match + case denot: SymDenotation + if denot.validFor.firstPhaseId < ctx.phase.id + && lastDenot != null + && lastDenot.validFor.lastPhaseId > denot.validFor.firstPhaseId + && !lastDenot.isInstanceOf[SymDenotation] => + // 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( + UniqueRefDenotation( + denot.symbol, denot.info, + Period(ctx.runId, ctx.phaseId, denot.validFor.lastPhaseId), + this.prefix)) + case _ => + result.setDenot(denot) result.asInstanceOf[ThisType] - } else // don't assign NoDenotation, we might need to recover later. Test case is pos/avoid.scala. this From 9f9abfded6fdce95cc533b87bd37d4aece226a1d Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 6 May 2022 15:09:36 +0200 Subject: [PATCH 3/4] Fix bug in GenericSignatures --- compiler/src/dotty/tools/dotc/core/Types.scala | 13 +++++++------ .../tools/dotc/transform/GenericSignatures.scala | 4 +++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 8617dff102a3..3d2a77a05c82 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1079,17 +1079,13 @@ object Types { * @param checkClassInfo if true we check that ClassInfos are within bounds of abstract types */ final def overrides(that: Type, relaxedCheck: Boolean, matchLoosely: => Boolean, checkClassInfo: Boolean = true)(using Context): Boolean = { - def widenNullary(tp: Type) = tp match { - case tp @ MethodType(Nil) => tp.resultType - case _ => tp - } val overrideCtx = if relaxedCheck then ctx.relaxedOverrideContext else ctx inContext(overrideCtx) { !checkClassInfo && this.isInstanceOf[ClassInfo] || (this.widenExpr frozen_<:< that.widenExpr) || matchLoosely && { - val this1 = widenNullary(this) - val that1 = widenNullary(that) + val this1 = this.widenNullaryMethod + val that1 = that.widenNullaryMethod ((this1 `ne` this) || (that1 `ne` that)) && this1.overrides(that1, relaxedCheck, false, checkClassInfo) } } @@ -1326,6 +1322,11 @@ object Types { this } + /** If this is a nullary method type, its result type */ + def widenNullaryMethod(using Context): Type = this match + case tp @ MethodType(Nil) => tp.resType + case _ => this + /** The singleton types that must or may be in this type. @see Atoms. * Overridden and cached in OrType. */ diff --git a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala index 6ce708068393..0aa5b2b86b51 100644 --- a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala +++ b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala @@ -189,7 +189,9 @@ object GenericSignatures { fullNameInSig(tp.typeSymbol) builder.append(';') case _ => - boxedSig(tp) + boxedSig(tp.widenDealias.widenNullaryMethod) + // `tp` might be a singleton type referring to a getter. + // Hence the widenNullaryMethod. } if (pre.exists) { From ce54c431c38977139db8cbf7e0560d0a978b2bd1 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 18 May 2022 11:45:50 +0200 Subject: [PATCH 4/4] Fix comment --- compiler/src/dotty/tools/dotc/Compiler.scala | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index d41c57ab116e..57defeda0a10 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -24,13 +24,8 @@ class Compiler { * all refs to it would become outdated - they could not be dereferenced in the * new phase. * - * After erasure, signature changing denot-transformers are OK because erasure - * will make sure that only term refs with fixed SymDenotations survive beyond it. This - * is possible because: - * - * - splitter has run, so every ident or select refers to a unique symbol - * - after erasure, asSeenFrom is the identity, so every reference has a - * plain SymDenotation, as opposed to a UniqueRefDenotation. + * After erasure, signature changing denot-transformers are OK because signatures + * are never recomputed later than erasure. */ def phases: List[List[Phase]] = frontendPhases ::: picklerPhases ::: transformPhases ::: backendPhases