Fix #4811: widen Local modifier in ExtensionMethods and Getter

This fixes the crash because in Getter we don't look for a setter that was not
generated.

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.

We instead let ExtensionMethods remove the flag when needed. And then
remove all Local flags in Getters once there are not needed anymore.

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