From 17d3051ff4643a77a5edb50a045ad53dd3f77712 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 May 2018 11:01:31 +0200 Subject: [PATCH 1/4] Change AccessProxies scheme Several fixes detected during refactorings of inliner --- compiler/src/dotty/tools/dotc/transform/AccessProxies.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala index c596daee5e2d..6f5841bdbd7c 100644 --- a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala +++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala @@ -49,6 +49,7 @@ abstract class AccessProxies { /** 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) + transforms.println(i"add accessors for $cls: $accDefs%, %") if (accDefs.isEmpty) body else body ++ accDefs } @@ -126,4 +127,4 @@ abstract class AccessProxies { object AccessProxies { def hostForAccessorOf(accessed: Symbol)(implicit ctx: Context): Symbol = ctx.owner.ownersIterator.findSymbol(_.derivesFrom(accessed.owner)) -} \ No newline at end of file +} From a9e9275fb1ff72754aa9c7a66c4df7161cf263f2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 May 2018 15:29:00 +0200 Subject: [PATCH 2/4] Change naming scheme for inline and protedcted accessors Use inline$x inline$x_= protected$x protected$x_= instead of inline_get$x inline_set$x protected_get$x protected_set$x Advantage: more idiomatic, and it's possible to make this work on untyped trees as well. The old scheme would have required accessors for every operator assignment when extending it to untyped trees. --- .../src/dotty/tools/dotc/core/NameKinds.scala | 6 +- .../src/dotty/tools/dotc/core/NameTags.scala | 10 +-- .../tools/dotc/core/tasty/TastyFormat.scala | 7 +- .../tools/dotc/transform/AccessProxies.scala | 69 ++++++++++++------- .../dotc/transform/ProtectedAccessors.scala | 33 ++++----- .../src/dotty/tools/dotc/typer/Inliner.scala | 24 +++++-- 6 files changed, 84 insertions(+), 65 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala index 9746b60bbd01..6474a1e084f2 100644 --- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala +++ b/compiler/src/dotty/tools/dotc/core/NameKinds.scala @@ -354,10 +354,8 @@ object NameKinds { val SuperAccessorName = new PrefixNameKind(SUPERACCESSOR, "super$") val InitializerName = new PrefixNameKind(INITIALIZER, "initial$") - val ProtectedGetterName = new PrefixNameKind(PROTECTEDGETTER, "protected_get$") - val ProtectedSetterName = new PrefixNameKind(PROTECTEDSETTER, "protected_set$") - val InlineGetterName = new PrefixNameKind(INLINEGETTER, "inline_get$") - val InlineSetterName = new PrefixNameKind(INLINESETTER, "inline_set$") + val ProtectedAccessorName = new PrefixNameKind(PROTECTEDACCESSOR, "protected$") + val InlineAccessorName = new PrefixNameKind(INLINEACCESSOR, "inline$") val AvoidClashName = new SuffixNameKind(AVOIDCLASH, "$_avoid_name_clash_$") val DirectMethodName = new SuffixNameKind(DIRECT, "$direct") { override def definesNewName = true } diff --git a/compiler/src/dotty/tools/dotc/core/NameTags.scala b/compiler/src/dotty/tools/dotc/core/NameTags.scala index 0c3c30f73f9c..f5c2923ffb11 100644 --- a/compiler/src/dotty/tools/dotc/core/NameTags.scala +++ b/compiler/src/dotty/tools/dotc/core/NameTags.scala @@ -15,9 +15,7 @@ object NameTags extends TastyFormat.NameTags { // outer accessor that will be filled in by ExplicitOuter. // indicates the number of hops needed to select the outer field. - final val PROTECTEDGETTER = 24 // The name of a protected getter `protected_get$` created by ProtectedAccessors. - - final val PROTECTEDSETTER = 25 // The name of a protected setter `protected_set$` created by ProtectedAccessors. + final val PROTECTEDACCESSOR = 24 // The name of a protected accesor `protected$` created by ProtectedAccessors. final val INITIALIZER = 26 // A mixin initializer method @@ -51,10 +49,8 @@ object NameTags extends TastyFormat.NameTags { case OUTERSELECT => "OUTERSELECT" case SUPERACCESSOR => "SUPERACCESSOR" - case INLINEGETTER => "INLINEGETTER" - case INLINESETTER => "INLINESETTER" - case PROTECTEDGETTER => "PROTECTEDGETTER" - case PROTECTEDSETTER => "PROTECTEDSETTER" + case INLINEACCESSOR => "INLINEACCESSOR" + case PROTECTEDACCESSOR => "PROTECTEDACCESSOR" case INITIALIZER => "INITIALIZER" case AVOIDCLASH => "AVOIDCLASH" case DIRECT => "DIRECT" diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala index 10fe328312d6..6e25c37c0f71 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala @@ -39,8 +39,7 @@ Macro-format: VARIANT Length underlying_NameRef variance_Nat // 0: Contravariant, 1: Covariant SUPERACCESSOR Length underlying_NameRef - INLINEGETTER Length underlying_NameRef - INLINESETTER Length underlying_NameRef + INLINEACCESSOR Length underlying_NameRef OBJECTCLASS Length underlying_NameRef SIGNED Length original_NameRef resultSig_NameRef paramSig_NameRef* @@ -252,9 +251,7 @@ object TastyFormat { final val SUPERACCESSOR = 20 // The name of a super accessor `super$name` created by SuperAccesors. - final val INLINEGETTER = 21 // The name of an inline getter `inline_get$name` - - final val INLINESETTER = 22 // The name of an inline setter `inline_set$name` + final val INLINEACCESSOR = 21 // The name of an inline accessor `inline$name` final val OBJECTCLASS = 23 // The name of an object class (or: module class) `$`. diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala index 6f5841bdbd7c..ab11a5a37211 100644 --- a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala +++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala @@ -6,6 +6,7 @@ import Contexts.Context import Symbols._ import Flags._ import Names._ +import NameOps._ import Decorators._ import TypeUtils._ import Annotations.Annotation @@ -23,9 +24,6 @@ abstract class AccessProxies { import ast.tpd._ import AccessProxies._ - def getterName: ClassifiedNameKind - def setterName: ClassifiedNameKind - /** accessor -> accessed */ private val accessedBy = newMutableSymbolMap[Symbol] @@ -38,7 +36,7 @@ abstract class AccessProxies { polyDefDef(accessor.asTerm, tps => argss => { val accessRef = ref(TermRef(cls.thisType, accessed)) val rhs = - if (accessor.name.is(setterName) && + if (accessor.name.isSetterName && argss.nonEmpty && argss.head.nonEmpty) // defensive conditions accessRef.becomes(argss.head.head) else @@ -56,6 +54,7 @@ abstract class AccessProxies { trait Insert { import ast.tpd._ + def accessorNameKind: ClassifiedNameKind def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean /** A fresh accessor symbol */ @@ -65,6 +64,30 @@ abstract class AccessProxies { sym } + private def rewire(reference: RefTree, accessor: Symbol)(implicit ctx: Context): Tree = { + reference match { + case Select(qual, _) => qual.select(accessor) + case Ident(name) => ref(accessor) + } + }.withPos(reference.pos) + + private def isAccessor(sym: Symbol, accessed: Symbol) = accessedBy.get(sym) == Some(accessed) + + def useSetter(getterRef: RefTree)(implicit ctx: Context): Tree = { + val getter = getterRef.symbol + val accessed = accessedBy(getter) + val accessedName = accessed.name.asTermName + val setterName = accessorNameKind(accessedName.setterName) + val setter = + getter.owner.info.decl(setterName).suchThat(isAccessor(_, accessed)).symbol.orElse { + val setterInfo = MethodType(getter.info.widenExpr :: Nil, defn.UnitType) + val setter = newAccessorSymbol(getter.owner, setterName, setterInfo, getter.pos) + accessedBy(setter) = accessed + setter + } + rewire(getterRef, setter) + } + /** Create an accessor unless one exists already, and replace the original * access with a reference to the accessor. * @@ -72,11 +95,10 @@ abstract class AccessProxies { * @param onLHS The reference is on the left-hand side of an assignment */ def useAccessor(reference: RefTree, onLHS: Boolean)(implicit ctx: Context): Tree = { + assert(!onLHS) - def nameKind = if (onLHS) setterName else getterName val accessed = reference.symbol.asTerm - def refersToAccessed(sym: Symbol) = accessedBy.get(sym) == Some(accessed) var accessorClass = hostForAccessorOf(accessed: Symbol) if (!accessorClass.exists) { @@ -87,26 +109,23 @@ abstract class AccessProxies { accessorClass = curCls } - val accessorRawInfo = - if (onLHS) MethodType(accessed.info :: Nil, defn.UnitType) - else accessed.info.ensureMethodic - val accessorInfo = - accessorRawInfo.asSeenFrom(accessorClass.thisType, accessed.owner) - val accessorName = nameKind(accessed.name) - - val accessorSymbol = - accessorClass.info.decl(accessorName).suchThat(refersToAccessed).symbol - .orElse { - val acc = newAccessorSymbol(accessorClass, accessorName, accessorInfo, accessed.pos) - accessedBy(acc) = accessed - acc - } - - { reference match { - case Select(qual, _) => qual.select(accessorSymbol) - case Ident(name) => ref(accessorSymbol) + val accessorName = accessorNameKind( + if (onLHS) accessed.name.setterName else accessed.name) + + + val accessor = + accessorClass.info.decl(accessorName).suchThat(isAccessor(_, accessed)).symbol.orElse { + val accessorRawInfo = + if (onLHS) MethodType(accessed.info :: Nil, defn.UnitType) + else accessed.info.ensureMethodic + val accessorInfo = + accessorRawInfo.asSeenFrom(accessorClass.thisType, accessed.owner) + + val acc = newAccessorSymbol(accessorClass, accessorName, accessorInfo, accessed.pos) + accessedBy(acc) = accessed + acc } - }.withPos(reference.pos) + rewire(reference, accessor) } /** Replace tree with a reference to an accessor if needed */ diff --git a/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala index 84d3db353b49..401aa6f900a3 100644 --- a/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala +++ b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala @@ -8,7 +8,6 @@ import core.Flags._ import core.Decorators._ import MegaPhase.MiniPhase import ast.Trees._ -import util.Property /** Add accessors for all protected accesses. An accessor is needed if * according to the rules of the JVM a protected class member is not accesissible @@ -18,8 +17,6 @@ import util.Property object ProtectedAccessors { val name = "protectedAccessors" - private val LHS = new Property.StickyKey[Unit] - /** Is the current context's owner inside the access boundary established by `sym`? */ def insideBoundaryOf(sym: Symbol)(implicit ctx: Context): Boolean = { if (sym.is(JavaDefined)) { @@ -56,32 +53,30 @@ class ProtectedAccessors extends MiniPhase { override def phaseName = ProtectedAccessors.name object Accessors extends AccessProxies { - def getterName = ProtectedGetterName - def setterName = ProtectedSetterName - val insert = new Insert { + def accessorNameKind = ProtectedAccessorName def needsAccessor(sym: Symbol)(implicit ctx: Context) = ProtectedAccessors.needsAccessor(sym) } } - override def prepareForAssign(tree: Assign)(implicit ctx: Context) = { - tree.lhs match { - case lhs: RefTree if needsAccessor(lhs.symbol) => lhs.putAttachment(LHS, ()) - case _ => - } - ctx - } - - private def isLHS(tree: RefTree) = tree.removeAttachment(LHS).isDefined - override def transformIdent(tree: Ident)(implicit ctx: Context): Tree = - if (isLHS(tree)) tree else Accessors.insert.accessorIfNeeded(tree) + Accessors.insert.accessorIfNeeded(tree) override def transformSelect(tree: Select)(implicit ctx: Context): Tree = - if (isLHS(tree)) tree else Accessors.insert.accessorIfNeeded(tree) + Accessors.insert.accessorIfNeeded(tree) override def transformAssign(tree: Assign)(implicit ctx: Context): Tree = - Accessors.insert.accessorIfNeeded(tree) + tree.lhs match { + case lhs: RefTree => + lhs.name match { + case ProtectedAccessorName(name) => + cpy.Apply(tree)(Accessors.insert.useSetter(lhs), tree.rhs :: Nil) + case _ => + tree + } + case _ => + tree + } override def transformTemplate(tree: Template)(implicit ctx: Context): Tree = cpy.Template(tree)(body = Accessors.addAccessorDefs(tree.symbol.owner, tree.body)) diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index eac354893f06..5e3b99cf1e2f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -15,7 +15,7 @@ import StdNames.nme import Contexts.Context import Names.{Name, TermName, EmptyTermName} import NameOps._ -import NameKinds.{ClassifiedNameKind, InlineGetterName, InlineSetterName} +import NameKinds.{ClassifiedNameKind, InlineAccessorName} import ProtoTypes.selectionProto import SymDenotations.SymDenotation import Annotations._ @@ -32,13 +32,12 @@ object Inliner { import tpd._ class InlineAccessors extends AccessProxies { - def getterName = InlineGetterName - def setterName = InlineSetterName /** A tree map which inserts accessors for all non-public term members accessed * from inlined code. Accessors are collected in the `accessors` buffer. */ class MakeInlineable(inlineSym: Symbol) extends TreeMap with Insert { + def accessorNameKind = InlineAccessorName /** A definition needs an accessor if it is private, protected, or qualified private * and it is not part of the tree that gets inlined. The latter test is implemented @@ -54,8 +53,23 @@ object Inliner { // This is quite tricky, as such types can appear anywhere, including as parts // of types of other things. For the moment we do nothing and complain // at the implicit expansion site if there's a reference to an inaccessible type. - override def transform(tree: Tree)(implicit ctx: Context): Tree = - super.transform(accessorIfNeeded(tree)) + override def transform(tree: Tree)(implicit ctx: Context): Tree = tree match { + case tree: Assign => + transform(tree.lhs) match { + case lhs1: RefTree => + lhs1.name match { + case InlineAccessorName(name) => + cpy.Apply(tree)(useSetter(lhs1), transform(tree.rhs) :: Nil) + case _ => + super.transform(tree) + } + case _ => + super.transform(tree) + } + case _ => + super.transform(accessorIfNeeded(tree)) + } + } /** Adds accessors for all non-public term members accessed From 5a42cfaf192300f4617b04f3c0cd8a1a8da494ba Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 May 2018 15:42:36 +0200 Subject: [PATCH 3/4] Polishings --- .../tools/dotc/transform/AccessProxies.scala | 59 ++++++++----------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala index ab11a5a37211..f5b207926f8a 100644 --- a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala +++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala @@ -58,12 +58,23 @@ abstract class AccessProxies { def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean /** A fresh accessor symbol */ - def newAccessorSymbol(owner: Symbol, name: TermName, info: Type, pos: Position)(implicit ctx: Context): TermSymbol = { + private def newAccessorSymbol(owner: Symbol, name: TermName, info: Type, pos: Position)(implicit ctx: Context): TermSymbol = { val sym = ctx.newSymbol(owner, name, Synthetic | Method, info, coord = pos).entered if (sym.allOverriddenSymbols.exists(!_.is(Deferred))) sym.setFlag(Override) sym } + /** An accessor symbol, create a fresh one unless one exists already */ + private def accessorSymbol(owner: Symbol, accessorName: TermName, accessorInfo: Type, accessed: Symbol)(implicit ctx: Context) = { + def refersToAccessed(sym: Symbol) = accessedBy.get(sym) == Some(accessed) + owner.info.decl(accessorName).suchThat(refersToAccessed).symbol.orElse { + val acc = newAccessorSymbol(owner, accessorName, accessorInfo, accessed.pos) + accessedBy(acc) = accessed + acc + } + } + + /** Rewire reference to refer to `accessor` symbol */ private def rewire(reference: RefTree, accessor: Symbol)(implicit ctx: Context): Tree = { reference match { case Select(qual, _) => qual.select(accessor) @@ -71,20 +82,14 @@ abstract class AccessProxies { } }.withPos(reference.pos) - private def isAccessor(sym: Symbol, accessed: Symbol) = accessedBy.get(sym) == Some(accessed) - + /** Given a reference to a getter accessor, the corresponding setter reference */ def useSetter(getterRef: RefTree)(implicit ctx: Context): Tree = { val getter = getterRef.symbol val accessed = accessedBy(getter) val accessedName = accessed.name.asTermName val setterName = accessorNameKind(accessedName.setterName) - val setter = - getter.owner.info.decl(setterName).suchThat(isAccessor(_, accessed)).symbol.orElse { - val setterInfo = MethodType(getter.info.widenExpr :: Nil, defn.UnitType) - val setter = newAccessorSymbol(getter.owner, setterName, setterInfo, getter.pos) - accessedBy(setter) = accessed - setter - } + val setterInfo = MethodType(getter.info.widenExpr :: Nil, defn.UnitType) + val setter = accessorSymbol(getter.owner, setterName, setterInfo, accessed) rewire(getterRef, setter) } @@ -94,12 +99,8 @@ abstract class AccessProxies { * @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 = { - assert(!onLHS) - + def useAccessor(reference: RefTree)(implicit ctx: Context): Tree = { val accessed = reference.symbol.asTerm - - var accessorClass = hostForAccessorOf(accessed: Symbol) if (!accessorClass.exists) { val curCls = ctx.owner.enclosingClass @@ -108,23 +109,10 @@ abstract class AccessProxies { reference.pos) accessorClass = curCls } - - val accessorName = accessorNameKind( - if (onLHS) accessed.name.setterName else accessed.name) - - - val accessor = - accessorClass.info.decl(accessorName).suchThat(isAccessor(_, accessed)).symbol.orElse { - val accessorRawInfo = - if (onLHS) MethodType(accessed.info :: Nil, defn.UnitType) - else accessed.info.ensureMethodic - val accessorInfo = - accessorRawInfo.asSeenFrom(accessorClass.thisType, accessed.owner) - - val acc = newAccessorSymbol(accessorClass, accessorName, accessorInfo, accessed.pos) - accessedBy(acc) = accessed - acc - } + val accessorName = accessorNameKind(accessed.name) + val accessorInfo = + accessed.info.ensureMethodic.asSeenFrom(accessorClass.thisType, accessed.owner) + val accessor = accessorSymbol(accessorClass, accessorName, accessorInfo, accessed) rewire(reference, accessor) } @@ -135,15 +123,16 @@ abstract class AccessProxies { 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)) + else useAccessor(tree) case _ => tree } } } object AccessProxies { + /** Where an accessor for the `accessed` symbol should be placed. + * This is the closest enclosing class that has `accessed` as a member. + */ def hostForAccessorOf(accessed: Symbol)(implicit ctx: Context): Symbol = ctx.owner.ownersIterator.findSymbol(_.derivesFrom(accessed.owner)) } From 266a696987b84ed48e329bc822a272fb4d265226 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 May 2018 16:03:45 +0200 Subject: [PATCH 4/4] Allow user-defined setters as targets of `becomes` in `tpd` --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index f78a6610a8a9..e77b56e4c27f 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -836,12 +836,15 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { tree.select(defn.Boolean_||).appliedTo(that) /** The translation of `tree = rhs`. - * This is either the tree as an assignment, to a setter call. + * This is either the tree as an assignment, or a setter call. */ - def becomes(rhs: Tree)(implicit ctx: Context): Tree = - if (tree.symbol is Method) { - val setter = tree.symbol.setter - assert(setter.exists, tree.symbol.showLocated) + def becomes(rhs: Tree)(implicit ctx: Context): Tree = { + val sym = tree.symbol + if (sym is Method) { + val setter = sym.setter.orElse { + assert(sym.name.isSetterName && sym.info.firstParamTypes.nonEmpty) + sym + } val qual = tree match { case id: Ident => desugarIdentPrefix(id) case Select(qual, _) => qual @@ -849,6 +852,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { qual.select(setter).appliedTo(rhs) } else Assign(tree, rhs) + } /** A synthetic select with that will be turned into an outer path by ExplicitOuter. * @param levels How many outer levels to select