From 17a04293d930ea169474f4f195ac823f6cf9fab4 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 4 Jul 2018 17:03:56 +0200 Subject: [PATCH 1/4] Fix #4754: Don't generate inline accessor for constants --- .../src/dotty/tools/dotc/transform/PostTyper.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Inliner.scala | 11 +++++++---- tests/pos/i4754.scala | 13 +++++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 tests/pos/i4754.scala diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 918d85afeca6..85e9af414d60 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -17,7 +17,7 @@ object PostTyper { /** A macro transform that runs immediately after typer and that performs the following functions: * - * (1) Add super accessors and protected accessors (@see SuperAccessors) + * (1) Add super accessors (@see SuperAccessors) * * (2) Convert parameter fields that have the same name as a corresponding * public parameter field in a superclass to a forwarder to the superclass diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index cd55764631a6..53ff2d950bbd 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -48,13 +48,16 @@ object Inliner { 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 - * 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. + * + * Constant vals don't need accessors since they are inlined in FirstTransform. + */ def needsAccessor(sym: Symbol)(implicit ctx: Context) = sym.isTerm && (sym.is(AccessFlags) || sym.privateWithin.exists) && - !sym.isContainedIn(inlineSym) + !sym.isContainedIn(inlineSym) && + !(sym.isStable && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) def preTransform(tree: Tree)(implicit ctx: Context): Tree diff --git a/tests/pos/i4754.scala b/tests/pos/i4754.scala new file mode 100644 index 000000000000..49cfeab4de20 --- /dev/null +++ b/tests/pos/i4754.scala @@ -0,0 +1,13 @@ +object Foo { + private final val x = 1 + private def y = 2 +} + +class Foo { + import Foo._ + inline def foo = x + Foo.x + y + Foo.y +} + +class Test { + (new Foo).foo +} From ff6a9b62950bc821cee08315bc1d0732a78205c2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 6 Jul 2018 09:49:00 +0200 Subject: [PATCH 2/4] Fix inline accessors for private members of companion objects --- .../src/dotty/tools/dotc/transform/AccessProxies.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala index 23ac69d57c71..fa3c9efbfa9a 100644 --- a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala +++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala @@ -96,8 +96,8 @@ abstract class AccessProxies { /** 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) - case Ident(name) => ref(accessor) + case Select(qual, _) if qual.tpe.derivesFrom(accessor.owner) => qual.select(accessor) + case _ => ref(accessor) } }.withPos(reference.pos) @@ -161,8 +161,8 @@ object AccessProxies { def hostForAccessorOf(accessed: Symbol)(implicit ctx: Context): Symbol = { def recur(cls: Symbol): Symbol = if (!cls.exists) NoSymbol - else if (cls.derivesFrom(accessed.owner)) cls - else if (cls.companionModule.moduleClass == accessed.owner) accessed.owner + else if (cls.derivesFrom(accessed.owner) || + cls.companionModule.moduleClass == accessed.owner) cls else recur(cls.owner) recur(ctx.owner) } From db4872cba4519eac088ea7b5901a4ba3292a9ec2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 6 Jul 2018 10:15:37 +0200 Subject: [PATCH 3/4] Allow accesses to private constant values Need to tweak ensureAccessible to use the constant type instead of the reference. --- compiler/src/dotty/tools/dotc/typer/Inliner.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 53ff2d950bbd..9a0b84811f5c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -127,7 +127,7 @@ object Inliner { if needsAccessor(tree.symbol) && tree.isTerm && !tree.symbol.isConstructor => val (refPart, targs, argss) = decomposeCall(tree) val qual = qualifier(refPart) - println(i"adding receiver passing inline accessor for $tree/$refPart -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))") + inlining.println(i"adding receiver passing inline accessor for $tree/$refPart -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))") // Need to dealias in order to cagtch all possible references to abstracted over types in // substitutions @@ -545,9 +545,10 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { override def ensureAccessible(tpe: Type, superAccess: Boolean, pos: Position)(implicit ctx: Context): Type = { tpe match { - case tpe @ TypeRef(pre, _) if !tpe.symbol.isAccessibleFrom(pre, superAccess) => + case tpe: NamedType if !tpe.symbol.isAccessibleFrom(tpe.prefix, superAccess) => tpe.info match { case TypeAlias(alias) => return ensureAccessible(alias, superAccess, pos) + case info: ConstantType if tpe.symbol.isStable => return info case _ => } case _ => From 95d3b43021af6e977325763870a25f2f9d3bd5dd Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Fri, 6 Jul 2018 10:53:38 +0200 Subject: [PATCH 4/4] Make the test a run test --- tests/pos/i4754.scala | 13 ------------- tests/run/i4754.check | 3 +++ tests/run/i4754.scala | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 13 deletions(-) delete mode 100644 tests/pos/i4754.scala create mode 100644 tests/run/i4754.check create mode 100644 tests/run/i4754.scala diff --git a/tests/pos/i4754.scala b/tests/pos/i4754.scala deleted file mode 100644 index 49cfeab4de20..000000000000 --- a/tests/pos/i4754.scala +++ /dev/null @@ -1,13 +0,0 @@ -object Foo { - private final val x = 1 - private def y = 2 -} - -class Foo { - import Foo._ - inline def foo = x + Foo.x + y + Foo.y -} - -class Test { - (new Foo).foo -} diff --git a/tests/run/i4754.check b/tests/run/i4754.check new file mode 100644 index 000000000000..7c127dc4c4c9 --- /dev/null +++ b/tests/run/i4754.check @@ -0,0 +1,3 @@ +Side effect +Side effect +12 diff --git a/tests/run/i4754.scala b/tests/run/i4754.scala new file mode 100644 index 000000000000..0907880e6499 --- /dev/null +++ b/tests/run/i4754.scala @@ -0,0 +1,16 @@ +object Foo { + private final val x = 1 + private def y = 2 + private def z: 3 = { println("Side effect"); 3 } +} + +class Foo { + import Foo._ + inline def foo = x + Foo.x + y + Foo.y + z + Foo.z +} + +object Test { + def main(args: Array[String]): Unit = { + println((new Foo).foo) + } +}