From 2a5abb34bcf09959120ca45e60bdaf7beba2127e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 6 May 2018 14:59:16 +0200 Subject: [PATCH 1/6] Fix #4322: Avoid generating more than one definition for an inline accessor --- .../src/dotty/tools/dotc/typer/Inliner.scala | 123 ++++++++++-------- tests/pos/i4322.scala | 10 ++ 2 files changed, 78 insertions(+), 55 deletions(-) create mode 100644 tests/pos/i4322.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 1cb9bf0936ae..b4e48babe6b7 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -49,6 +49,10 @@ object Inliner { object addAccessors extends TreeMap { val accessors = new mutable.ListBuffer[MemberDef] + type AccessorMap = mutable.HashMap[Symbol, DefDef] + private val getters = new AccessorMap + private val setters = new AccessorMap + /** 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 * by excluding all symbols properly contained in the inlined method. @@ -85,71 +89,77 @@ object Inliner { * @param rhs A function that builds the right-hand side of the accessor, * given a reference to the accessed symbol and any type and * value arguments the need to be integrated. + * @param seen An map of already generated accessor methods of this kind (getter or setter) * @return The call to the accessor method that replaces the original access. */ def addAccessor(tree: Tree, refPart: Tree, targs: List[Tree], argss: List[List[Tree]], - accessedType: Type, rhs: (Tree, List[Type], List[List[Tree]]) => Tree)(implicit ctx: Context): Tree = { + accessedType: Type, rhs: (Tree, List[Type], List[List[Tree]]) => Tree, + seen: AccessorMap)(implicit ctx: Context): Tree = { val qual = qualifier(refPart) + def refIsLocal = qual match { case qual: This => qual.symbol == refPart.symbol.owner case _ => false } - val (accessorDef, accessorRef) = - if (refPart.symbol.isStatic || refIsLocal) { - // Easy case: Reference to a static symbol or a symbol referenced via `this.` - val accessorType = accessedType.ensureMethodic - val accessor = accessorSymbol(tree, accessorType).asTerm - val accessorDef = polyDefDef(accessor, tps => argss => - rhs(refPart, tps, argss).withPos(tree.pos.focus)) - val accessorRef = ref(accessor).appliedToTypeTrees(targs).appliedToArgss(argss).withPos(tree.pos) - (accessorDef, accessorRef) - } else { - // Hard case: Reference needs to go via a dynamic prefix - inlining.println(i"adding inline accessor for $tree -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))") - - // Need to dealias in order to catch all possible references to abstracted over types in - // substitutions - val dealiasMap = new TypeMap { - def apply(t: Type) = mapOver(t.dealias) - } - val qualType = dealiasMap(qual.tpe.widen) + def accessorDef(accessorType: Type, accessorDefFn: TermSymbol => DefDef): DefDef = + seen.getOrElseUpdate(refPart.symbol, { + val acc = accessorSymbol(tree, accessorType).asTerm + val accessorDef = accessorDefFn(acc) + accessors += accessorDef + inlining.println(i"added inline accessor: $accessorDef") + accessorDef + }) + + if (refPart.symbol.isStatic || refIsLocal) { + // Easy case: Reference to a static symbol or a symbol referenced via `this.` + val accDef = accessorDef( + accessedType.ensureMethodic, + polyDefDef(_, tps => argss => rhs(refPart, tps, argss).withPos(tree.pos.focus))) - // Add qualifier type as leading method argument to argument `tp` - def addQualType(tp: Type): Type = tp match { - case tp: PolyType => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, addQualType(tp.resultType)) - case tp: ExprType => addQualType(tp.resultType) - case tp => MethodType(qualType :: Nil, tp) - } + ref(accDef.symbol).appliedToTypeTrees(targs).appliedToArgss(argss).withPos(tree.pos) + } + else { + // Hard case: Reference needs to go via a dynamic prefix + inlining.println(i"adding inline accessor for $tree -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))") + + // Need to dealias in order to catch all possible references to abstracted over types in + // substitutions + val dealiasMap = new TypeMap { + def apply(t: Type) = mapOver(t.dealias) + } - // The types that are local to the inlined method, and that therefore have - // to be abstracted out in the accessor, which is external to the inlined method - val localRefs = qualType.namedPartsWith(ref => - ref.isType && ref.symbol.isContainedIn(inlineMethod)).toList + val qualType = dealiasMap(qual.tpe.widen) - // Abstract accessed type over local refs - def abstractQualType(mtpe: Type): Type = - if (localRefs.isEmpty) mtpe - else PolyType.fromParams(localRefs.map(_.symbol.asType), mtpe) - .asInstanceOf[PolyType].flatten + // Add qualifier type as leading method argument to argument `tp` + def addQualType(tp: Type): Type = tp match { + case tp: PolyType => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, addQualType(tp.resultType)) + case tp: ExprType => addQualType(tp.resultType) + case tp => MethodType(qualType :: Nil, tp) + } + + // The types that are local to the inlined method, and that therefore have + // to be abstracted out in the accessor, which is external to the inlined method + val localRefs = qualType.namedPartsWith(ref => + ref.isType && ref.symbol.isContainedIn(inlineMethod)).toList - val accessorType = abstractQualType(addQualType(dealiasMap(accessedType))) - val accessor = accessorSymbol(tree, accessorType).asTerm + // Abstract accessed type over local refs + def abstractQualType(mtpe: Type): Type = + if (localRefs.isEmpty) mtpe + else PolyType.fromParams(localRefs.map(_.symbol.asType), mtpe) + .asInstanceOf[PolyType].flatten - val accessorDef = polyDefDef(accessor, tps => argss => + val accDef = accessorDef( + abstractQualType(addQualType(dealiasMap(accessedType))), + polyDefDef(_, tps => argss => rhs(argss.head.head.select(refPart.symbol), tps.drop(localRefs.length), argss.tail) - .withPos(tree.pos.focus) - ) - - val accessorRef = ref(accessor) - .appliedToTypeTrees(localRefs.map(TypeTree(_)) ++ targs) - .appliedToArgss((qual :: Nil) :: argss) - .withPos(tree.pos) - (accessorDef, accessorRef) - } - accessors += accessorDef - inlining.println(i"added inline accessor: $accessorDef") - accessorRef + .withPos(tree.pos.focus))) + + ref(accDef.symbol) + .appliedToTypeTrees(localRefs.map(TypeTree(_)) ++ targs) + .appliedToArgss((qual :: Nil) :: argss) + .withPos(tree.pos) + } } override def transform(tree: Tree)(implicit ctx: Context): Tree = super.transform { @@ -160,12 +170,14 @@ object Inliner { if (methPart.symbol.isConstructor && needsAccessor(methPart.symbol)) { ctx.error("Cannot use private constructors in inline methods", tree.pos) tree // TODO: create a proper accessor for the private constructor - } else { + } + else addAccessor(tree, methPart, targs, argss, accessedType = methPart.tpe.widen, - rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss)) - } - } else { + rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss), + seen = getters) + } + else { // TODO: Handle references to non-public types. // 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 @@ -181,7 +193,8 @@ object Inliner { case Assign(lhs: RefTree, rhs) if needsAccessor(lhs.symbol) => addAccessor(tree, lhs, Nil, (rhs :: Nil) :: Nil, accessedType = MethodType(rhs.tpe.widen :: Nil, defn.UnitType), - rhs = (lhs, tps, argss) => lhs.becomes(argss.head.head)) + rhs = (lhs, tps, argss) => lhs.becomes(argss.head.head), + seen = setters) case _ => tree } } diff --git a/tests/pos/i4322.scala b/tests/pos/i4322.scala new file mode 100644 index 000000000000..2994126d2e6e --- /dev/null +++ b/tests/pos/i4322.scala @@ -0,0 +1,10 @@ +object Foo { + private[this] var x: Int = 1 + + inline def foo: Int = x + x + x + + inline def bar = { + x += 1 + x += 1 + } +} From 9cde3dd8556d2eb688fca9deff8bd85dec0eff0f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 21 May 2018 14:51:02 +0200 Subject: [PATCH 2/6] New annotation on inline accessors scala.annottation.internal.Accessed indicates which symbol is accessed by an inline accessor. --- .../dotty/tools/dotc/core/Annotations.scala | 23 +++++++++++++++---- .../dotty/tools/dotc/core/Definitions.scala | 2 ++ .../core/unpickleScala2/Scala2Unpickler.scala | 2 +- .../scala/annotation/internal/Accessed.scala | 8 +++++++ 4 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 library/src/scala/annotation/internal/Accessed.scala diff --git a/compiler/src/dotty/tools/dotc/core/Annotations.scala b/compiler/src/dotty/tools/dotc/core/Annotations.scala index 2da8c95f0b71..712a2256b3aa 100644 --- a/compiler/src/dotty/tools/dotc/core/Annotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Annotations.scala @@ -146,11 +146,26 @@ object Annotations { def deferredResolve(atp: Type, args: List[Tree])(implicit ctx: Context): Annotation = deferred(atp.classSymbol, implicit ctx => resolveConstructor(atp, args)) - def makeAlias(sym: TermSymbol)(implicit ctx: Context) = - apply(defn.AliasAnnot, List( - ref(TermRef(sym.owner.thisType, sym.name, sym)))) + class TermRefAnnotExtractor(annotFn: Context => Symbol) { - /** Extractor for child annotations */ + def apply(sym: TermSymbol)(implicit ctx: Context): Annotation = + Annotation(annotFn(ctx).typeRef, List(ref(TermRef(sym.owner.thisType, sym.name, sym)))) + + def unapply(ann: Annotation)(implicit ctx: Context): Option[Symbol] = + if (ann.symbol == annotFn(ctx)) { + val ast.Trees.Apply(fn, arg :: Nil) = ann.tree + Some(arg.symbol) + } + else None + } + + /** Extractor for "accessed" annotations */ + object Accessed extends TermRefAnnotExtractor(implicit ctx => defn.AccessedAnnot) + + /** Extractor for "aliased" annotations */ + object Alias extends TermRefAnnotExtractor(implicit ctx => defn.AliasAnnot) + + /** Extractor for "child" annotations */ object Child { /** A deferred annotation to the result of a given child computation */ diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index ef5d0d1f7022..86d78b41c48b 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -692,6 +692,8 @@ class Definitions { // Annotation classes lazy val AliasAnnotType = ctx.requiredClassRef("scala.annotation.internal.Alias") def AliasAnnot(implicit ctx: Context) = AliasAnnotType.symbol.asClass + lazy val AccessedAnnotType = ctx.requiredClassRef("scala.annotation.internal.Accessed") + def AccessedAnnot(implicit ctx: Context) = AccessedAnnotType.symbol.asClass lazy val AnnotationDefaultAnnotType = ctx.requiredClassRef("scala.annotation.internal.AnnotationDefault") def AnnotationDefaultAnnot(implicit ctx: Context) = AnnotationDefaultAnnotType.symbol.asClass lazy val BodyAnnotType = ctx.requiredClassRef("scala.annotation.internal.Body") diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index f7dbda218e0e..a8a26c24f2b4 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -578,7 +578,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas } } val alias = readDisambiguatedSymbolRef(disambiguate).asTerm - denot.addAnnotation(Annotation.makeAlias(alias)) + denot.addAnnotation(Annotation.Alias(alias)) } } // println(s"unpickled ${denot.debugString}, info = ${denot.info}") !!! DEBUG diff --git a/library/src/scala/annotation/internal/Accessed.scala b/library/src/scala/annotation/internal/Accessed.scala new file mode 100644 index 000000000000..cf365ea801e9 --- /dev/null +++ b/library/src/scala/annotation/internal/Accessed.scala @@ -0,0 +1,8 @@ +package scala.annotation.internal + +import scala.annotation.Annotation + +/** An annotation to record the method accessed by an inline accessor. + * @param aliased A TermRef pointing to the accessed method + */ +class Accessed(aliased: Any) extends Annotation From 1986c035692861c40e75dc0e1855b69e765c2297 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 21 May 2018 18:03:38 +0200 Subject: [PATCH 3/6] Create only one inline accessor per accessed member Revise scheme how inline accessors are created so that exactly one inline accessor is created for each member that needs one. --- .../src/dotty/tools/dotc/core/NameKinds.scala | 4 +- .../src/dotty/tools/dotc/core/NameTags.scala | 14 +- .../tools/dotc/core/tasty/TastyFormat.scala | 6 +- .../src/dotty/tools/dotc/typer/Inliner.scala | 200 +++++------------- .../src/dotty/tools/dotc/typer/ReTyper.scala | 4 +- .../src/dotty/tools/dotc/typer/Typer.scala | 17 +- tests/pos/i4322.scala | 6 +- 7 files changed, 90 insertions(+), 161 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala index bf758ac77300..ffe221c02b63 100644 --- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala +++ b/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") { diff --git a/compiler/src/dotty/tools/dotc/core/NameTags.scala b/compiler/src/dotty/tools/dotc/core/NameTags.scala index 11c890823100..fd65c5243fab 100644 --- a/compiler/src/dotty/tools/dotc/core/NameTags.scala +++ b/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. // 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 { diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala index c273b27bc0a6..962acf20b759 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala +++ b/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) `$`. + 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. } diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index b4e48babe6b7..952db2b06482 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.InlineAccessorName +import NameKinds.{ClassifiedNameKind, InlineGetterName, InlineSetterName} import ProtoTypes.selectionProto import SymDenotations.SymDenotation import Annotations._ @@ -47,11 +47,6 @@ object Inliner { * from inlined code. Accessors are collected in the `accessors` buffer. */ object addAccessors extends TreeMap { - val accessors = new mutable.ListBuffer[MemberDef] - - type AccessorMap = mutable.HashMap[Symbol, DefDef] - private val getters = new AccessorMap - private val setters = new AccessorMap /** 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 @@ -62,140 +57,64 @@ object Inliner { (sym.is(AccessFlags) || sym.privateWithin.exists) && !sym.owner.isContainedIn(inlineMethod) - /** The name of the next accessor to be generated */ - def accessorName(implicit ctx: Context) = InlineAccessorName.fresh(inlineMethod.name.asTermName) - /** A fresh accessor symbol. * * @param tree The tree representing the original access to the non-public member - * @param accessorInfo The type of the accessor */ - def accessorSymbol(tree: Tree, accessorInfo: Type)(implicit ctx: Context): Symbol = - ctx.newSymbol( - owner = inlineMethod.owner, - name = if (tree.isTerm) accessorName.toTermName else accessorName.toTypeName, - flags = if (tree.isTerm) Synthetic | Method else Synthetic, - info = accessorInfo, - coord = tree.pos).entered - - /** Add an accessor to a non-public method and replace the original access with a - * call to the accessor. + def newAccessorSymbol(accessed: TermSymbol, name: TermName, info: Type)(implicit ctx: Context): TermSymbol = + ctx.newSymbol(accessed.owner, name, Synthetic | Method, info, coord = accessed.pos).entered + + /** Create an inline accessor unless one exists already, and replace the original + * access with a reference to the accessor. * - * @param tree The original access to the non-public symbol - * @param refPart The part that refers to the method or field of the original access - * @param targs All type arguments passed in the access, if any - * @param argss All value arguments passed in the access, if any - * @param accessedType The type of the accessed method or field, as seen from the access site. - * @param rhs A function that builds the right-hand side of the accessor, - * given a reference to the accessed symbol and any type and - * value arguments the need to be integrated. - * @param seen An map of already generated accessor methods of this kind (getter or setter) - * @return The call to the accessor method that replaces the original access. + * @param reference The original reference to the non-public symbol + * @param onLHS The reference is on the left-hand side of an assignment */ - def addAccessor(tree: Tree, refPart: Tree, targs: List[Tree], argss: List[List[Tree]], - accessedType: Type, rhs: (Tree, List[Type], List[List[Tree]]) => Tree, - seen: AccessorMap)(implicit ctx: Context): Tree = { - val qual = qualifier(refPart) - - def refIsLocal = qual match { - case qual: This => qual.symbol == refPart.symbol.owner - case _ => false - } + def useAccessor(reference: RefTree, onLHS: Boolean)(implicit ctx: Context): Tree = { - def accessorDef(accessorType: Type, accessorDefFn: TermSymbol => DefDef): DefDef = - seen.getOrElseUpdate(refPart.symbol, { - val acc = accessorSymbol(tree, accessorType).asTerm - val accessorDef = accessorDefFn(acc) - accessors += accessorDef - inlining.println(i"added inline accessor: $accessorDef") - accessorDef - }) - - if (refPart.symbol.isStatic || refIsLocal) { - // Easy case: Reference to a static symbol or a symbol referenced via `this.` - val accDef = accessorDef( - accessedType.ensureMethodic, - polyDefDef(_, tps => argss => rhs(refPart, tps, argss).withPos(tree.pos.focus))) + def nameKind = if (onLHS) InlineSetterName else InlineGetterName + val accessed = reference.symbol.asTerm - ref(accDef.symbol).appliedToTypeTrees(targs).appliedToArgss(argss).withPos(tree.pos) + def refersToAccessed(sym: Symbol) = sym.getAnnotation(defn.AccessedAnnot) match { + case Some(Annotation.Accessed(sym)) => sym `eq` accessed + case _ => false } - else { - // Hard case: Reference needs to go via a dynamic prefix - inlining.println(i"adding inline accessor for $tree -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))") - - // Need to dealias in order to catch all possible references to abstracted over types in - // substitutions - val dealiasMap = new TypeMap { - def apply(t: Type) = mapOver(t.dealias) - } - val qualType = dealiasMap(qual.tpe.widen) + 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) + acc.addAnnotation(Annotation.Accessed(accessed)) + acc + } - // Add qualifier type as leading method argument to argument `tp` - def addQualType(tp: Type): Type = tp match { - case tp: PolyType => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, addQualType(tp.resultType)) - case tp: ExprType => addQualType(tp.resultType) - case tp => MethodType(qualType :: Nil, tp) + { reference match { + case Select(qual, _) => qual.select(accessorSymbol) + case Ident(name) => ref(accessorSymbol) } - - // The types that are local to the inlined method, and that therefore have - // to be abstracted out in the accessor, which is external to the inlined method - val localRefs = qualType.namedPartsWith(ref => - ref.isType && ref.symbol.isContainedIn(inlineMethod)).toList - - // Abstract accessed type over local refs - def abstractQualType(mtpe: Type): Type = - if (localRefs.isEmpty) mtpe - else PolyType.fromParams(localRefs.map(_.symbol.asType), mtpe) - .asInstanceOf[PolyType].flatten - - val accDef = accessorDef( - abstractQualType(addQualType(dealiasMap(accessedType))), - polyDefDef(_, tps => argss => - rhs(argss.head.head.select(refPart.symbol), tps.drop(localRefs.length), argss.tail) - .withPos(tree.pos.focus))) - - ref(accDef.symbol) - .appliedToTypeTrees(localRefs.map(TypeTree(_)) ++ targs) - .appliedToArgss((qual :: Nil) :: argss) - .withPos(tree.pos) - } + }.withPos(reference.pos) } + // TODO: Also handle references to non-public types. + // 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 { tree match { - case _: Apply | _: TypeApply | _: RefTree if needsAccessor(tree.symbol) => - if (tree.isTerm) { - val (methPart, targs, argss) = decomposeCall(tree) - if (methPart.symbol.isConstructor && needsAccessor(methPart.symbol)) { - ctx.error("Cannot use private constructors in inline methods", tree.pos) - tree // TODO: create a proper accessor for the private constructor - } - else - addAccessor(tree, methPart, targs, argss, - accessedType = methPart.tpe.widen, - rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss), - seen = getters) - } - else { - // TODO: Handle references to non-public types. - // 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. - // Draft code (incomplete): - // - // val accessor = accessorSymbol(tree, TypeAlias(tree.tpe)).asType - // myAccessors += TypeDef(accessor).withPos(tree.pos.focus) - // ref(accessor).withPos(tree.pos) - // - tree + 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) => - addAccessor(tree, lhs, Nil, (rhs :: Nil) :: Nil, - accessedType = MethodType(rhs.tpe.widen :: Nil, defn.UnitType), - rhs = (lhs, tps, argss) => lhs.becomes(argss.head.head), - seen = setters) - case _ => tree + cpy.Apply(tree)(useAccessor(lhs, onLHS = true), List(rhs)) + case _ => + tree } } } @@ -204,12 +123,23 @@ object Inliner { // Inline methods in local scopes can only be called in the scope they are defined, // so no accessors are needed for them. tree - else { - val tree1 = addAccessors.transform(tree) - flatTree(tree1 :: addAccessors.accessors.toList) - } + else addAccessors.transform(tree) } + /** The inline accessor definitions that need to be added to class `cls` */ + def accessorDefs(cls: Symbol)(implicit ctx: Context): List[DefDef] = + for (accessor <- cls.info.decls.filter(sym => sym.name.is(InlineGetterName) || sym.name.is(InlineSetterName))) + yield polyDefDef(accessor.asTerm, tps => argss => { + val Annotation.Accessed(accessed) = accessor.getAnnotation(defn.AccessedAnnot).get + val rhs = + if (accessor.name.is(InlineSetterName) && + argss.nonEmpty && argss.head.nonEmpty) // defensive conditions + ref(accessed).becomes(argss.head.head) + else + ref(accessed).appliedToTypes(tps).appliedToArgss(argss) + rhs.withPos(accessed.pos) + }) + /** Register inline info for given inline method `sym`. * * @param sym The symbol denotatioon of the inline method for which info is registered @@ -241,27 +171,11 @@ object Inliner { def hasBodyToInline(sym: SymDenotation)(implicit ctx: Context): Boolean = sym.isInlineMethod && sym.hasAnnotation(defn.BodyAnnot) - private def bodyAndAccessors(sym: SymDenotation)(implicit ctx: Context): (Tree, List[MemberDef]) = - sym.unforcedAnnotation(defn.BodyAnnot).get.tree match { - case Thicket(body :: accessors) => (body, accessors.asInstanceOf[List[MemberDef]]) - case body => (body, Nil) - } - /** The body to inline for method `sym`. * @pre hasBodyToInline(sym) */ def bodyToInline(sym: SymDenotation)(implicit ctx: Context): Tree = - bodyAndAccessors(sym)._1 - - /** The accessors to non-public members needed by the inlinable body of `sym`. - * These accessors are dropped as a side effect of calling this method. - * @pre hasBodyToInline(sym) - */ - def removeInlineAccessors(sym: SymDenotation)(implicit ctx: Context): List[MemberDef] = { - val (body, accessors) = bodyAndAccessors(sym) - if (accessors.nonEmpty) sym.updateAnnotation(ConcreteBodyAnnotation(body)) - accessors - } + sym.unforcedAnnotation(defn.BodyAnnot).get.tree /** Try to inline a call to a `@inline` method. Fail with error if the maximal * inline depth is exceeded. diff --git a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala index 689464d6cc0c..a43d85d37222 100644 --- a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala +++ b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala @@ -122,7 +122,7 @@ class ReTyper extends Typer with ReChecking { override def inferView(from: Tree, to: Type)(implicit ctx: Context): Implicits.SearchResult = Implicits.NoMatchingImplicitsFailure override def checkCanEqual(ltp: Type, rtp: Type, pos: Position)(implicit ctx: Context): Unit = () - override def inlineExpansion(mdef: DefDef)(implicit ctx: Context): List[Tree] = mdef :: Nil - + override def inlineExpansion(mdef: DefDef)(implicit ctx: Context): Tree = mdef + override protected def addInlineAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = body override protected def checkEqualityEvidence(tree: tpd.Tree, pt: Type)(implicit ctx: Context): Unit = () } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 24ed19112fb6..ffb6b4c0fb67 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1523,7 +1523,8 @@ class Typer extends Namer cdef.withType(UnspecifiedErrorType) } else { val dummy = localDummy(cls, impl) - val body1 = typedStats(impl.body, dummy)(ctx.inClassContext(self1.symbol)) + val body1 = addInlineAccessorDefs(cls, + typedStats(impl.body, dummy)(ctx.inClassContext(self1.symbol))) if (!ctx.isAfterTyper) cls.setNoInitsFlags((NoInitsInterface /: body1) ((fs, stat) => fs & defKind(stat))) @@ -1561,6 +1562,11 @@ class Typer extends Namer } } + protected def addInlineAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = { + val accDefs = Inliner.accessorDefs(cls) + if (accDefs.isEmpty) body else body ++ accDefs + } + /** Ensure that the first type in a list of parent types Ps points to a non-trait class. * If that's not already the case, add one. The added class type CT is determined as follows. * First, let C be the unique class such that @@ -1882,7 +1888,7 @@ class Typer extends Namer case none => typed(mdef) match { case mdef1: DefDef if Inliner.hasBodyToInline(mdef1.symbol) => - buf ++= inlineExpansion(mdef1) + buf += inlineExpansion(mdef1) case mdef1 => import untpd.modsDeco mdef match { @@ -1913,12 +1919,11 @@ class Typer extends Namer } /** Given an inline method `mdef`, the method rewritten so that its body - * uses accessors to access non-public members, followed by the accessor definitions. + * uses accessors to access non-public members. * Overwritten in Retyper to return `mdef` unchanged. */ - protected def inlineExpansion(mdef: DefDef)(implicit ctx: Context): List[Tree] = - tpd.cpy.DefDef(mdef)(rhs = Inliner.bodyToInline(mdef.symbol)) :: - Inliner.removeInlineAccessors(mdef.symbol) + protected def inlineExpansion(mdef: DefDef)(implicit ctx: Context): Tree = + tpd.cpy.DefDef(mdef)(rhs = Inliner.bodyToInline(mdef.symbol)) def typedExpr(tree: untpd.Tree, pt: Type = WildcardType)(implicit ctx: Context): Tree = typed(tree, pt)(ctx retractMode Mode.PatternOrTypeBits) diff --git a/tests/pos/i4322.scala b/tests/pos/i4322.scala index 2994126d2e6e..70039b7a5e17 100644 --- a/tests/pos/i4322.scala +++ b/tests/pos/i4322.scala @@ -1,10 +1,14 @@ object Foo { private[this] var x: Int = 1 - inline def foo: Int = x + x + x + private def x(n: String): Int = n.toInt + + inline def foo: Int = x + x + x("22") inline def bar = { x += 1 x += 1 } + + inline def baz = { x += x("11") } } From 0610cf201c65432bbd2cd49662a2db112d52fb58 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 21 May 2018 19:22:52 +0200 Subject: [PATCH 4/6] Remove `Accessed` annotations once accessors are generated. --- compiler/src/dotty/tools/dotc/typer/Inliner.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 952db2b06482..439fab9a9346 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -126,11 +126,15 @@ object Inliner { else addAccessors.transform(tree) } - /** The inline accessor definitions that need to be added to class `cls` */ + /** The inline accessor definitions that need to be added to class `cls` + * As a side-effect, this method removes the `Accessed` annotations from + * the accessor symbols. So a second call of the same method will yield the empty list. + */ def accessorDefs(cls: Symbol)(implicit ctx: Context): List[DefDef] = for (accessor <- cls.info.decls.filter(sym => sym.name.is(InlineGetterName) || sym.name.is(InlineSetterName))) yield polyDefDef(accessor.asTerm, tps => argss => { val Annotation.Accessed(accessed) = accessor.getAnnotation(defn.AccessedAnnot).get + accessor.removeAnnotation(defn.AccessedAnnot) val rhs = if (accessor.name.is(InlineSetterName) && argss.nonEmpty && argss.head.nonEmpty) // defensive conditions From 2b53d677721b7180ee1b76a7c6d5f8efda016c03 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 22 May 2018 09:44:03 +0200 Subject: [PATCH 5/6] Unify handling of inline accessors and protected accessors This is a preparation step to enable migrating generation of protected accessors after pickling. The actual migration will be done at a later point. To do this, I refactored much of the logic needed for generating acessors into a new class `AccessProxies`. --- .../tools/dotc/transform/AccessProxies.scala | 108 +++++++++++++++ .../dotty/tools/dotc/transform/Erasure.scala | 31 ++++- .../src/dotty/tools/dotc/typer/Inliner.scala | 128 ++++-------------- .../src/dotty/tools/dotc/typer/ReTyper.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 9 +- 5 files changed, 171 insertions(+), 107 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/transform/AccessProxies.scala diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala new file mode 100644 index 000000000000..484157d9c4ac --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala @@ -0,0 +1,108 @@ +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 + +abstract class AccessProxies { + import ast.tpd._ + + def getterName: ClassifiedNameKind + def setterName: ClassifiedNameKind + + /** The accessor definitions that need to be added to class `cls` + * As a side-effect, this method removes the `Accessed` annotations from + * the accessor symbols. So a second call of the same method will yield the empty list. + */ + private def accessorDefs(cls: Symbol)(implicit ctx: Context): List[DefDef] = + for { + accessor <- cls.info.decls.filter(sym => sym.name.is(getterName) || sym.name.is(setterName)) + Annotation.Accessed(accessed) <- accessor.getAnnotation(defn.AccessedAnnot) + } + yield polyDefDef(accessor.asTerm, tps => argss => { + accessor.removeAnnotation(defn.AccessedAnnot) + val rhs = + if (accessor.name.is(setterName) && + argss.nonEmpty && argss.head.nonEmpty) // defensive conditions + ref(accessed).becomes(argss.head.head) + else + ref(accessed).appliedToTypes(tps).appliedToArgss(argss) + rhs.withPos(accessed.pos) + }) + + 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) = sym.getAnnotation(defn.AccessedAnnot) match { + case Some(Annotation.Accessed(sym)) => sym `eq` accessed + case _ => false + } + + 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) + acc.addAnnotation(Annotation.Accessed(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 + } + } +} \ No newline at end of file diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index d0f7daa2cadc..48080e8718a8 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/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 { diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 439fab9a9346..7b1fdd5227be 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -19,7 +19,7 @@ import NameKinds.{ClassifiedNameKind, InlineGetterName, InlineSetterName} import ProtoTypes.selectionProto import SymDenotations.SymDenotation import Annotations._ -import transform.ExplicitOuter +import transform.{ExplicitOuter, AccessProxies} import Inferencing.fullyDefinedType import config.Printers.inlining import ErrorReporting.errorTree @@ -31,119 +31,51 @@ import util.Positions.Position object Inliner { import tpd._ - /** Adds accessors for all non-public term members accessed - * from `tree`. Non-public type members are currently left as they are. - * This means that references to a private type will lead to typing failures - * on the code when it is inlined. Less than ideal, but hard to do better (see below). - * - * @return If there are accessors generated, a thicket consisting of the rewritten `tree` - * and all accessors, otherwise the original tree. - */ - private def makeInlineable(tree: Tree)(implicit ctx: Context) = { - - val inlineMethod = ctx.owner + object 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. - */ - object addAccessors extends TreeMap { + * from inlined code. Accessors are collected in the `accessors` buffer. + */ + class MakeInlineable(inlineSym: Symbol) extends TreeMap with Insert { /** 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 - * by excluding all symbols properly contained in the inlined method. - */ + * and it is not part of the tree that gets inlined. The latter test is implemented + * by excluding all symbols properly contained in the inlined method. + */ def needsAccessor(sym: Symbol)(implicit ctx: Context) = sym.isTerm && (sym.is(AccessFlags) || sym.privateWithin.exists) && - !sym.owner.isContainedIn(inlineMethod) - - /** A fresh accessor symbol. - * - * @param tree The tree representing the original access to the non-public member - */ - def newAccessorSymbol(accessed: TermSymbol, name: TermName, info: Type)(implicit ctx: Context): TermSymbol = - ctx.newSymbol(accessed.owner, name, Synthetic | Method, info, coord = accessed.pos).entered - - /** Create an inline 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) InlineSetterName else InlineGetterName - val accessed = reference.symbol.asTerm - - def refersToAccessed(sym: Symbol) = sym.getAnnotation(defn.AccessedAnnot) match { - case Some(Annotation.Accessed(sym)) => sym `eq` accessed - case _ => false - } + !sym.isContainedIn(inlineSym) - 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) - acc.addAnnotation(Annotation.Accessed(accessed)) - acc - } - - { reference match { - case Select(qual, _) => qual.select(accessorSymbol) - case Ident(name) => ref(accessorSymbol) - } - }.withPos(reference.pos) - } // TODO: Also handle references to non-public types. // 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 { - 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 - } - } + override def transform(tree: Tree)(implicit ctx: Context): Tree = + super.transform(accessorIfNeeded(tree)) } - if (inlineMethod.owner.isTerm) - // Inline methods in local scopes can only be called in the scope they are defined, - // so no accessors are needed for them. - tree - else addAccessors.transform(tree) + /** Adds accessors for all non-public term members accessed + * from `tree`. Non-public type members are currently left as they are. + * This means that references to a private type will lead to typing failures + * on the code when it is inlined. Less than ideal, but hard to do better (see below). + * + * @return If there are accessors generated, a thicket consisting of the rewritten `tree` + * and all accessors, otherwise the original tree. + */ + def makeInlineable(tree: Tree)(implicit ctx: Context) = { + val inlineSym = ctx.owner + if (inlineSym.owner.isTerm) + // Inline methods in local scopes can only be called in the scope they are defined, + // so no accessors are needed for them. + tree + else new MakeInlineable(inlineSym).transform(tree) + } } - /** The inline accessor definitions that need to be added to class `cls` - * As a side-effect, this method removes the `Accessed` annotations from - * the accessor symbols. So a second call of the same method will yield the empty list. - */ - def accessorDefs(cls: Symbol)(implicit ctx: Context): List[DefDef] = - for (accessor <- cls.info.decls.filter(sym => sym.name.is(InlineGetterName) || sym.name.is(InlineSetterName))) - yield polyDefDef(accessor.asTerm, tps => argss => { - val Annotation.Accessed(accessed) = accessor.getAnnotation(defn.AccessedAnnot).get - accessor.removeAnnotation(defn.AccessedAnnot) - val rhs = - if (accessor.name.is(InlineSetterName) && - argss.nonEmpty && argss.head.nonEmpty) // defensive conditions - ref(accessed).becomes(argss.head.head) - else - ref(accessed).appliedToTypes(tps).appliedToArgss(argss) - rhs.withPos(accessed.pos) - }) - /** Register inline info for given inline method `sym`. * * @param sym The symbol denotatioon of the inline method for which info is registered @@ -163,7 +95,7 @@ object Inliner { sym.updateAnnotation(LazyBodyAnnotation { _ => implicit val ctx = inlineCtx val body = treeExpr(ctx) - if (ctx.reporter.hasErrors) body else makeInlineable(body) + if (ctx.reporter.hasErrors) body else InlineAccessors.makeInlineable(body) }) } } diff --git a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala index a43d85d37222..3a330209a352 100644 --- a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala +++ b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala @@ -123,6 +123,6 @@ class ReTyper extends Typer with ReChecking { Implicits.NoMatchingImplicitsFailure override def checkCanEqual(ltp: Type, rtp: Type, pos: Position)(implicit ctx: Context): Unit = () override def inlineExpansion(mdef: DefDef)(implicit ctx: Context): Tree = mdef - override protected def addInlineAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = body + override protected def addAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = body override protected def checkEqualityEvidence(tree: tpd.Tree, pt: Type)(implicit ctx: Context): Unit = () } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index ffb6b4c0fb67..5198407e6225 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -25,7 +25,6 @@ import ErrorReporting._ import Checking._ import Inferencing._ import EtaExpansion.etaExpand -import dotty.tools.dotc.transform.Erasure.Boxing import util.Positions._ import util.common._ import util.{Property, SourcePosition} @@ -1523,7 +1522,7 @@ class Typer extends Namer cdef.withType(UnspecifiedErrorType) } else { val dummy = localDummy(cls, impl) - val body1 = addInlineAccessorDefs(cls, + val body1 = addAccessorDefs(cls, typedStats(impl.body, dummy)(ctx.inClassContext(self1.symbol))) if (!ctx.isAfterTyper) cls.setNoInitsFlags((NoInitsInterface /: body1) ((fs, stat) => fs & defKind(stat))) @@ -1562,10 +1561,8 @@ class Typer extends Namer } } - protected def addInlineAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = { - val accDefs = Inliner.accessorDefs(cls) - if (accDefs.isEmpty) body else body ++ accDefs - } + protected def addAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = + Inliner.InlineAccessors.addAccessorDefs(cls, body) /** Ensure that the first type in a list of parent types Ps points to a non-trait class. * If that's not already the case, add one. The added class type CT is determined as follows. From 8d8ce455d9ff57dd296d2592ac1a8729844b2473 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 22 May 2018 10:07:04 +0200 Subject: [PATCH 6/6] Use a map instead of an annotation to keep track of accessors Reverted: New annotation on inline accessors (reverted from commit 9cde3dd8556d2eb688fca9deff8bd85dec0eff0f) Also: Fix accessor definition to work for accesses to members of superclasses. --- .../dotty/tools/dotc/CompilationUnit.scala | 8 +++- .../dotty/tools/dotc/core/Annotations.scala | 23 ++-------- .../dotty/tools/dotc/core/Definitions.scala | 2 - .../core/unpickleScala2/Scala2Unpickler.scala | 2 +- .../tools/dotc/transform/AccessProxies.scala | 45 ++++++++++--------- .../src/dotty/tools/dotc/typer/Inliner.scala | 4 +- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- .../scala/annotation/internal/Accessed.scala | 8 ---- tests/run/inlineProtected.scala | 22 +++++++++ 9 files changed, 59 insertions(+), 57 deletions(-) delete mode 100644 library/src/scala/annotation/internal/Accessed.scala create mode 100644 tests/run/inlineProtected.scala diff --git a/compiler/src/dotty/tools/dotc/CompilationUnit.scala b/compiler/src/dotty/tools/dotc/CompilationUnit.scala index 52cf181a2240..f94558e438ee 100644 --- a/compiler/src/dotty/tools/dotc/CompilationUnit.scala +++ b/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 { diff --git a/compiler/src/dotty/tools/dotc/core/Annotations.scala b/compiler/src/dotty/tools/dotc/core/Annotations.scala index 712a2256b3aa..2da8c95f0b71 100644 --- a/compiler/src/dotty/tools/dotc/core/Annotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Annotations.scala @@ -146,26 +146,11 @@ object Annotations { def deferredResolve(atp: Type, args: List[Tree])(implicit ctx: Context): Annotation = deferred(atp.classSymbol, implicit ctx => resolveConstructor(atp, args)) - class TermRefAnnotExtractor(annotFn: Context => Symbol) { + def makeAlias(sym: TermSymbol)(implicit ctx: Context) = + apply(defn.AliasAnnot, List( + ref(TermRef(sym.owner.thisType, sym.name, sym)))) - def apply(sym: TermSymbol)(implicit ctx: Context): Annotation = - Annotation(annotFn(ctx).typeRef, List(ref(TermRef(sym.owner.thisType, sym.name, sym)))) - - def unapply(ann: Annotation)(implicit ctx: Context): Option[Symbol] = - if (ann.symbol == annotFn(ctx)) { - val ast.Trees.Apply(fn, arg :: Nil) = ann.tree - Some(arg.symbol) - } - else None - } - - /** Extractor for "accessed" annotations */ - object Accessed extends TermRefAnnotExtractor(implicit ctx => defn.AccessedAnnot) - - /** Extractor for "aliased" annotations */ - object Alias extends TermRefAnnotExtractor(implicit ctx => defn.AliasAnnot) - - /** Extractor for "child" annotations */ + /** Extractor for child annotations */ object Child { /** A deferred annotation to the result of a given child computation */ diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 86d78b41c48b..ef5d0d1f7022 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -692,8 +692,6 @@ class Definitions { // Annotation classes lazy val AliasAnnotType = ctx.requiredClassRef("scala.annotation.internal.Alias") def AliasAnnot(implicit ctx: Context) = AliasAnnotType.symbol.asClass - lazy val AccessedAnnotType = ctx.requiredClassRef("scala.annotation.internal.Accessed") - def AccessedAnnot(implicit ctx: Context) = AccessedAnnotType.symbol.asClass lazy val AnnotationDefaultAnnotType = ctx.requiredClassRef("scala.annotation.internal.AnnotationDefault") def AnnotationDefaultAnnot(implicit ctx: Context) = AnnotationDefaultAnnotType.symbol.asClass lazy val BodyAnnotType = ctx.requiredClassRef("scala.annotation.internal.Body") diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index a8a26c24f2b4..f7dbda218e0e 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -578,7 +578,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas } } val alias = readDisambiguatedSymbolRef(disambiguate).asTerm - denot.addAnnotation(Annotation.Alias(alias)) + denot.addAnnotation(Annotation.makeAlias(alias)) } } // println(s"unpickled ${denot.debugString}, info = ${denot.info}") !!! DEBUG diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala index 484157d9c4ac..46352f0143d2 100644 --- a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala +++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala @@ -15,32 +15,36 @@ 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 the `Accessed` annotations from - * the accessor symbols. So a second call of the same method will yield the empty list. + * 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): List[DefDef] = - for { - accessor <- cls.info.decls.filter(sym => sym.name.is(getterName) || sym.name.is(setterName)) - Annotation.Accessed(accessed) <- accessor.getAnnotation(defn.AccessedAnnot) - } - yield polyDefDef(accessor.asTerm, tps => argss => { - accessor.removeAnnotation(defn.AccessedAnnot) - val rhs = - if (accessor.name.is(setterName) && - argss.nonEmpty && argss.head.nonEmpty) // defensive conditions - ref(accessed).becomes(argss.head.head) - else - ref(accessed).appliedToTypes(tps).appliedToArgss(argss) - rhs.withPos(accessed.pos) - }) + 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 @@ -67,10 +71,7 @@ abstract class AccessProxies { def nameKind = if (onLHS) setterName else getterName val accessed = reference.symbol.asTerm - def refersToAccessed(sym: Symbol) = sym.getAnnotation(defn.AccessedAnnot) match { - case Some(Annotation.Accessed(sym)) => sym `eq` accessed - case _ => false - } + def refersToAccessed(sym: Symbol) = accessedBy.get(sym) == Some(accessed) val accessorInfo = if (onLHS) MethodType(accessed.info :: Nil, defn.UnitType) @@ -80,7 +81,7 @@ abstract class AccessProxies { accessed.owner.info.decl(accessorName).suchThat(refersToAccessed).symbol .orElse { val acc = newAccessorSymbol(accessed, accessorName, accessorInfo) - acc.addAnnotation(Annotation.Accessed(accessed)) + accessedBy(acc) = accessed acc } diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 7b1fdd5227be..eac354893f06 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -31,7 +31,7 @@ import util.Positions.Position object Inliner { import tpd._ - object InlineAccessors extends AccessProxies { + class InlineAccessors extends AccessProxies { def getterName = InlineGetterName def setterName = InlineSetterName @@ -95,7 +95,7 @@ object Inliner { sym.updateAnnotation(LazyBodyAnnotation { _ => implicit val ctx = inlineCtx val body = treeExpr(ctx) - if (ctx.reporter.hasErrors) body else InlineAccessors.makeInlineable(body) + if (ctx.reporter.hasErrors) body else ctx.compilationUnit.inlineAccessors.makeInlineable(body) }) } } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 5198407e6225..ef714f516ccb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1562,7 +1562,7 @@ class Typer extends Namer } protected def addAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] = - Inliner.InlineAccessors.addAccessorDefs(cls, body) + ctx.compilationUnit.inlineAccessors.addAccessorDefs(cls, body) /** Ensure that the first type in a list of parent types Ps points to a non-trait class. * If that's not already the case, add one. The added class type CT is determined as follows. diff --git a/library/src/scala/annotation/internal/Accessed.scala b/library/src/scala/annotation/internal/Accessed.scala deleted file mode 100644 index cf365ea801e9..000000000000 --- a/library/src/scala/annotation/internal/Accessed.scala +++ /dev/null @@ -1,8 +0,0 @@ -package scala.annotation.internal - -import scala.annotation.Annotation - -/** An annotation to record the method accessed by an inline accessor. - * @param aliased A TermRef pointing to the accessed method - */ -class Accessed(aliased: Any) extends Annotation diff --git a/tests/run/inlineProtected.scala b/tests/run/inlineProtected.scala new file mode 100644 index 000000000000..1a725ec27346 --- /dev/null +++ b/tests/run/inlineProtected.scala @@ -0,0 +1,22 @@ +package P { + class C { + protected def f(): Int = 22 + } +} + +package Q { + class D extends P.C { + class Inner { + inline def g() = f() + } + } +} + +object Test extends App { + import Q._ + + val d = new D + val i = new d.Inner + val x = i.g() + assert(x == 22) +}