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
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
45 changes: 45 additions & 0 deletions src/dotty/tools/dotc/transform/ExpandPrivate.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package dotty.tools.dotc
package transform

import core._
import dotty.tools.dotc.core.DenotTransformers.{SymTransformer, 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._

/** Make private term members that are accessed from another class
* non-private by resetting the Private flag and expanding their name.
*/
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
4 changes: 2 additions & 2 deletions src/dotty/tools/dotc/transform/SuperAccessors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ class SuperAccessors(thisTransformer: DenotTransformer) {

val superAcc = clazz.info.decl(supername).suchThat(_.signature == sym.signature).symbol orElse {
ctx.debuglog(s"add super acc ${sym.showLocated} to $clazz")
val maybeDeferred = if (clazz is Trait) Deferred else EmptyFlags
val deferredOrPrivate = if (clazz is Trait) Deferred else Private
val acc = ctx.newSymbol(
clazz, supername, SuperAccessor | Private | Artifact | Method | maybeDeferred,
clazz, supername, SuperAccessor | Artifact | Method | deferredOrPrivate,
sel.tpe.widenSingleton.ensureMethodic, coord = sym.coord).enteredAfter(thisTransformer)
// Diagnostic for SI-7091
if (!accDefs.contains(clazz))
Expand Down
3 changes: 3 additions & 0 deletions src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ class TreeChecker extends Phase with SymTransformer {
testDuplicate(sym, seenClasses, "class")
}

if (sym.is(Method) && sym.is(Deferred) && sym.is(Private))
assert(false, s"$sym is both Deferred and Private")

checkCompanion(symd)

symd
Expand Down
33 changes: 2 additions & 31 deletions src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ object RefChecks {

def ignoreDeferred(member: SingleDenotation) =
member.isType ||
member.symbol.is(SuperAccessor) || // not yet synthesized
member.symbol.is(JavaDefined) && hasJavaErasedOverriding(member.symbol)

// 2. Check that only abstract classes have deferred members
Expand Down Expand Up @@ -726,42 +727,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 +785,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()
}

}