Make fewer trait methods not-{private, protected}

No longer making trait methods not-protected.
(The backend only does public/private because of the poor
mapping between visibility from Scala to the JVM).

Note that protected trait members will not receive static forwarders
in module classes (when mixed into objects).

Historic note: we used to `makeNotPrivate` during explicitouter,
now we do it later, which means more private methods must be excluded
(e.g., lambdaLIFTED ones).

2 files changed, 27 insertions, 17 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala
index 6e064e2a24..26e517743a 100644
--- a/src/compiler/scala/tools/nsc/transform/Fields.scala
+++ b/src/compiler/scala/tools/nsc/transform/Fields.scala
@@ -249,16 +249,14 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
             val accessorUnderConsideration = !(member hasFlag (DEFERRED | LAZY))
 
             // destructively mangle accessor's name (which may cause rehashing of decls), also sets flags
-            // TODO: technically, only necessary for stored fields
-            if (member hasFlag PRIVATE) member makeNotPrivate clazz
-
-            // Need to mark as notPROTECTED, so that it's carried over to the synthesized member in subclasses,
-            // since the trait member will receive this flag later in ExplicitOuter, but the synthetic subclass member will not.
-            // If we don't add notPROTECTED to the synthesized one, the member will not be seen as overriding the trait member.
-            // Therefore, addForwarders's call to membersBasedOnFlags would see the deferred member in the trait,
-            // instead of the concrete (desired) one in the class
-            // TODO: encapsulate as makeNotProtected, similar to makeNotPrivate (also do moduleClass, e.g.)
-            if (member hasFlag PROTECTED) member setFlag notPROTECTED
+            // this accessor has to be implemented in a subclass -- can't be private
+            if ((member hasFlag PRIVATE) && fieldMemoization.stored) member makeNotPrivate clazz
+
+            // This must remain in synch with publicizeTraitMethod in Mixins, so that the
+            // synthesized member in a subclass and the trait member remain in synch regarding access.
+            // Otherwise, the member will not be seen as overriding the trait member, and `addForwarders`'s call to
+            // `membersBasedOnFlags` would see the deferred member in the trait, instead of the concrete (desired) one in the class
+            // not doing: if (member hasFlag PROTECTED) member setFlag notPROTECTED
 
             // must not reset LOCAL, as we must maintain protected[this]ness to allow that variance hole
             // (not sure why this only problem only arose when we started setting the notPROTECTED flag)
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index e6ffe328da..dae0deeccd 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -21,19 +21,31 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
 
   /** Some trait methods need to be implemented in subclasses, so they cannot be private.
     *
+    * We used to publicize during explicitouter (for some reason), so the condition is a bit more involved now it's done here
+    * (need to exclude lambdaLIFTED methods, as they do no exist during explicitouter and thus did not need to be excluded...)
+    *
     * They may be protected, now that traits are compiled 1:1 to interfaces.
+    * (The same disclaimers about mapping Scala's notion of visibility to Java's apply.)
+    *
     *
-    * TODO: interfaces can also have private members, so there's also less need to make trait members non-private
-    * can we leave more methods private?
-    * (they still may need to be implemented in subclasses, though we could make those protected...).
+    * TODO: can we just set the right flags from the start??
+    *  could we use the final flag to indicate a private method is really-really-private?
     */
   def publicizeTraitMethod(sym: Symbol): Unit = {
-    if ((sym hasFlag PRIVATE) &&
-          (  (sym hasFlag SUPERACCESSOR)  // super accessors by definition must be implemented in a subclass, so can't have the private (TODO: why are they ever private in a trait to begin with!?!?)
-          || (sym hasFlag ACCESSOR | MODULE))) // an accessor / module *may* need to be implemented in a subclass, and thus cannot be private
+    if ((sym hasFlag PRIVATE) && !(sym hasFlag LIFTED) && ( // lambdalifted methods can remain private
+        // super accessors by definition must be implemented in a subclass, so can't be private
+        // TODO: why are they ever private in a trait to begin with!?!? (could just name mangle them to begin with)
+        // TODO: can we add the SYNTHESIZE_IMPL_IN_SUBCLASS flag to super accessors symbols?
+        (sym hasFlag SUPERACCESSOR)
+        // an accessor / module *may* need to be implemented in a subclass, and thus cannot be private
+        // TODO: document how we get here (lambdalift? fields has already made accessors not-private)
+        || (sym hasFlag ACCESSOR | MODULE) && (sym hasFlag SYNTHESIZE_IMPL_IN_SUBCLASS)))
       sym.makeNotPrivate(sym.owner)
 
-    if (sym hasFlag PROTECTED) sym setFlag notPROTECTED
+    // no need to make trait methods not-protected
+    // (we used to have to move them to another class when interfaces could not have concrete methods)
+    // see note in `synthFieldsAndAccessors` in Fields.scala
+    // if (sym hasFlag PROTECTED) sym setFlag notPROTECTED
   }
 
   /** This map contains a binding (class -> info) if