Merge pull request #517 from dotty-staging/add/expand-privates

Expand private members if necessary

Author: odersky

src/dotty/tools/dotc/Compiler.scala

@@ -67,7 +67,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)
     )
 

src/dotty/tools/dotc/core/Flags.scala

@@ -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>")
 

src/dotty/tools/dotc/core/SymDenotations.scala

@@ -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
@@ -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

src/dotty/tools/dotc/transform/Constructors.scala

@@ -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 = {

src/dotty/tools/dotc/transform/ExpandPrivate.scala

@@ -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
+  }
+}

src/dotty/tools/dotc/transform/LambdaLift.scala

@@ -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)

src/dotty/tools/dotc/transform/Mixin.scala

@@ -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
 

src/dotty/tools/dotc/transform/PrivateToStatic.scala

@@ -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)) {

src/dotty/tools/dotc/transform/SuperAccessors.scala

@@ -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))

src/dotty/tools/dotc/transform/TreeChecker.scala

@@ -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

src/dotty/tools/dotc/typer/RefChecks.scala

@@ -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
@@ -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
 
@@ -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
     }
 

tests/pos/privates.scala

@@ -6,4 +6,8 @@ trait Test {
 
   private def bar() = foo()
 
+  class Inner {
+    foo()
+  }
+
 }