rewrite of nested objects implementation.

review by odersky, dragos and whoever feels like it.

10 files changed, 44 insertions, 91 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
index b655507658..406d819147 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
@@ -272,7 +272,7 @@ abstract class TreeGen {
 
   // Builds a tree of the form "{ lhs = rhs ; lhs  }"
   def mkAssignAndReturn(lhs: Symbol, rhs: Tree): Tree = {
-    val lhsRef = mkAttributedRef(lhs)
+    val lhsRef = mkUnattributedRef(lhs)
     Block(Assign(lhsRef, rhs) :: Nil, lhsRef)
   }
 
@@ -280,9 +280,10 @@ abstract class TreeGen {
     val mval = (
       accessor.owner.newVariable(accessor.pos.focus, nme.moduleVarName(accessor.name))
       setInfo accessor.tpe.finalResultType
-      setFlag (LAZY | MODULEVAR)
-      setLazyAccessor accessor
+      setFlag (MODULEVAR)
     )
+
+    mval.addAnnotation(AnnotationInfo(VolatileAttr.tpe, Nil, Nil))
     if (mval.owner.isClass) {
       mval setFlag (PRIVATE | LOCAL | SYNTHETIC)
       mval.owner.info.decls.enter(mval)
diff --git a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
index db616e9657..2798d856f9 100644
--- a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
+++ b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
@@ -241,7 +241,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { thisFactory
     def isDocTemplate = true
     def companion = sym.companionSymbol match {
       case NoSymbol => None
-      case comSym if !isEmptyJavaObject(comSym) && (comSym.isClass || comSym.isModule || isNestedObjectLazyVal(comSym)) =>
+      case comSym if !isEmptyJavaObject(comSym) && (comSym.isClass || comSym.isModule) =>
         Some(makeDocTemplate(comSym, inTpl))
       case _ => None
     }
@@ -387,12 +387,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { thisFactory
           members collect { case d: Constructor => d }
         def primaryConstructor = constructors find { _.isPrimary }
       }
-    else if (isNestedObjectLazyVal(bSym)) {
-      new DocTemplateImpl(bSym, minimumInTpl) with Object {
-        override def isObject = true
-        override def isLazyVal = false
-      }
-    } else
+    else
       throw new Error("'" + bSym + "' that isn't a class, trait or object cannot be built as a documentable template")
   }
 
@@ -432,11 +427,6 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { thisFactory
 
     def makeMember0(bSym: Symbol): Option[MemberImpl] = {
       if (bSym.isGetter && bSym.isLazy)
-        if (isNestedObjectLazyVal(bSym))
-          if (templateShouldDocument(bSym))
-            Some(makeDocTemplate(bSym, inTpl))
-          else None
-        else
           Some(new NonTemplateMemberImpl(bSym, inTpl) with Val {
             override lazy val comment = // The analyser does not duplicate the lazy val's DocDef when it introduces its accessor.
               thisFactory.comment(bSym.accessed, inTpl) // This hack should be removed after analyser is fixed.
@@ -644,16 +634,12 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { thisFactory
     ( aSym.owner == NoSymbol || templateShouldDocument(aSym.owner) ) && !isEmptyJavaObject(aSym)
   }
 
-  def isNestedObjectLazyVal(aSym: Symbol): Boolean = {
-    aSym.isLazyAccessor && !aSym.isRootPackage && !aSym.owner.isPackageClass
-  }
-
   def isEmptyJavaObject(aSym: Symbol): Boolean = {
     def hasMembers = aSym.info.members.exists(s => localShouldDocument(s) && (!s.isConstructor || s.owner == aSym))
     aSym.isModule && aSym.isJavaDefined && !hasMembers
   }
 
   def localShouldDocument(aSym: Symbol): Boolean = {
-    !aSym.isPrivate && (aSym.isProtected || aSym.privateWithin == NoSymbol) && (!aSym.isSynthetic || isNestedObjectLazyVal(aSym))
+    !aSym.isPrivate && (aSym.isProtected || aSym.privateWithin == NoSymbol) && !aSym.isSynthetic
   }
 }
diff --git a/src/compiler/scala/tools/nsc/symtab/Symbols.scala b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
index 524b5a047d..308eeeb590 100644
--- a/src/compiler/scala/tools/nsc/symtab/Symbols.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
@@ -1323,26 +1323,12 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
      *  companion module of a class. If a companion module exists, its symbol is
      *  returned, otherwise, `NoSymbol` is returned. The method assumes that
      *  `this` symbol has already been checked to be a class (using `isClass`).
-     *
-     *  After refchecks nested objects get transformed to lazy vals so we
-     *  filter on LAZY flag as well.
-     * @note The resident compiler may run many times over, and symbols may be
-     *       reused. Therefore, a module symbol that has been translated to a
-     *       lazy val by refchecks is not guaranteed to have MODULE set on the
-     *       next run (even before refcheck). Flags are not part of symbol
-     *       history. Instead we rely on the fact that a synthetic lazy value
-     *       must have been a  module.
-     */
-    private final def companionModule0: Symbol = {
-      def isSyntheticLazy(sym: Symbol) =
-        (sym.hasAllFlags(LAZY | SYNTHETIC))
-
+     */
+    private final def companionModule0: Symbol =
       flatOwnerInfo.decl(name.toTermName).suchThat(
-        sym => (sym isCoDefinedWith this)
-                && (sym.hasFlag(MODULE) || isSyntheticLazy(sym)))
-    }
+        sym => sym.hasFlag(MODULE) && (sym isCoDefinedWith this) && !sym.isMethod)
 
-    /** For a class: the module or case class factory wiht the same name in the same package.
+    /** For a class: the module or case class factory with the same name in the same package.
      *  For all others: NoSymbol
      *  Note: does not work for modules owned by methods, see Namers.companionModuleOf
      *
diff --git a/src/compiler/scala/tools/nsc/symtab/Types.scala b/src/compiler/scala/tools/nsc/symtab/Types.scala
index 61e1d118c5..87fc2b4963 100644
--- a/src/compiler/scala/tools/nsc/symtab/Types.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Types.scala
@@ -3811,20 +3811,7 @@ A type's typeSymbol should never be inspected directly.
       if (phase.flatClasses) {
         sym
       } else if (sym.isModuleClass) {
-        val adaptedSym = adaptToNewRun(pre, sym.sourceModule)
-        // Handle nested objects properly
-        val result0 = if (adaptedSym.isLazy) adaptedSym.lazyAccessor else adaptedSym.moduleClass
-        val result = if (result0 == NoSymbol)
-          // The only possible way we got here is when
-          // object is defined inside the method and unfortunately
-          // we have no way of retrieving that information (and using it)
-          // at this point, so just use the old symbol.
-          // This also means that sym.sourceModule == adaptedSym since
-          // pre == NoPrefix. see #4215
-          sym
-        else result0
-
-        result
+        adaptToNewRun(pre, sym.sourceModule).moduleClass
       } else if ((pre eq NoPrefix) || (pre eq NoType) || sym.isPackageClass) {
         sym
       } else {
@@ -4729,7 +4716,7 @@ A type's typeSymbol should never be inspected directly.
   def specializesSym(tp: Type, sym: Symbol): Boolean =
     tp.typeSymbol == NothingClass ||
     tp.typeSymbol == NullClass && (sym.owner isSubClass ObjectClass) ||
-    (tp.member(sym.name).alternatives exists
+    (tp.nonPrivateMember(sym.name).alternatives exists
       (alt => sym == alt || specializesSym(tp.narrow, alt, sym.owner.thisType, sym)))
 
   /** Does member `sym1' of `tp1' have a stronger type
diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
index 79f13cc6d3..6737c3e23f 100644
--- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
@@ -179,7 +179,7 @@ abstract class ExplicitOuter extends InfoTransform
       // class needs to have a common naming scheme, independently of whether
       // the field was accessed from an inner class or not. See #2946
       if (sym.owner.isTrait && sym.hasLocalFlag &&
-              ((sym.getter(sym.owner.toInterface) == NoSymbol) && !sym.isLazyAccessor))
+              (sym.getter(sym.owner.toInterface) == NoSymbol))
         sym.makeNotPrivate(sym.owner)
       tp
   }
diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala
index 596f7c1d1c..d147d408e4 100644
--- a/src/compiler/scala/tools/nsc/transform/Flatten.scala
+++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala
@@ -50,9 +50,7 @@ abstract class Flatten extends InfoTransform {
           for (sym <- decls.toList) {
             if (sym.isTerm && !sym.isStaticModule) {
               decls1 enter sym
-              if (sym.isModule) sym.moduleClass setFlag LIFTED  // Only top modules
-              // Nested modules (MODULE flag is reset so we access through lazy):
-              if (sym.isModuleVar && sym.isLazy) sym.lazyAccessor.lazyAccessor setFlag LIFTED
+              if (sym.isModule) sym.moduleClass setFlag LIFTED
             } else if (sym.isClass) {
               liftClass(sym)
               if (sym.needsImplClass) liftClass(erasure.implClass(sym))
diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
index 9cfd497c76..6764de85d6 100644
--- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
+++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
@@ -399,6 +399,7 @@ abstract class LambdaLift extends InfoTransform {
         case Block(stats, expr0) =>
           val (lzyVals, rest) = stats.partition {
                       case stat@ValDef(_, _, _, _) if stat.symbol.isLazy => true
+                      case stat@ValDef(_, _, _, _) if stat.symbol.hasFlag(MODULEVAR) => true
                       case _                                             => false
                   }
           treeCopy.Block(tree, lzyVals:::rest, expr0)
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index d98281aaa4..406e97bb8c 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -828,6 +828,18 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
         typedPos(init.head.pos)(BLOCK(result, retVal))
       }
 
+      def mkInnerClassAccessorDoubleChecked(clazz: Symbol, rhs: Tree): Tree =
+        rhs match {
+          case Block(List(assign), returnTree) =>
+            val Assign(moduleVarRef, _) = assign
+            val cond = Apply(Select(moduleVarRef, nme.eq),List(Literal(Constant(null))))
+            val doubleSynchrTree = gen.mkDoubleCheckedLocking(clazz, cond, List(assign), Nil)
+            Block(List(doubleSynchrTree), returnTree)
+          case _ =>
+            assert(false, "Invalid getter " + rhs + " for module in class " + clazz)
+            EmptyTree
+        }
+
       def mkCheckedAccessor(clazz: Symbol, retVal: Tree, offset: Int, pos: Position, fieldSym: Symbol): Tree = {
         val bitmapSym = bitmapFor(clazz, offset, fieldSym.getter(fieldSym.owner))
         val mask      = LIT(1 << (offset % FLAGS_PER_WORD))
@@ -877,7 +889,10 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
                         Block(List(rhs, localTyper.typed(mkSetFlag(clazz, fieldOffset(getter), getter))), UNIT))
               else
                 stat
-
+          case DefDef(mods, name, tp, vp, tpt, rhs)
+            if sym.isModule && !clazz.isTrait && !sym.hasFlag(BRIDGE) =>
+              val rhs1 = mkInnerClassAccessorDoubleChecked(clazz, rhs)
+              treeCopy.DefDef(stat, mods, name, tp, vp, tpt, typedPos(stat.pos)(rhs1))
           case _ => stat
         }
         stats1
@@ -1068,6 +1083,15 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
                 } else
                   gen.mkCheckInit(accessedRef)
               })
+            } else if (sym.isModule && !(sym hasFlag LIFTED | BRIDGE)) {
+              // add modules
+              val vdef = gen.mkModuleVarDef(sym)
+              addDef(position(sym), vdef)
+
+              val rhs  = gen.newModule(sym, vdef.symbol.tpe)
+              val assignAndRet = gen.mkAssignAndReturn(vdef.symbol, rhs)
+              val rhs1 = mkInnerClassAccessorDoubleChecked(clazz, assignAndRet)
+              addDef(position(sym), DefDef(sym, rhs1))
             } else if (!sym.isMethod) {
               // add fields
               addDef(position(sym), ValDef(sym))
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index c08c55f4e4..f1eb904c58 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -51,24 +51,11 @@ abstract class RefChecks extends InfoTransform {
     new RefCheckTransformer(unit)
   override def changesBaseClasses = false
 
-  def transformInfo(sym: Symbol, tp: Type): Type = {
-    def isNestedModule = sym.isModule && !sym.isRootPackage && !sym.owner.isPackageClass
-
+  def transformInfo(sym: Symbol, tp: Type): Type =
     if (sym.isModule && !sym.isStatic) {
-
       sym setFlag (lateMETHOD | STABLE)
-      if (isNestedModule) {
-        val moduleVar = sym.owner.info.decl(nme.moduleVarName(sym.name.toTermName))
-        if (moduleVar == NoSymbol) {
-          val tree = gen.mkModuleVarDef(sym)
-          tree.symbol.setInfo(tp)
-          sym.resetFlag(MODULE)
-          sym.setFlag(LAZY | ACCESSOR)
-        }
-      }
       NullaryMethodType(tp)
     } else tp
-  }
 
   val toJavaRepeatedParam = new TypeMap {
     def apply(tp: Type) = tp match {
@@ -1040,17 +1027,12 @@ abstract class RefChecks extends InfoTransform {
       val ModuleDef(mods, name, impl) = tree
       val sym = tree.symbol
 
-      // transformedInfo check is necessary here because the object info may already
-      // have been transformed, and we do not want to have duplicate lazy accessors
-      // (through duplicate nested object -> lazy val transformation.)
-      val transformedInfo = sym.isLazy
-      val classSym        = if (transformedInfo) sym.lazyAccessor else sym.moduleClass
+      val classSym        = sym.moduleClass
       val cdef            = ClassDef(mods | MODULE, name.toTypeName, Nil, impl) setSymbol classSym setType NoType
 
       def findOrCreateModuleVar() = localTyper.typedPos(tree.pos) {
         lazy val createModuleVar = gen.mkModuleVarDef(sym)
-        if (!transformedInfo) createModuleVar
-        else sym.owner.info.decl(nme.moduleVarName(sym.name.toTermName)) match {
+        sym.owner.info.decl(nme.moduleVarName(sym.name.toTermName)) match {
           // In case we are dealing with local symbol then we already have
           // to correct error with forward reference
           case NoSymbol => createModuleVar
@@ -1071,13 +1053,6 @@ abstract class RefChecks extends InfoTransform {
           val vsym = vdef.symbol
           atPhase(phase.next) {
             val rhs  = gen.newModule(sym, vsym.tpe)
-            // side effecting symbol flags
-            if (!transformedInfo) {
-              sym resetFlag (MODULE | FINAL | CASE)
-              sym setFlag (LAZY | ACCESSOR | SYNTHETIC)
-              sym setInfo NullaryMethodType(sym.tpe)
-              sym setFlag (lateMETHOD | STABLE)
-            }
             val body = if (sym.owner.isTrait) rhs else gen.mkAssignAndReturn(vsym, rhs)
             DefDef(sym, body.changeOwner(vsym -> sym))
           }
diff --git a/test/files/run/bug2873.scala b/test/files/run/bug2873.scala
deleted file mode 100644
index a21ab56063..0000000000
--- a/test/files/run/bug2873.scala
+++ /dev/null
@@ -1,5 +0,0 @@
-object Test {
-  def main(args: Array[String]): Unit = {
-    classOf[scala.collection.immutable.RedBlack[_]].getMethod("Empty").getGenericReturnType
-  }
-}