From 1f6f7f8aa94c622665a35343de8108ea66a787b7 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 26 May 2016 20:11:26 -0700 Subject: Don't cache `MethodSymbol`'s `memberType`. Correct caching is impossible because `sym.tpeHK.asSeenFrom(pre, sym.owner)` may have different results even for reference-identical `sym.tpeHK` and `pre` (even in the same period). For example, `pre` could be a `ThisType`. For such a type, `tpThen eq tpNow` does not imply `tpThen` and `tpNow` mean the same thing, because `tpThen.typeSymbol.info` could have been different from what it is now, and the cache won't know simply by looking at `pre`. Somehow this distinction never caused trouble, but when starting to desugar module definitions during the fields phase, it causes several test failures. I tried keying the cache on the current period to no avail. --- src/reflect/scala/reflect/internal/Symbols.scala | 20 -------------------- src/reflect/scala/reflect/internal/Types.scala | 22 ++++++++++------------ .../reflect/runtime/SynchronizedSymbols.scala | 7 +------ 3 files changed, 11 insertions(+), 38 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 1456022b10..e438856160 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -2906,11 +2906,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => /** A class for method symbols */ class MethodSymbol protected[Symbols] (initOwner: Symbol, initPos: Position, initName: TermName) extends TermSymbol(initOwner, initPos, initName) with MethodSymbolApi { - private[this] var mtpePeriod = NoPeriod - private[this] var mtpePre: Type = _ - private[this] var mtpeResult: Type = _ - private[this] var mtpeInfo: Type = _ - override def isLabel = this hasFlag LABEL override def isVarargsMethod = this hasFlag VARARGS override def isLiftedMethod = this hasFlag LIFTED @@ -2928,21 +2923,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => // are a case accessor (you can also be a field.) override def isCaseAccessorMethod = isCaseAccessor - def typeAsMemberOf(pre: Type): Type = { - if (mtpePeriod == currentPeriod) { - if ((mtpePre eq pre) && (mtpeInfo eq info)) return mtpeResult - } else if (isValid(mtpePeriod)) { - mtpePeriod = currentPeriod - if ((mtpePre eq pre) && (mtpeInfo eq info)) return mtpeResult - } - val res = pre.computeMemberType(this) - mtpePeriod = currentPeriod - mtpePre = pre - mtpeInfo = info - mtpeResult = res - res - } - override def isVarargs: Boolean = definitions.isVarArgsList(paramss.flatten) override def returnType: Type = { diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 895bb60a08..fb78aa5009 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -686,23 +686,21 @@ trait Types * }}} */ def memberInfo(sym: Symbol): Type = { - require(sym ne NoSymbol, this) +// assert(sym ne NoSymbol, this) sym.info.asSeenFrom(this, sym.owner) } /** The type of `sym`, seen as a member of this type. */ - def memberType(sym: Symbol): Type = sym match { - case meth: MethodSymbol => - meth.typeAsMemberOf(this) - case _ => - computeMemberType(sym) - } - - def computeMemberType(sym: Symbol): Type = sym.tpeHK match { //@M don't prematurely instantiate higher-kinded types, they will be instantiated by transform, typedTypeApply, etc. when really necessary - case OverloadedType(_, alts) => - OverloadedType(this, alts) + def memberType(sym: Symbol): Type = sym.tpeHK match { + case OverloadedType(_, alts) => OverloadedType(this, alts) case tp => - if (sym eq NoSymbol) NoType else tp.asSeenFrom(this, sym.owner) + // Correct caching is nearly impossible because `sym.tpeHK.asSeenFrom(pre, sym.owner)` + // may have different results even for reference-identical `sym.tpeHK` and `pre` (even in the same period). + // For example, `pre` could be a `ThisType`. For such a type, `tpThen eq tpNow` does not imply + // `tpThen` and `tpNow` mean the same thing, because `tpThen.typeSymbol.info` could have been different + // from what it is now, and the cache won't know simply by looking at `pre`. + if (sym eq NoSymbol) NoType + else tp.asSeenFrom(this, sym.owner) } /** Substitute types `to` for occurrences of references to diff --git a/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala b/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala index 313ec89311..237afa082b 100644 --- a/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala +++ b/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala @@ -199,12 +199,7 @@ private[reflect] trait SynchronizedSymbols extends internal.Symbols { self: Symb trait SynchronizedTermSymbol extends SynchronizedSymbol - trait SynchronizedMethodSymbol extends MethodSymbol with SynchronizedTermSymbol { - // we can keep this lock fine-grained, because it's just a cache over asSeenFrom, which makes deadlocks impossible - // unfortunately we cannot elide this lock, because the cache depends on `pre` - private lazy val typeAsMemberOfLock = new Object - override def typeAsMemberOf(pre: Type): Type = gilSynchronizedIfNotThreadsafe { typeAsMemberOfLock.synchronized { super.typeAsMemberOf(pre) } } - } + trait SynchronizedMethodSymbol extends MethodSymbol with SynchronizedTermSymbol trait SynchronizedModuleSymbol extends ModuleSymbol with SynchronizedTermSymbol -- cgit v1.2.3