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

This fix the crash because we don't need to generate a getter or setter
for private[this] field anymore.

The original documentation for ElimLocals says:

This phase drops the Local flag from all private[this] and protected[this] members.
This allows subsequent code motions where code is moved from a class to its
companion object. It invalidates variance checking, which is consequently disabled
when retyping.

I believe this is legacy code and not true anymore. This phase is still
required to Ycheck Flatten. Not sure why...

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
@@ -111,7 +110,8 @@ class Compiler {
          new LambdaLift,             // Lifts out nested functions to class scope, storing free variables in environments
                                         // Note: in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here
          new ElimStaticThis) ::      // Replace `this` references to static objects by global identifiers
-    List(new Flatten,                // Lift all inner classes to package scope
+    List(new ElimLocals,             // Widens private[this] and protected[this] modifiers to just private/protected
+         new Flatten,                // Lift all inner classes to package scope
          new RenameLifted,           // Renames lifted classes to local numbering scheme
          new TransformWildcards,     // Replace wildcards with default values
          new MoveStatics,            // Move static methods to companion classes

compiler/src/dotty/tools/dotc/transform/NormalizeFlags.scala renamed to compiler/src/dotty/tools/dotc/transform/ElimLocals.scala

@@ -9,17 +9,18 @@ import SymDenotations.SymDenotation
 import MegaPhase.MiniPhase
 import Flags._, Symbols._
 
-/** 1. Widens all private[this] and protected[this] qualifiers to just private/protected
- *  2. Sets PureInterface flag for traits that only have pure interface members and that
- *     do not have initialization code. A pure interface member is either an abstract
- *     or alias type definition or a deferred val or def.
+/** Widens private[this] and protected[this] modifiers to just private/protected
  */
-class NormalizeFlags extends MiniPhase with SymTransformer {
-  override def phaseName = "normalizeFlags"
+class ElimLocals extends MiniPhase with SymTransformer {
+  override def phaseName = ElimLocals.name
 
   def transformSym(ref: SymDenotation)(implicit ctx: Context) = {
     var newFlags = ref.flags &~ Local
     if (newFlags != ref.flags) ref.copySymDenotation(initFlags = newFlags)
     else ref
   }
 }
+
+object ElimLocals {
+  def name = "elimlocals"
+}

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

@@ -17,6 +17,9 @@ class Flatten extends MiniPhase with SymTransformer {
 
   override def phaseName = "flatten"
 
+  // private[this] modifiers on classes need be widened before lifting the class
+  override def runsAfter = Set(ElimLocals.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

@@ -54,7 +54,7 @@ class Getters extends MiniPhase with SymTransformer {
   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

tests/pos/flatten.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/i4811.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
+}