Merge pull request #4468 from dotty-staging/fix-#4322

Fix #4322: Avoid generating multiple inline accessors for the same ac…

Author: odersky

compiler/src/dotty/tools/dotc/CompilationUnit.scala

@@ -1,10 +1,11 @@
 package dotty.tools
 package dotc
 
-import dotty.tools.dotc.core.Types.Type // Do not remove me #3383
+import core.Types.Type // Do not remove me #3383
 import util.SourceFile
 import ast.{tpd, untpd}
-import dotty.tools.dotc.ast.tpd.{ Tree, TreeTraverser }
+import tpd.{ Tree, TreeTraverser }
+import typer.Inliner.InlineAccessors
 import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.core.SymDenotations.ClassDenotation
 import dotty.tools.dotc.core.Symbols._
@@ -27,6 +28,9 @@ class CompilationUnit(val source: SourceFile) {
    *  is used in phase ReifyQuotes in order to avoid traversing a quote-less tree.
    */
   var containsQuotesOrSplices: Boolean = false
+
+  /** A structure containing a temporary map for generating inline accessors */
+  val inlineAccessors = new InlineAccessors
 }
 
 object CompilationUnit {

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

@@ -275,7 +275,6 @@ object NameKinds {
   }
 
   /** Other unique names */
-  val InlineAccessorName      = new UniqueNameKind("$_inlineAccessor_$")
   val TempResultName          = new UniqueNameKind("ev$")
   val EvidenceParamName       = new UniqueNameKind("evidence$")
   val DepParamName            = new UniqueNameKind("(param)")
@@ -356,6 +355,9 @@ object NameKinds {
   val InitializerName = new PrefixNameKind(INITIALIZER, "initial$")
   val ProtectedAccessorName = new PrefixNameKind(PROTECTEDACCESSOR, "protected$")
   val ProtectedSetterName = new PrefixNameKind(PROTECTEDSETTER, "protected$set") // dubious encoding, kept for Scala2 compatibility
+  val InlineGetterName = new PrefixNameKind(INLINEGETTER, "inline_get$")
+  val InlineSetterName = new PrefixNameKind(INLINESETTER, "inline_set$")
+
   val AvoidClashName = new SuffixNameKind(AVOIDCLASH, "$_avoid_name_clash_$")
   val DirectMethodName = new SuffixNameKind(DIRECT, "$direct") { override def definesNewName = true }
   val FieldName = new SuffixNameKind(FIELD, "$$local") {

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

@@ -15,23 +15,23 @@ object NameTags extends TastyFormat.NameTags {
                                 // outer accessor that will be filled in by ExplicitOuter.
                                 // <num> indicates the number of hops needed to select the outer field.
 
-  final val INITIALIZER = 24    // A mixin initializer method
+  final val INITIALIZER = 26    // A mixin initializer method
 
-  final val AVOIDCLASH = 25     // Adds a suffix to avoid a name clash;
+  final val AVOIDCLASH = 27     // Adds a suffix to avoid a name clash;
                                 // Used in FirstTransform for synthesized companion objects of classes
                                 // if they would clash with another value.
 
-  final val DIRECT = 26         // Used by ShortCutImplicits for the name of methods that
+  final val DIRECT = 28         // Used by ShortCutImplicits for the name of methods that
                                 // implement implicit function result types directly.
 
-  final val FIELD = 27          // Used by Memoize to tag the name of a class member field.
+  final val FIELD = 29          // Used by Memoize to tag the name of a class member field.
 
-  final val EXTMETH = 28        // Used by ExtensionMethods for the name of an extension method
+  final val EXTMETH = 30        // Used by ExtensionMethods for the name of an extension method
                                 // implementing a value class method.
 
-  final val ADAPTEDCLOSURE = 29 // Used in Erasure to adapt closures over primitive types.
+  final val ADAPTEDCLOSURE = 31 // Used in Erasure to adapt closures over primitive types.
 
-  final val IMPLMETH = 30       // Used to define methods in implementation classes
+  final val IMPLMETH = 32       // Used to define methods in implementation classes
                                 // (can probably be removed).
 
   def nameTagToString(tag: Int): String = tag match {

compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala

@@ -227,7 +227,7 @@ object TastyFormat {
 
   final val header = Array(0x5C, 0xA1, 0xAB, 0x1F)
   val MajorVersion = 7
-  val MinorVersion = 0
+  val MinorVersion = 1
 
   /** Tags used to serialize names */
   class NameTags {
@@ -260,6 +260,10 @@ object TastyFormat {
 
     final val OBJECTCLASS = 23       // The name of an object class (or: module class) `<name>$`.
 
+    final val INLINEGETTER = 24     // The name of an inline getter `inline_get$name`
+
+    final val INLINESETTER = 25     // The name of an inline setter `inline_set$name`
+
     final val SIGNED = 63            // A pair of a name and a signature, used to idenitfy
                                      // possibly overloaded methods.
   }

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

@@ -0,0 +1,109 @@
+package dotty.tools.dotc
+package transform
+
+import core._
+import Contexts.Context
+import Symbols._
+import Flags._
+import Names._
+import Decorators._
+import TypeUtils._
+import Annotations.Annotation
+import Types._
+import NameKinds.ClassifiedNameKind
+import ast.Trees._
+import util.Property
+import util.Positions.Position
+
+/** A utility class for generating access proxies. Currently used for
+ *  inline accessors and protected accessors.
+ */
+abstract class AccessProxies {
+  import ast.tpd._
+
+  def getterName: ClassifiedNameKind
+  def setterName: ClassifiedNameKind
+
+  /** accessor -> accessed */
+  private val accessedBy = newMutableSymbolMap[Symbol]
+
+  /** The accessor definitions that need to be added to class `cls`
+   *  As a side-effect, this method removes entries from the `accessedBy` map.
+   *  So a second call of the same method will yield the empty list.
+   */
+  private def accessorDefs(cls: Symbol)(implicit ctx: Context): Iterator[DefDef] =
+    for (accessor <- cls.info.decls.iterator; accessed <- accessedBy.remove(accessor)) yield
+      polyDefDef(accessor.asTerm, tps => argss => {
+        val accessRef = ref(TermRef(cls.thisType, accessed))
+        val rhs =
+          if (accessor.name.is(setterName) &&
+              argss.nonEmpty && argss.head.nonEmpty) // defensive conditions
+            accessRef.becomes(argss.head.head)
+          else
+            accessRef.appliedToTypes(tps).appliedToArgss(argss)
+        rhs.withPos(accessed.pos)
+      })
+
+  /** Add all needed accessors to the `body` of class `cls` */
+  def addAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = {
+    val accDefs = accessorDefs(cls)
+    if (accDefs.isEmpty) body else body ++ accDefs
+  }
+
+  trait Insert {
+    import ast.tpd._
+
+    def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean
+
+    /** A fresh accessor symbol */
+    def newAccessorSymbol(accessed: Symbol, name: TermName, info: Type)(implicit ctx: Context): TermSymbol =
+      ctx.newSymbol(accessed.owner.enclosingSubClass, name, Synthetic | Method,
+                    info, coord = accessed.pos).entered
+
+    /** Create an accessor unless one exists already, and replace the original
+      *  access with a reference to the accessor.
+      *
+      *  @param reference    The original reference to the non-public symbol
+      *  @param onLHS        The reference is on the left-hand side of an assignment
+      */
+    def useAccessor(reference: RefTree, onLHS: Boolean)(implicit ctx: Context): Tree = {
+
+      def nameKind = if (onLHS) setterName else getterName
+      val accessed = reference.symbol.asTerm
+
+      def refersToAccessed(sym: Symbol) = accessedBy.get(sym) == Some(accessed)
+
+      val accessorInfo =
+        if (onLHS) MethodType(accessed.info :: Nil, defn.UnitType)
+        else accessed.info.ensureMethodic
+      val accessorName = nameKind(accessed.name)
+      val accessorSymbol =
+        accessed.owner.info.decl(accessorName).suchThat(refersToAccessed).symbol
+          .orElse {
+            val acc = newAccessorSymbol(accessed, accessorName, accessorInfo)
+            accessedBy(acc) = accessed
+            acc
+          }
+
+      { reference match {
+          case Select(qual, _) => qual.select(accessorSymbol)
+          case Ident(name) => ref(accessorSymbol)
+        }
+      }.withPos(reference.pos)
+    }
+
+    /** Replace tree with a reference to an accessor if needed */
+    def accessorIfNeeded(tree: Tree)(implicit ctx: Context): Tree = tree match {
+      case tree: RefTree if needsAccessor(tree.symbol) =>
+        if (tree.symbol.isConstructor) {
+          ctx.error("Implementation restriction: cannot use private constructors in inline methods", tree.pos)
+          tree // TODO: create a proper accessor for the private constructor
+        }
+        else useAccessor(tree, onLHS = false)
+      case Assign(lhs: RefTree, rhs) if needsAccessor(lhs.symbol) =>
+        cpy.Apply(tree)(useAccessor(lhs, onLHS = true), List(rhs))
+      case _ =>
+        tree
+    }
+  }
+}

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

@@ -20,6 +20,7 @@ import typer.ProtoTypes._
 import typer.ErrorReporting._
 import core.TypeErasure._
 import core.Decorators._
+import core.NameKinds._
 import dotty.tools.dotc.ast.{Trees, tpd, untpd}
 import ast.Trees._
 import scala.collection.mutable.ListBuffer
@@ -30,7 +31,6 @@ import ExplicitOuter._
 import core.Mode
 import reporting.trace
 
-
 class Erasure extends Phase with DenotTransformer {
 
   override def phaseName: String = Erasure.name
@@ -315,6 +315,10 @@ object Erasure {
       }
   }
 
+  /** The erasure typer.
+   *  Also inserts protected accessors where needed. This logic is placed here
+   *  since it is most naturally done in a macro transform.
+   */
   class Typer extends typer.ReTyper with NoChecking {
     import Boxing._
 
@@ -323,6 +327,23 @@ object Erasure {
       if (tree.isTerm) erasedRef(tp) else valueErasure(tp)
     }
 
+    object ProtectedAccessors extends AccessProxies {
+      def getterName = ProtectedAccessorName
+      def setterName = ProtectedSetterName
+
+      val insert = new Insert {
+        def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean =
+          false &&
+          sym.isTerm && sym.is(Flags.Protected) &&
+          ctx.owner.enclosingPackageClass != sym.enclosingPackageClass &&
+          !ctx.owner.enclosingClass.derivesFrom(sym.owner) &&
+            { println(i"need protected acc $sym accessed from ${ctx.owner}"); assert(false); false }
+        }
+    }
+
+    override def addAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] =
+      ProtectedAccessors.addAccessorDefs(cls, body)
+
     override def promote(tree: untpd.Tree)(implicit ctx: Context): tree.ThisTree[Type] = {
       assert(tree.hasType)
       val erasedTp = erasedType(tree)
@@ -357,6 +378,9 @@ object Erasure {
       else
         super.typedLiteral(tree)
 
+    override def typedIdent(tree: untpd.Ident, pt: Type)(implicit ctx: Context): Tree =
+      ProtectedAccessors.insert.accessorIfNeeded(super.typedIdent(tree, pt))
+
     /** Type check select nodes, applying the following rewritings exhaustively
      *  on selections `e.m`, where `OT` is the type of the owner of `m` and `ET`
      *  is the erased type of the selection's original qualifier expression.
@@ -437,9 +461,12 @@ object Erasure {
         }
       }
 
-      recur(typed(tree.qualifier, AnySelectionProto))
+      ProtectedAccessors.insert.accessorIfNeeded(recur(typed(tree.qualifier, AnySelectionProto)))
     }
 
+    override def typedAssign(tree: untpd.Assign, pt: Type)(implicit ctx: Context): Tree =
+      ProtectedAccessors.insert.accessorIfNeeded(super.typedAssign(tree, pt))
+
     override def typedThis(tree: untpd.This)(implicit ctx: Context): Tree =
       if (tree.symbol == ctx.owner.lexicallyEnclosingClass || tree.symbol.isStaticOwner) promote(tree)
       else {