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.

3 files changed, 11 insertions, 38 deletions

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