Merge pull request #11330 from dotty-staging/reuseSymDenotations

Make it possible to always set the prefix of denotations

Author: smarter

compiler/src/dotty/tools/dotc/config/Config.scala

@@ -205,4 +205,13 @@ object Config {
 
   /** When in IDE, turn StaleSymbol errors into warnings instead of crashing */
   inline val ignoreStaleInIDE = true
+
+  /** If true, `Denotation#asSeenFrom` is allowed to return an existing
+   *  `SymDenotation` instead of allocating a new `SingleDenotation` if
+   *  the two would only differ in their `prefix` (SymDenotation always
+   *  have `NoPrefix` as their prefix).
+   *  This is done for performance reasons: when compiling Dotty itself this
+   *  reduces the number of allocated denotations by ~50%.
+   */
+  inline val reuseSymDenotations = true
 }

compiler/src/dotty/tools/dotc/config/JavaPlatform.scala

@@ -21,7 +21,7 @@ class JavaPlatform extends Platform {
   }
 
   // The given symbol is a method with the right name and signature to be a runnable java program.
-  def isMainMethod(sym: SymDenotation)(using Context): Boolean =
+  def isMainMethod(sym: Symbol)(using Context): Boolean =
     (sym.name == nme.main) && (sym.info match {
       case MethodTpe(_, defn.ArrayOf(el) :: Nil, restpe) => el =:= defn.StringType && (restpe isRef defn.UnitClass)
       case _ => false

compiler/src/dotty/tools/dotc/config/Platform.scala

@@ -38,12 +38,10 @@ abstract class Platform {
   def newClassLoader(bin: AbstractFile)(using Context): SymbolLoader
 
   /** The given symbol is a method with the right name and signature to be a runnable program. */
-  def isMainMethod(sym: SymDenotation)(using Context): Boolean
+  def isMainMethod(sym: Symbol)(using Context): Boolean
 
   /** The given class has a main method. */
   final def hasMainMethod(sym: Symbol)(using Context): Boolean =
-    sym.info.member(nme.main).hasAltWith {
-      case x: SymDenotation => isMainMethod(x) && (sym.is(Module) || x.isStatic)
-      case _ => false
-    }
+    sym.info.member(nme.main).hasAltWith(d =>
+      isMainMethod(d.symbol) && (sym.is(Module) || d.symbol.isStatic))
 }

compiler/src/dotty/tools/dotc/core/Denotations.scala

@@ -573,8 +573,12 @@ object Denotations {
 
     final def name(using Context): Name = symbol.name
 
-    /** If this is not a SymDenotation: The prefix under which the denotation was constructed.
-     *  NoPrefix for SymDenotations.
+    /** For SymDenotation, this is NoPrefix. For other denotations this is the prefix
+     *  under which the denotation was constructed.
+     *
+     *  Note that `asSeenFrom` might return a `SymDenotation` and therefore in
+     *  general one cannot rely on `prefix` being set, see
+     *  `Config.reuseSymDenotations` for details.
      */
     def prefix: Type = NoPrefix
 
@@ -1044,24 +1048,25 @@ object Denotations {
       }
 
       /** The derived denotation with the given `info` transformed with `asSeenFrom`.
-       *  The prefix of the derived denotation is the new prefix `pre` if the type is
-       *  opaque, or if the current prefix is already different from `NoPrefix`.
-       *  That leaves SymDenotations (which have NoPrefix as the prefix), which are left
-       *  as SymDenotations unless the type is opaque. The treatment of opaque types
-       *  is needed, without it i7159.scala fails in from-tasty. Without the treatment,
-       *  opaque type denotations in subclasses are kept as SymDenotations, which means
-       *  that the transform in `ElimOpaque` will return the symbol's opaque alias without
-       *  adding the needed asSeenFrom.
        *
-       *  Logically, the right thing to do would be to extend the same treatment to all denotations
-       *  Currently this fails the bootstrap. There's also a concern that this generalization
-       *  would create more denotation objects, at a price in performance.
+       *  As a performance hack, we might reuse an existing SymDenotation,
+       *  instead of creating a new denotation with a given `prefix`,
+       *  see `Config.reuseSymDenotations`.
        */
       def derived(info: Type) =
-        derivedSingleDenotation(
-          symbol,
-          info.asSeenFrom(pre, owner),
-          if (symbol.is(Opaque) || this.prefix != NoPrefix) pre else this.prefix)
+        /** Do we need to return a denotation with a prefix set? */
+        def needsPrefix =
+          // For opaque types, the prefix is used in `ElimOpaques#transform`,
+          // without this i7159.scala would fail when compiled from tasty.
+          symbol.is(Opaque)
+
+        val derivedInfo = info.asSeenFrom(pre, owner)
+        if Config.reuseSymDenotations && this.isInstanceOf[SymDenotation]
+           && (derivedInfo eq info) && !needsPrefix then
+          this
+        else
+          derivedSingleDenotation(symbol, derivedInfo, pre)
+      end derived
 
       // Tt could happen that we see the symbol with prefix `this` as a member a different class
       // through a self type and that it then has a different info. In this case we have to go

compiler/src/dotty/tools/dotc/transform/LambdaLift.scala

@@ -556,6 +556,8 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisPhase =>
     // The Lifter updates the type of symbols using `installAfter` to give them a
     // new `SymDenotation`, but that doesn't affect non-sym denotations, so we
     // reload them manually here.
+    // Note: If you tweak this code, make sure to test your changes with
+    // `Config.reuseSymDenotations` set to false to exercise this path more.
     if denot.isInstanceOf[NonSymSingleDenotation] && lifter.free.contains(sym) then
       tree.qualifier.select(sym).withSpan(tree.span)
     else tree

compiler/src/dotty/tools/dotc/util/Signatures.scala

@@ -66,7 +66,7 @@ object Signatures {
           case _ =>
             val funSymbol = fun.symbol
             val alternatives = funSymbol.owner.info.member(funSymbol.name).alternatives
-            val alternativeIndex = alternatives.indexOf(funSymbol.denot) max 0
+            val alternativeIndex = alternatives.map(_.symbol).indexOf(funSymbol) max 0
             (alternativeIndex, alternatives)
         }