Make-not private

Refchecks now makes all members not-private that need it. This is done by
setting flag NotJavaPrivate. No name change is involved.

4 files changed, 55 insertions, 29 deletions

diff --git a/src/dotty/tools/dotc/core/Flags.scala b/src/dotty/tools/dotc/core/Flags.scala
index 20427516d..532f6d00f 100644
--- a/src/dotty/tools/dotc/core/Flags.scala
+++ b/src/dotty/tools/dotc/core/Flags.scala
@@ -190,6 +190,8 @@ object Flags {
 
   /** Labeled with `private` modifier */
   final val Private = commonFlag(2, "private")
+  final val PrivateTerm = Private.toTermFlags
+  final val PrivateType = Private.toTypeFlags
 
   /** Labeled with `protected` modifier */
   final val Protected = commonFlag(3, "protected")
@@ -356,6 +358,9 @@ object Flags {
 
   // Flags following this one are not pickled
 
+  /** 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>")
 
diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala
index bd269bbcc..310dde912 100644
--- a/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -805,7 +805,7 @@ object SymDenotations {
      */
     final def accessBoundary(base: Symbol)(implicit ctx: Context): Symbol = {
       val fs = flags
-      if (fs is Private) owner
+      if (fs is (Private, butNot = NotJavaPrivate)) owner
       else if (fs is StaticProtected) defn.RootClass
       else if (privateWithin.exists && !ctx.phase.erasedTypes) privateWithin
       else if (fs is Protected) base
@@ -890,22 +890,6 @@ object SymDenotations {
     /** Install this denotation as the result of the given denotation transformer. */
     override def installAfter(phase: DenotTransformer)(implicit ctx: Context): Unit =
       super.installAfter(phase)
-
-    /** Remove private modifier from symbol's definition. If this symbol
-     *  is not a constructor nor a static module, rename it by expanding its name to avoid name clashes
-     *  @param base  the fully qualified name of this class will be appended if name expansion is needed
-     */
-    final def makeNotPrivateAfter(base: Symbol, phase: DenotTransformer)(implicit ctx: Context): Unit = {
-      if (this.is(Private)) {
-        val newName =
-          if (this.is(Module) && isStatic || isClassConstructor) name
-          else {
-            if (this.is(Module)) moduleClass.makeNotPrivateAfter(base, phase)
-            name.expandedName(base)
-          }
-        copySymDenotation(name = newName, initFlags = flags &~ Private).installAfter(phase)
-      }
-    }
   }
 
   /** The contents of a class definition during a period
diff --git a/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/src/dotty/tools/dotc/transform/ExtensionMethods.scala
index a218731a6..0f7667309 100644
--- a/src/dotty/tools/dotc/transform/ExtensionMethods.scala
+++ b/src/dotty/tools/dotc/transform/ExtensionMethods.scala
@@ -156,10 +156,6 @@ class ExtensionMethods extends MacroTransform with DenotTransformer with FullPar
              wrap over other value classes anyway.
             checkNonCyclic(ctx.owner.pos, Set(), ctx.owner) */
             extensionDefs(ctx.owner.linkedClass) = new mutable.ListBuffer[Tree]
-            ctx.owner.primaryConstructor.makeNotPrivateAfter(NoSymbol, thisTransformer)
-            // SI-7859 make param accessors accessible so the erasure can generate unbox operations.
-            val paramAccessors = ctx.owner.info.decls.filter(_.is(TermParamAccessor))
-            paramAccessors.foreach(_.makeNotPrivateAfter(ctx.owner, thisTransformer))
             super.transform(tree)
           } else if (ctx.owner.isStaticOwner) {
             val tree1 @ Template(_, _, _, body) = super.transform(tree)
diff --git a/src/dotty/tools/dotc/typer/RefChecks.scala b/src/dotty/tools/dotc/typer/RefChecks.scala
index 715960ee5..e338aa392 100644
--- a/src/dotty/tools/dotc/typer/RefChecks.scala
+++ b/src/dotty/tools/dotc/typer/RefChecks.scala
@@ -662,9 +662,9 @@ object RefChecks {
 }
 import RefChecks._
 
-/** Post-attribution checking and transformation.
+/** Post-attribution checking and transformation, which fulfills the following roles
  *
- *  This phase performs the following checks.
+ *  1. This phase performs the following checks.
  *
  *  - only one overloaded alternative defines default arguments
  *  - applyDynamic methods are not overloaded
@@ -673,15 +673,26 @@ import RefChecks._
  *  - this(...) constructor calls do not forward reference other definitions in their block (not even lazy vals).
  *  - no forward reference in a local block jumps over a non-lazy val definition.
  *
- *  It warns about references to symbols labeled deprecated or migration.
- *
- *  It performs the following transformations:
+ *  2. It warns about references to symbols labeled deprecated or migration.
+
+ *  3. It performs the following transformations:
  *
  *  - if (true) A else B  --> A
  *    if (false) A else B --> B
  *  - macro definitions are eliminated.
+ *
+ *  4. It makes members not private where necessary. The following members
+ *  cannot be private in the Java model:
+ *   - term members of traits
+ *   - the primary constructor of a value class
+ *   - the parameter accessor of a value class
+ *   - members accessed from an inner or companion class.
+ *  All these members are marked as NotJavaPrivate.
+ *  Unlike in Scala 2.x not-private members keep their name. It is
+ *  up to the backend to find a unique expanded name for them. The
+ *  rationale to do name changes that late is that they are very fragile.
  */
-class RefChecks extends MiniPhase with IdentityDenotTransformer { thisTransformer =>
+class RefChecks extends MiniPhase with SymTransformer { thisTransformer =>
 
   import tpd._
 
@@ -689,8 +700,38 @@ class RefChecks extends MiniPhase with IdentityDenotTransformer { thisTransforme
 
   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
+    override def treeTransformPhase = thisTransformer.next
+
     override def prepareForStats(trees: List[Tree])(implicit ctx: Context) = {
       // println(i"preparing for $trees%; %, owner = ${ctx.owner}")
       if (ctx.owner.isTerm) new Transform(new LevelInfo(currentLevel.levelAndIndex, trees))
@@ -722,8 +763,6 @@ class RefChecks extends MiniPhase with IdentityDenotTransformer { thisTransforme
       checkOverloadedRestrictions(cls)
       checkAllOverrides(cls)
       checkAnyValSubclass(cls)
-      if (isDerivedValueClass(cls))
-        cls.primaryConstructor.makeNotPrivateAfter(NoSymbol, thisTransformer) // SI-6601, must be done *after* pickler!
       tree
     }
 
@@ -738,12 +777,14 @@ class RefChecks extends MiniPhase with IdentityDenotTransformer { thisTransforme
 
     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
     }