WIP move module-var and -accessor synthesis out of mixin

src/compiler/scala/tools/nsc/transform/LazyVals.scala

@@ -54,6 +54,12 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD
     private val lazyVals = perRunCaches.newMap[Symbol, Int]() withDefaultValue 0
 
     import symtab.Flags._
+    private def flattenThickets(stats: List[Tree]): List[Tree] = stats.flatMap(_ match {
+      case b @ Block(List(d1@DefDef(_, n1, _, _, _, _)), d2@DefDef(_, n2, _, _, _, _)) if b.tpe == null && n1.endsWith(nme.LAZY_SLOW_SUFFIX) =>
+        List(d1, d2)
+      case stat =>
+        List(stat)
+    })
 
     /** Perform the following transformations:
      *  - for a lazy accessor inside a method, make it check the initialization bitmap
@@ -72,20 +78,14 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD
         case Block(_, _) =>
           val block1 = super.transform(tree)
           val Block(stats, expr) = block1
-          val stats1 = stats.flatMap(_ match {
-            case Block(List(d1@DefDef(_, n1, _, _, _, _)), d2@DefDef(_, n2, _, _, _, _)) if (nme.newLazyValSlowComputeName(n2) == n1) =>
-              List(d1, d2)
-            case stat =>
-              List(stat)
-          })
-          treeCopy.Block(block1, stats1, expr)
+          treeCopy.Block(block1, flattenThickets(stats), expr)
 
         case DefDef(_, _, _, _, _, rhs) => atOwner(tree.symbol) {
           val (res, slowPathDef) = if (!sym.owner.isClass && sym.isLazy) {
             val enclosingClassOrDummyOrMethod = {
               val enclMethod = sym.enclMethod
 
-              if (enclMethod != NoSymbol ) {
+              if (enclMethod != NoSymbol) {
                 val enclClass = sym.enclClass
                 if (enclClass != NoSymbol && enclMethod == enclClass.enclMethod)
                   enclClass
@@ -100,11 +100,26 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD
             val (rhs1, sDef) = mkLazyDef(enclosingClassOrDummyOrMethod, transform(rhs), idx, sym)
             sym.resetFlag((if (lazyUnit(sym)) 0 else LAZY) | ACCESSOR)
             (rhs1, sDef)
-          } else
+          } else if (!sym.owner.isTrait && sym.isModule && sym.isMethod) {
+            rhs match {
+              case b @ Block((assign @ Assign(lhs, rhs)) :: Nil, expr) =>
+                val cond = Apply(Select(lhs, Object_eq), List(Literal(Constant(null))))
+                val moduleDefs = mkFastPathBody(sym.owner.enclClass, lhs.symbol, cond, transform(assign) :: Nil, Nil, transform(expr))
+                (localTyper.typedPos(tree.pos)(moduleDefs._1), localTyper.typedPos(tree.pos)(moduleDefs._2))
+              case rhs =>
+                global.reporter.error(tree.pos, "Unexpected tree on the RHS of a module accessor: " + rhs)
+                (rhs, EmptyTree)
+            }
+          } else {
             (transform(rhs), EmptyTree)
+          }
 
           val ddef1 = deriveDefDef(tree)(_ => if (LocalLazyValFinder.find(res)) typed(addBitmapDefs(sym, res)) else res)
-          if (slowPathDef != EmptyTree) Block(slowPathDef, ddef1) else ddef1
+          if (slowPathDef != EmptyTree) {
+            // The contents of this block are flattened into the enclosing statement sequence, see flattenThickets
+            // This is a poor man's version of dotty's Thicket: https://github.com/lampepfl/dotty/blob/d5280358d1/src/dotty/tools/dotc/ast/Trees.scala#L707
+            Block(slowPathDef, ddef1)
+          } else ddef1
         }
 
         case Template(_, _, body) => atOwner(currentOwner) {
@@ -135,7 +150,7 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD
                 })
                 toAdd0
             } else List()
-          deriveTemplate(tree)(_ => innerClassBitmaps ++ stats)
+          deriveTemplate(tree)(_ => innerClassBitmaps ++ flattenThickets(stats))
         }
 
         case ValDef(_, _, _, _) if !sym.owner.isModule && !sym.owner.isClass =>

src/compiler/scala/tools/nsc/transform/Mixin.scala

@@ -861,16 +861,6 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
         typedPos(init.head.pos)(mkFastPathLazyBody(clazz, lzyVal, cond, syncBody, nulls, retVal))
       }
 
-      def mkInnerClassAccessorDoubleChecked(attrThis: Tree, rhs: Tree, moduleSym: Symbol, args: List[Tree]): Tree =
-        rhs match {
-          case Block(List(assign), returnTree) =>
-            val Assign(moduleVarRef, _) = assign
-            val cond                    = Apply(Select(moduleVarRef, Object_eq), List(NULL))
-            mkFastPathBody(clazz, moduleSym, cond, List(assign), List(NULL), returnTree, attrThis, args)
-          case _ =>
-            abort(s"Invalid getter $rhs for module in $clazz")
-        }
-
       def mkCheckedAccessor(clazz: Symbol, retVal: Tree, offset: Int, pos: Position, fieldSym: Symbol): Tree = {
         val sym = fieldSym.getterIn(fieldSym.owner)
         val bitmapSym = bitmapFor(clazz, offset, sym)
@@ -926,18 +916,6 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
               deriveDefDef(stat)(rhs => Block(List(rhs, localTyper.typed(mkSetFlag(clazz, fieldOffset(getter), getter, bitmapKind(getter)))), UNIT))
             else stat
           }
-          else if (sym.isModule && (!clazz.isTrait || clazz.isImplClass) && !sym.isBridge) {
-            deriveDefDef(stat)(rhs =>
-              typedPos(stat.pos)(
-                mkInnerClassAccessorDoubleChecked(
-                  // Martin to Hubert: I think this can be replaced by selfRef(tree.pos)
-                  // @PP: It does not seem so, it crashes for me trying to bootstrap.
-                  if (clazz.isImplClass) gen.mkAttributedIdent(stat.vparamss.head.head.symbol) else gen.mkAttributedThis(clazz),
-                  rhs, sym, stat.vparamss.head
-                )
-              )
-            )
-          }
           else stat
         }
         stats map {
@@ -1082,18 +1060,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
             })
           }
           else if (sym.isModule && !(sym hasFlag LIFTED | BRIDGE)) {
-            // add modules
-            val vsym = sym.owner.newModuleVarSymbol(sym)
-            addDef(position(sym), ValDef(vsym))
-
-            // !!! TODO - unravel the enormous duplication between this code and
-            // eliminateModuleDefs in RefChecks.
-            val rhs          = gen.newModule(sym, vsym.tpe)
-            val assignAndRet = gen.mkAssignAndReturn(vsym, rhs)
-            val attrThis     = gen.mkAttributedThis(clazz)
-            val rhs1         = mkInnerClassAccessorDoubleChecked(attrThis, assignAndRet, sym, List())
-
-            addDefDef(sym, rhs1)
+            // Moved to Refchecks
           }
           else if (!sym.isMethod) {
             // add fields

src/compiler/scala/tools/nsc/transform/TypingTransformers.scala

@@ -26,7 +26,7 @@ trait TypingTransformers {
 
     def atOwner[A](tree: Tree, owner: Symbol)(trans: => A): A = {
       val savedLocalTyper = localTyper
-      localTyper = localTyper.atOwner(tree, if (owner.isModule) owner.moduleClass else owner)
+      localTyper = localTyper.atOwner(tree, if (owner.isModuleNotMethod) owner.moduleClass else owner)
       val result = super.atOwner(owner)(trans)
       localTyper = savedLocalTyper
       result

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -1188,26 +1188,6 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
       // set NoType so it will be ignored.
       val cdef          = ClassDef(module.moduleClass, impl) setType NoType
 
-      // Create the module var unless the immediate owner is a class and
-      // the module var already exists there. See SI-5012, SI-6712.
-      def findOrCreateModuleVar() = {
-        val vsym = (
-          if (site.isTerm) NoSymbol
-          else site.info decl nme.moduleVarName(moduleName)
-        )
-        vsym orElse (site newModuleVarSymbol module)
-      }
-      def newInnerObject() = {
-        // Create the module var unless it is already in the module owner's scope.
-        // The lookup is on module.enclClass and not module.owner lest there be a
-        // nullary method between us and the class; see SI-5012.
-        val moduleVar = findOrCreateModuleVar()
-        val rhs       = gen.newModule(module, moduleVar.tpe)
-        val body      = if (site.isTrait) rhs else gen.mkAssignAndReturn(moduleVar, rhs)
-        val accessor  = DefDef(module, body.changeOwner(moduleVar -> module))
-
-        ValDef(moduleVar) :: accessor :: Nil
-      }
       def matchingInnerObject() = {
         val newFlags = (module.flags | STABLE) & ~MODULE
         val newInfo  = NullaryMethodType(module.moduleClass.tpe)
@@ -1219,10 +1199,45 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
         if (module.isStatic)
           if (module.isOverridingSymbol) matchingInnerObject() else Nil
         else
-          newInnerObject()
+          newInnerObject(site, module)
       )
       transformTrees(newTrees map localTyper.typedPos(moduleDef.pos))
     }
+    def newInnerObject(site: Symbol, module: Symbol): List[Tree] = {
+      // Create the module var unless the immediate owner is a class and
+      // the module var already exists there. See SI-5012, SI-6712.
+      def findOrCreateModuleVar() = {
+        val vsym = (
+          if (site.isTerm) NoSymbol
+          else site.info decl nme.moduleVarName(module.name.toTermName)
+          )
+        vsym orElse (site newModuleVarSymbol module)
+      }
+      // Create the module var unless it is already in the module owner's scope.
+      // The lookup is on module.enclClass and not module.owner lest there be a
+      // nullary method between us and the class; see SI-5012.
+      val moduleVar = findOrCreateModuleVar()
+      val rhs       = gen.newModule(module, moduleVar.tpe)
+      val body      = if (site.isTrait) rhs else {
+        if (moduleVar.enclClass == NoSymbol) gen.mkAssignAndReturn(moduleVar, rhs) // e.g. neg/t6666c.scala, will be an error later on.
+        else gen.mkAssignAndReturn(moduleVar, rhs) // will be wrapped in double checked lock idiom in lazyvals
+      }
+      val accessor  = if (module.owner == site) module else {
+        site.newMethod(module.name.toTermName, site.pos, STABLE | MODULE | MIXEDIN).setInfo(moduleVar.tpe).andAlso(self => if (module.isPrivate) self.expandName(module.owner))
+      }
+      val accessorDef = DefDef(accessor, body.changeOwner(moduleVar -> accessor))
+
+      ValDef(moduleVar) :: accessorDef :: Nil
+    }
+
+    def mixinModuleDefs(clazz: Symbol): List[Tree] = {
+      val res = for {
+        mixinClass <- clazz.mixinClasses.iterator
+        module     <- mixinClass.info.decls.iterator.filter(_.isModule)
+        newMember  <- newInnerObject(clazz, module)
+      } yield transform(localTyper.typedPos(clazz.pos)(newMember))
+      res.toList
+    }
 
     def transformStat(tree: Tree, index: Int): List[Tree] = tree match {
       case t if treeInfo.isSelfConstrCall(t) =>
@@ -1651,11 +1666,12 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
             // SI-7870 default getters for constructors live in the companion module
             checkOverloadedRestrictions(currentOwner, currentOwner.companionModule)
             val bridges = addVarargBridges(currentOwner)
+            val moduleDesugared = if (currentOwner.isTrait) Nil else mixinModuleDefs(currentOwner)
             checkAllOverrides(currentOwner)
             checkAnyValSubclass(currentOwner)
             if (currentOwner.isDerivedValueClass)
               currentOwner.primaryConstructor makeNotPrivate NoSymbol // SI-6601, must be done *after* pickler!
-            if (bridges.nonEmpty) deriveTemplate(tree)(_ ::: bridges) else tree
+            if (bridges.nonEmpty || moduleDesugared.nonEmpty) deriveTemplate(tree)(_ ::: bridges ::: moduleDesugared) else tree
 
           case dc@TypeTreeWithDeferredRefCheck() => abort("adapt should have turned dc: TypeTreeWithDeferredRefCheck into tpt: TypeTree, with tpt.original == dc")
           case tpt@TypeTree() =>

src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala

@@ -387,7 +387,7 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
       val savedValid = validCurrentOwner
       if (owner.isClass) validCurrentOwner = true
       val savedLocalTyper = localTyper
-      localTyper = localTyper.atOwner(tree, if (owner.isModule) owner.moduleClass else owner)
+      localTyper = localTyper.atOwner(tree, if (owner.isModuleNotMethod) owner.moduleClass else owner)
       typers = typers updated (owner, localTyper)
       val result = super.atOwner(tree, owner)(trans)
       localTyper = savedLocalTyper

src/reflect/scala/reflect/internal/StdNames.scala

@@ -876,7 +876,7 @@ trait StdNames {
     val toCharacter: NameType = "toCharacter"
     val toInteger: NameType   = "toInteger"
 
-    def newLazyValSlowComputeName(lzyValName: Name) = lzyValName append LAZY_SLOW_SUFFIX
+    def newLazyValSlowComputeName(lzyValName: Name) = (lzyValName stripSuffix MODULE_VAR_SUFFIX append LAZY_SLOW_SUFFIX).toTermName
 
     // ASCII names for operators
     val ADD       = encode("+")

src/reflect/scala/reflect/internal/Symbols.scala

@@ -323,7 +323,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def newModuleVarSymbol(accessor: Symbol): TermSymbol = {
       val newName  = nme.moduleVarName(accessor.name.toTermName)
       val newFlags = MODULEVAR | ( if (this.isClass) PrivateLocal | SYNTHETIC else 0 )
-      val newInfo  = accessor.tpe.finalResultType
+      val newInfo  = thisType.memberType(accessor).finalResultType
       val mval     = newVariable(newName, accessor.pos.focus, newFlags.toLong) addAnnotation VolatileAttr
 
       if (this.isClass)

src/reflect/scala/reflect/internal/transform/Erasure.scala

@@ -123,7 +123,7 @@ trait Erasure {
       case tref @ TypeRef(pre, sym, args) =>
         if (sym == ArrayClass)
           if (unboundedGenericArrayLevel(tp) == 1) ObjectTpe
-          else if (args.head.typeSymbol.isBottomClass) arrayType(ObjectTpe)
+          else if (args.head.typeSymbol.isBottomClass)  arrayType(ObjectTpe)
           else typeRef(apply(pre), sym, args map applyInArray)
         else if (sym == AnyClass || sym == AnyValClass || sym == SingletonClass) ObjectTpe
         else if (sym == UnitClass) BoxedUnitTpe

test/files/run/delambdafy_t6028.check

@@ -38,10 +38,24 @@ package <empty> {
       <synthetic> <stable> <artifact> def $outer(): T = MethodLocalObject$2.this.$outer;
       <synthetic> <stable> <artifact> def $outer(): T = MethodLocalObject$2.this.$outer
     };
-    final <stable> private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
-      MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
+    final <stable> private[this] def MethodLocalObject$lzycompute$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
+      {
+        T.this.synchronized({
+          if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+            {
+              MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
+              ()
+            };
+          scala.runtime.BoxedUnit.UNIT
+        });
+        ()
+      };
       MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
     };
+    final <stable> private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+      T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1)
+    else
+      MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]();
     abstract trait MethodLocalTrait$1$class extends Object with T#MethodLocalTrait$1 {
       def /*MethodLocalTrait$1$class*/$init$(barParam$1: String): Unit = {
         ()

test/files/run/t6028.check

@@ -50,10 +50,24 @@ package <empty> {
       <synthetic> <stable> <artifact> def $outer(): T = MethodLocalObject$2.this.$outer;
       <synthetic> <stable> <artifact> def $outer(): T = MethodLocalObject$2.this.$outer
     };
-    final <stable> private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
-      MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
+    final <stable> private[this] def MethodLocalObject$lzycompute$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
+      {
+        T.this.synchronized({
+          if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+            {
+              MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
+              ()
+            };
+          scala.runtime.BoxedUnit.UNIT
+        });
+        ()
+      };
       MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
     };
