Skip to content

Expand private members if necessary #517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 4, 2015
Merged
7 changes: 6 additions & 1 deletion src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ class Compiler {
List(new LambdaLift, // in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here
new Flatten,
new RestoreScopes),
List(/*new PrivateToStatic,*/ new CollectEntryPoints, new LabelDefs, new ElimWildcardIdents, new TraitConstructors),
List(/*new PrivateToStatic,*/
new ExpandPrivate,
new CollectEntryPoints,
new LabelDefs,
new ElimWildcardIdents,
new TraitConstructors),
List(new GenBCode)
)

Expand Down
3 changes: 0 additions & 3 deletions src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,6 @@ object Flags {
/** Symbol is defined in a super call */
final val InSuperCall = commonFlag(46, "<in supercall>")

/** Symbol with private access is accessed outside its private scope */
final val NotJavaPrivate = commonFlag(47, "<not-java-private>")

/** Denotation is in train of being loaded and completed, used to catch cyclic dependencies */
final val Touched = commonFlag(48, "<touched>")

Expand Down
14 changes: 13 additions & 1 deletion src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ object SymDenotations {
*/
final def accessBoundary(base: Symbol)(implicit ctx: Context): Symbol = {
val fs = flags
if (fs is (Private, butNot = NotJavaPrivate)) owner
if (fs is Private) owner
else if (fs is StaticProtected) defn.RootClass
else if (privateWithin.exists && !ctx.phase.erasedTypes) privateWithin
else if (fs is Protected) base
Expand Down Expand Up @@ -1049,6 +1049,18 @@ object SymDenotations {
*/
override def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(implicit ctx: Context): Unit =
super.transformAfter(phase, f)

/** If denotation is private, remove the Private flag and expand the name if necessary */
def ensureNotPrivate(implicit ctx: Context) =
if (is(Private))
copySymDenotation(
name =
if (is(ExpandedName) || isConstructor) this.name
else this.name.expandedName(initial.asSymDenotation.owner),
// need to use initial owner to disambiguate, as multiple private symbols with the same name
// might have been moved from different origins into the same class
initFlags = this.flags &~ Private | ExpandedName)
else this
}

/** The contents of a class definition during a period
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/transform/Constructors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor
private def mightBeDropped(sym: Symbol)(implicit ctx: Context) =
sym.is(Private, butNot = KeeperFlags) && !sym.is(MutableParamAccessor)

private final val KeeperFlags = Method | Lazy | NotJavaPrivate
private final val KeeperFlags = Method | Lazy
private final val MutableParamAccessor = allOf(Mutable, ParamAccessor)

override def transformTemplate(tree: Template)(implicit ctx: Context, info: TransformerInfo): Tree = {
Expand Down
49 changes: 49 additions & 0 deletions src/dotty/tools/dotc/transform/ExpandPrivate.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package dotty.tools.dotc
package transform

import core._
import DenotTransformers.IdentityDenotTransformer
import Contexts.Context
import Symbols._
import Scopes._
import Flags._
import StdNames._
import SymDenotations._
import Types._
import collection.mutable
import TreeTransforms._
import Decorators._
import ast.Trees._
import TreeTransforms._

/** Makes private methods static, provided they not deferred, accessors, or static,
* by rewriting a method `m` in class `C` as follows:
*
* private def m(ps) = e
*
* --> private static def($this: C, ps) = [this -> $this] e
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description was copy-pasted from PrivateToStatic and does not match what this phase does.

*/
class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { thisTransform =>
import ast.tpd._

override def phaseName: String = "expandPrivate"

/** Make private terms accessed from different classes non-private.
* Note: this happens also for accesses between class and linked module class.
* If we change the scheme at one point to make static module class computations
* static members of the companion class, we should tighten the condition below.
*/
private def ensurePrivateAccessible(d: SymDenotation)(implicit ctx: Context) =
if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass)
d.ensureNotPrivate.installAfter(thisTransform)

override def transformIdent(tree: Ident)(implicit ctx: Context, info: TransformerInfo) = {
ensurePrivateAccessible(tree.symbol)
tree
}

override def transformSelect(tree: Select)(implicit ctx: Context, info: TransformerInfo) = {
ensurePrivateAccessible(tree.symbol)
tree
}
}
3 changes: 1 addition & 2 deletions src/dotty/tools/dotc/transform/LambdaLift.scala
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,10 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
val (newOwner, maybeStatic) =
if (lOwner is Package) (local.topLevelClass, JavaStatic)
else (lOwner, EmptyFlags)
val maybeNotJavaPrivate = if (calledFromInner(local)) NotJavaPrivate else EmptyFlags
local.copySymDenotation(
owner = newOwner,
name = newName(local),
initFlags = local.flags &~ InSuperCall | Private | maybeStatic | maybeNotJavaPrivate,
initFlags = local.flags &~ InSuperCall | Private | maybeStatic,
info = liftedInfo(local)).installAfter(thisTransform)
if (local.isClass)
for (member <- local.asClass.info.decls)
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/transform/Mixin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class Mixin extends MiniPhaseTransform with SymTransformer { thisTransform =>

override def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation =
if (sym.is(Accessor, butNot = Deferred) && sym.owner.is(Trait))
sym.copySymDenotation(initFlags = sym.flags | Deferred)
sym.copySymDenotation(initFlags = sym.flags | Deferred).ensureNotPrivate
else
sym

Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/transform/PrivateToStatic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class PrivateToStatic extends MiniPhase with SymTransformer { thisTransform =>
def shouldBeStatic(sd: SymDenotation)(implicit ctx: Context) =
sd.current(ctx.withPhase(thisTransform)).asSymDenotation
.is(PrivateMethod, butNot = Immovable) &&
(sd.owner.is(Trait) || sd.is(NotJavaPrivate))
sd.owner.is(Trait)

override def transformSym(sd: SymDenotation)(implicit ctx: Context): SymDenotation =
if (shouldBeStatic(sd)) {
Expand Down
32 changes: 1 addition & 31 deletions src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -726,42 +726,14 @@ import RefChecks._
* todo: But RefChecks is not done yet. It's still a somewhat dirty port from the Scala 2 version.
* todo: move untrivial logic to their own mini-phases
*/
class RefChecks extends MiniPhase with SymTransformer { thisTransformer =>
class RefChecks extends MiniPhase { thisTransformer =>

import tpd._

override def phaseName: String = "refchecks"

val treeTransform = new Transform(NoLevelInfo)

/** Ensure the following members are not private:
* - term members of traits
* - the primary constructor of a value class
* - the parameter accessor of a value class
*/
override def transformSym(d: SymDenotation)(implicit ctx: Context) = {
def mustBePublicInValueClass = d.isPrimaryConstructor || d.is(ParamAccessor)
def mustBePublicInTrait = !d.is(Method) || d.isSetter || d.is(ParamAccessor)
def mustBePublic = {
val cls = d.owner
(isDerivedValueClass(cls) && mustBePublicInValueClass ||
cls.is(Trait) && mustBePublicInTrait)
}
if ((d is PrivateTerm) && mustBePublic) notPrivate(d) else d
}

/** Make private terms accessed from different classes non-private.
* Note: this happens also for accesses between class and linked module class.
* If we change the scheme at one point to make static module class computations
* static members of the companion class, we should tighten the condition below.
*/
private def ensurePrivateAccessible(d: SymDenotation)(implicit ctx: Context) =
if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass)
notPrivate(d).installAfter(thisTransformer)

private def notPrivate(d: SymDenotation)(implicit ctx: Context) =
d.copySymDenotation(initFlags = d.flags | NotJavaPrivate)

class Transform(currentLevel: RefChecks.OptLevelInfo = RefChecks.NoLevelInfo) extends TreeTransform {
def phase = thisTransformer

Expand Down Expand Up @@ -812,14 +784,12 @@ class RefChecks extends MiniPhase with SymTransformer { thisTransformer =>

override def transformIdent(tree: Ident)(implicit ctx: Context, info: TransformerInfo) = {
checkUndesiredProperties(tree.symbol, tree.pos)
ensurePrivateAccessible(tree.symbol)
currentLevel.enterReference(tree.symbol, tree.pos)
tree
}

override def transformSelect(tree: Select)(implicit ctx: Context, info: TransformerInfo) = {
checkUndesiredProperties(tree.symbol, tree.pos)
ensurePrivateAccessible(tree.symbol)
tree
}

Expand Down
4 changes: 4 additions & 0 deletions tests/pos/privates.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ trait Test {

private def bar() = foo()

class Inner {
foo()
}

}