From e4f60a4f0a7f5548489bebccc3fd88cea79b94af Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 8 Jun 2020 20:22:58 +0200 Subject: [PATCH] Ensure names used in signatures are stable Signatures should not change before erasure, but some phases before erasure can change the owner of a definition, which can change their full name and therefore any signature which refers to these names. This commit fixes this by ensuring we always use the initial symbol to compute signatures before erasure. The check for signature consistency in TreeChecker was also improved as it wasn't able to catch this issue before due to signature caching hiding the issue. --- .../src/dotty/tools/dotc/core/TypeErasure.scala | 11 ++++++++++- compiler/src/dotty/tools/dotc/core/Types.scala | 8 ++++---- .../dotty/tools/dotc/transform/TreeChecker.scala | 14 +++++++++++--- tests/pos/local-signature.scala | 10 ++++++++++ 4 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 tests/pos/local-signature.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index fef52118809b..5eaf99776135 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -605,7 +605,16 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean if (defn.isSyntheticFunctionClass(sym)) sigName(defn.erasedFunctionType(sym)) else - normalizeClass(sym.asClass).fullName.asTypeName + val cls = normalizeClass(sym.asClass) + val fullName = + if !ctx.erasedTypes then + // It's important to use the initial symbol to compute the full name + // because the current symbol might have a different name or owner + // and signatures are required to be stable before erasure. + cls.initial.fullName + else + cls.fullName + fullName.asTypeName case tp: AppliedType => val sym = tp.tycon.typeSymbol sigName( // todo: what about repeatedParam? diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 031658e62409..f562cfff3bf4 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1900,7 +1900,7 @@ object Types { * or if there is none, the signature of the symbol. Signatures are always * computed before erasure, since some symbols change their signature at erasure. */ - protected def computeSignature(implicit ctx: Context): Signature = + protected[dotc] def computeSignature(implicit ctx: Context): Signature = val lastd = lastDenotation if lastd != null then sigFromDenot(lastd) else if ctx.erasedTypes then computeSignature(using ctx.withPhase(ctx.erasurePhase)) @@ -3064,7 +3064,7 @@ object Types { protected var mySignature: Signature = _ protected var mySignatureRunId: Int = NoRunId - protected def computeSignature(implicit ctx: Context): Signature + protected[dotc] def computeSignature(implicit ctx: Context): Signature final override def signature(implicit ctx: Context): Signature = { if (ctx.runId != mySignatureRunId) { @@ -3357,7 +3357,7 @@ object Types { companion.eq(ContextualMethodType) || companion.eq(ErasedContextualMethodType) - def computeSignature(implicit ctx: Context): Signature = { + protected[dotc] def computeSignature(implicit ctx: Context): Signature = { val params = if (isErasedMethod) Nil else paramInfos resultSignature.prependTermParams(params, isJavaMethod) } @@ -3592,7 +3592,7 @@ object Types { assert(resType.isInstanceOf[TermType], this) assert(paramNames.nonEmpty) - def computeSignature(implicit ctx: Context): Signature = + protected[dotc] def computeSignature(implicit ctx: Context): Signature = resultSignature.prependTypeParams(paramNames.length) override def isContextualMethod = resType.isContextualMethod diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 3b518c7227b1..fadf853e77ad 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -92,12 +92,20 @@ class TreeChecker extends Phase with SymTransformer { if (ctx.phaseId <= ctx.erasurePhase.id) { val cur = symd.info val initial = symd.initial.info - assert(cur.signature == initial.signature, - i"""Signature of $symd changed at phase ${ctx.phase} + val curSig = cur match { + case cur: SignatureCachingType => + // Bypass the signature cache, it might hide a signature change + cur.computeSignature + case _ => + cur.signature + } + assert(curSig == initial.signature, + i"""Signature of ${sym.showLocated} changed at phase ${ctx.base.squashed(ctx.phase.prev)} |Initial info: ${initial} |Initial sig : ${initial.signature} |Current info: ${cur} - |Current sig : ${cur.signature}""") + |Current sig : ${curSig} + |Current cached sig: ${cur.signature}""") } symd diff --git a/tests/pos/local-signature.scala b/tests/pos/local-signature.scala new file mode 100644 index 000000000000..c3364250914e --- /dev/null +++ b/tests/pos/local-signature.scala @@ -0,0 +1,10 @@ +class A { + def bn(x: => Any): Any = x + def foo: Unit = { + bn({ + class A + def foo(x: A): Unit = {} + foo(new A) + }) + } +}