Merge pull request #4814 from dotty-staging/fix-4811

Fix #4811: Widen private[this]/protected[this] just before Flatten

Author: allanrenucci

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -63,7 +63,6 @@ class Compiler {
          new ElimPackagePrefixes) :: // Eliminate references to package prefixes in Select nodes
     List(new CheckStatic,            // Check restrictions that apply to @static members
          new ElimRepeated,           // Rewrite vararg parameters and arguments
-         new NormalizeFlags,         // Rewrite some definition flags
          new ExpandSAMs,             // Expand single abstract method closures to anonymous classes
          new ProtectedAccessors,     // Add accessors for protected members
          new ExtensionMethods,       // Expand methods of value classes with extension methods

compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala

@@ -35,6 +35,9 @@ import SymUtils._
  *
  * Finally, if the constructor of a value class is private pr protected
  * it is widened to public.
+ *
+ * Also, drop the Local flag from all private[this] and protected[this] members
+ * that will be moved to the companion object.
  */
 class ExtensionMethods extends MiniPhase with DenotTransformer with FullParameterization { thisPhase =>
 
@@ -46,7 +49,7 @@ class ExtensionMethods extends MiniPhase with DenotTransformer with FullParamete
 
   override def runsAfter = Set(
     ElimRepeated.name,
-    ProtectedAccessors.name  // protected accessors cannot handle code that is moved from class to companion object
+    ProtectedAccessors.name,  // protected accessors cannot handle code that is moved from class to companion object
   )
 
   override def runsAfterGroupsOf = Set(FirstTransform.name) // need companion objects to exist
@@ -97,17 +100,22 @@ class ExtensionMethods extends MiniPhase with DenotTransformer with FullParamete
           moduleClassSym
       }
     case ref: SymDenotation =>
+      var ref1 = ref
       if (isMethodWithExtension(ref.symbol) && ref.hasAnnotation(defn.TailrecAnnot)) {
-        val ref1 = ref.copySymDenotation()
+        ref1 = ref.copySymDenotation()
         ref1.removeAnnotation(defn.TailrecAnnot)
-        ref1
       }
       else if (ref.isConstructor && isDerivedValueClass(ref.owner) && ref.is(AccessFlags)) {
-        val ref1 = ref.copySymDenotation()
+        ref1 = ref.copySymDenotation()
         ref1.resetFlag(AccessFlags)
-        ref1
       }
-      else ref
+      // Drop the Local flag from all private[this] and protected[this] members
+      // that will be moved to the companion object.
+      if (ref.is(Local) && isDerivedValueClass(ref.owner)) {
+        if (ref1 ne ref) ref1.resetFlag(Local)
+        else ref1 = ref1.copySymDenotation(initFlags = ref1.flags &~ Local)
+      }
+      ref1
     case _ =>
       ref
   }

compiler/src/dotty/tools/dotc/transform/Flatten.scala

@@ -17,6 +17,10 @@ class Flatten extends MiniPhase with SymTransformer {
 
   override def phaseName = "flatten"
 
+  // private[this] and protected[this] modifiers must be dropped
+  // before classes are lifted. Getters drop these modifiers.
+  override def runsAfter = Set(Getters.name)
+
   override def changesMembers = true // the phase removes inner classes
 
   private var LiftedDefs: Store.Location[mutable.ListBuffer[Tree]] = _

compiler/src/dotty/tools/dotc/transform/Getters.scala

@@ -45,26 +45,38 @@ import ValueClasses._
  *      -->  p.x_=(e)
  *
  *  No fields are generated yet. This is done later in phase Memoize.
+ *
+ *  Also, drop the Local flag from all private[this] and protected[this] members.
+ *  This allows subsequent code motions in Flatten.
  */
 class Getters extends MiniPhase with SymTransformer {
   import ast.tpd._
 
-  override def phaseName = "getters"
+  override def phaseName = Getters.name
 
   override def transformSym(d: SymDenotation)(implicit ctx: Context): SymDenotation = {
     def noGetterNeeded =
       d.is(NoGetterNeeded) ||
-      d.initial.asInstanceOf[SymDenotation].is(PrivateLocal) && !d.owner.is(Trait) && !isDerivedValueClass(d.owner) && !d.is(Flags.Lazy) ||
+      d.is(PrivateLocal) && !d.owner.is(Trait) && !isDerivedValueClass(d.owner) && !d.is(Flags.Lazy) ||
       d.is(Module) && d.isStatic ||
       d.hasAnnotation(defn.ScalaStaticAnnot) ||
       d.isSelfSym
-    if (d.isTerm && (d.is(Lazy) || d.owner.isClass) && d.info.isValueType && !noGetterNeeded) {
-      val maybeStable = if (d.isStable) Stable else EmptyFlags
-      d.copySymDenotation(
-        initFlags = d.flags | maybeStable | AccessorCreationFlags,
-        info = ExprType(d.info))
+
+    var d1 =
+      if (d.isTerm && (d.is(Lazy) || d.owner.isClass) && d.info.isValueType && !noGetterNeeded) {
+        val maybeStable = if (d.isStable) Stable else EmptyFlags
+        d.copySymDenotation(
+          initFlags = d.flags | maybeStable | AccessorCreationFlags,
+          info = ExprType(d.info))
+      }
+      else d
+
+    // Drop the Local flag from all private[this] and protected[this] members.
+    if (d1.is(Local)) {
+      if (d1 ne d) d1.resetFlag(Local)
+      else d1 = d1.copySymDenotation(initFlags = d1.flags &~ Local)
     }
-    else d
+    d1
   }
   private val NoGetterNeeded = Method | Param | JavaDefined | JavaStatic
 
@@ -74,3 +86,7 @@ class Getters extends MiniPhase with SymTransformer {
   override def transformAssign(tree: Assign)(implicit ctx: Context): Tree =
     if (tree.lhs.symbol is Method) tree.lhs.becomes(tree.rhs).withPos(tree.pos) else tree
 }
+
+object Getters {
+  val name = "getters"
+}

compiler/src/dotty/tools/dotc/transform/NormalizeFlags.scala

@@ -0,0 +1,15 @@
+object A1 {
+  private[this] class B
+
+  private[this] class C {
+    def cons: B = new B
+  }
+}
+
+class A2 {
+  private[this] class B
+
+  private[this] class C {
+    def cons: B = new B
+  }
+}

tests/pos/flatten.scala

@@ -0,0 +1,15 @@
+class StringOps(val s: String) extends AnyVal {
+  def lines: Iterator[String] = new collection.AbstractIterator[String] {
+    private[this] var index = 0
+    def hasNext: Boolean = false
+    def next(): String = {
+      index = 1
+      ""
+    }
+  }
+}
+
+class FooOps(val s: String) extends AnyVal {
+  private[this] def bar = 2
+  def foo = bar
+}