Cleanup in Refchecks

For historical reasons, when eliminating ModuleDef trees, RefChecks
would check if moduleVar field already exists, and only create it if
not. In reality, the lookup would always fail.

When initially committed, the moduleVar could be created either by
the RefChecks transformer or info transformer, see 256aca6.

This was later changed (3f1f0a4), after which RefChecks only creates
a moduleVar when eliminating a ModuleDef.

3 files changed, 8 insertions, 17 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index 25d45cc819..53bd76f419 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -778,7 +778,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
         val defSym = clazz.newMethod(nme.newLazyValSlowComputeName(lzyVal.name.toTermName), lzyVal.pos, PRIVATE)
         val params = defSym newSyntheticValueParams args.map(_.symbol.tpe)
         defSym setInfoAndEnter MethodType(params, lzyVal.tpe.resultType)
-        val rhs: Tree = (gen.mkSynchronizedCheck(attrThis, cond, syncBody, stats)).changeOwner(currentOwner -> defSym)
+        val rhs: Tree = gen.mkSynchronizedCheck(attrThis, cond, syncBody, stats).changeOwner(currentOwner -> defSym)
         val strictSubst = new TreeSymSubstituterWithCopying(args.map(_.symbol), params)
         addDef(position(defSym), DefDef(defSym, strictSubst(BLOCK(rhs, retVal))))
         defSym
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 36423fa2aa..92b62f3b16 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -1188,20 +1188,8 @@ 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 moduleVar = site newModuleVarSymbol module
         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))
@@ -1217,6 +1205,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
       }
       val newTrees = cdef :: (
         if (module.isStatic)
+          // trait T { def f: Object }; object O extends T { object f }. Need to generate method f in O.
           if (module.isOverridingSymbol) matchingInnerObject() else Nil
         else
           newInnerObject()
diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
index d65d2092ad..c156b8c677 100644
--- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
@@ -335,16 +335,18 @@ trait SyntheticMethods extends ast.TreeDSL {
         }
         for ((m, impl) <- methods ; if shouldGenerate(m)) yield impl()
       }
-      def extras = (
+      def extras = {
         if (needsReadResolve) {
           // Aha, I finally decoded the original comment.
           // This method should be generated as private, but apparently if it is, then
           // it is name mangled afterward.  (Wonder why that is.) So it's only protected.
           // For sure special methods like "readResolve" should not be mangled.
-          List(createMethod(nme.readResolve, Nil, ObjectTpe)(m => { m setFlag PRIVATE ; REF(clazz.sourceModule) }))
+          List(createMethod(nme.readResolve, Nil, ObjectTpe)(m => {
+            m setFlag PRIVATE; REF(clazz.sourceModule)
+          }))
         }
         else Nil
-      )
+      }
 
       try impls ++ extras
       catch { case _: TypeError if reporter.hasErrors => Nil }