+    final <stable> private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+      T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1)
+    else
+      MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]();
     abstract trait MethodLocalTrait$1$class extends Object with T#MethodLocalTrait$1 {
       def /*MethodLocalTrait$1$class*/$init$(barParam$1: Int): Unit = {
         ()

test/files/run/trait-defaults-modules.scala

@@ -0,0 +1,20 @@
+trait T1 { def a: Any }
+
+trait T2 extends T1 { object a; object b; private object c; def usec: Any = c}
+trait T3 extends T2
+
+class C1 extends T1 { object a; object b }
+class C2 extends C1
+class C3 extends T2
+class C4 extends T3
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val (c1, c2, c3, c4) = (new C1, new C2, new C3, new C4)
+    c1.a; c1.b; (c1: T1).a
+    c2.a; c2.b; (c2: T1).a
+    c3.a; c3.b; (c3: T1).a; c3.usec
+    c4.a; c4.b; (c4: T1).a; c4.usec
+  }
+
+}

test/files/run/trait-defaults-modules2/T_1.scala

@@ -0,0 +1,4 @@
+trait T {
+  private object O
+  def useO: Any = O
+}

test/files/run/trait-defaults-modules2/Test_2.scala

@@ -0,0 +1,5 @@
+object Test extends T {
+  def main(args: Array[String]): Unit = {
+    useO
+  }
+}

test/files/run/trait-defaults-modules3.scala

@@ -0,0 +1,8 @@
+object Test {
+  def main(args: Array[String]): Unit = {
+    object O
+    val x = O
+    val y = O
+    assert(x eq y)
+  }
+}