SI-8907 Don't force symbol info in isModuleNotMethod

Test case by Jason.

RefChecks adds the lateMETHOD flag lazily in its info transformer.
This means that forcing the `sym.info` may change the value of
`sym.isMethod`.

0ccdb151f introduced a check to force the info in isModuleNotMethod,
but it turns out this leads to errors on stub symbols (SI-8907).

The responsibility to force info is transferred to callers, which
is the case for other operations on symbols, too.

3 files changed, 66 insertions, 24 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala
index fa53ef48b5..4662ef6224 100644
--- a/src/compiler/scala/tools/nsc/transform/Flatten.scala
+++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala
@@ -77,8 +77,11 @@ abstract class Flatten extends InfoTransform {
               if (sym.isTerm && !sym.isStaticModule) {
                 decls1 enter sym
                 if (sym.isModule) {
-                  // Nested, non-static moduls are transformed to methods.
-                  assert(sym.isMethod, s"Non-static $sym should have the lateMETHOD flag from RefChecks")
+                  // In theory, we could assert(sym.isMethod), because nested, non-static moduls are
+                  // transformed to methods (lateMETHOD flag added in RefChecks). But this requires
+                  // forcing sym.info (see comment on isModuleNotMethod), which forces stub symbols
+                  // too eagerly (SI-8907).
+
                   // Note that module classes are not entered into the 'decls' of the ClassInfoType
                   // of the outer class, only the module symbols are. So the current loop does
                   // not visit module classes. Therefore we set the LIFTED flag here for module
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index b0c23ef45d..2c039ab5a7 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -735,31 +735,31 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
 
     final def hasGetter = isTerm && nme.isLocalName(name)
 
-    /** A little explanation for this confusing situation.
-     *  Nested modules which have no static owner when ModuleDefs
-     *  are eliminated (refchecks) are given the lateMETHOD flag,
-     *  which makes them appear as methods after refchecks.
-     *  Here's an example where one can see all four of FF FT TF TT
-     *  for (isStatic, isMethod) at various phases.
+    /**
+     * Nested modules which have no static owner when ModuleDefs are eliminated (refchecks) are
+     * given the lateMETHOD flag, which makes them appear as methods after refchecks.
+     *
+     * Note: the lateMETHOD flag is added lazily in the info transformer of the RefChecks phase.
+     * This means that forcing the `sym.info` may change the value of `sym.isMethod`. Forcing the
+     * info is in the responsability of the caller. Doing it eagerly here was tried (0ccdb151f) but
+     * has proven to lead to bugs (SI-8907).
      *
-     *    trait A1 { case class Quux() }
-     *    object A2 extends A1 { object Flax }
-     *    // --  namer         object Quux in trait A1
-     *    // -M  flatten       object Quux in trait A1
-     *    // S-  flatten       object Flax in object A2
-     *    // -M  posterasure   object Quux in trait A1
-     *    // -M  jvm           object Quux in trait A1
-     *    // SM  jvm           object Quux in object A2
+     * Here's an example where one can see all four of FF FT TF TT for (isStatic, isMethod) at
+     * various phases.
      *
-     *  So "isModuleNotMethod" exists not for its achievement in
-     *  brevity, but to encapsulate the relevant condition.
+     *   trait A1 { case class Quux() }
+     *   object A2 extends A1 { object Flax }
+     *   // --  namer         object Quux in trait A1
+     *   // -M  flatten       object Quux in trait A1
+     *   // S-  flatten       object Flax in object A2
+     *   // -M  posterasure   object Quux in trait A1
+     *   // -M  jvm           object Quux in trait A1
+     *   // SM  jvm           object Quux in object A2
+     *
+     * So "isModuleNotMethod" exists not for its achievement in brevity, but to encapsulate the
+     * relevant condition.
      */
-    def isModuleNotMethod = {
-      if (isModule) {
-        if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there.
-        !isMethod
-      } else false
-    }
+    def isModuleNotMethod = isModule && !isMethod
 
     // After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should
     // be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module
diff --git a/test/files/run/t8907.scala b/test/files/run/t8907.scala
new file mode 100644
index 0000000000..7952ac82d9
--- /dev/null
+++ b/test/files/run/t8907.scala
@@ -0,0 +1,39 @@
+import scala.tools.partest._
+import java.io.File
+
+object Test extends StoreReporterDirectTest {
+  def code = ???
+
+  def compileCode(code: String) = {
+    val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
+    compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code)
+  }
+
+  def show(): Unit = {
+    compileCode("""
+      class C { class Inner }
+
+      class D {
+        object O {
+          def foo(c: C)(i: c.Inner): c.Inner = ???
+        }
+      }
+      """)
+    assert(filteredInfos.isEmpty, filteredInfos)
+    deleteClass("C")
+    compileCode("""
+      class E {
+        def foo = {
+          (null: D).toString
+        }
+      }
+    """)
+    assert(storeReporter.infos.isEmpty, storeReporter.infos.mkString("\n")) // Included a MissingRequirementError before.
+  }
+
+  def deleteClass(name: String) {
+    val classFile = new File(testOutput.path, name + ".class")
+    assert(classFile.exists)
+    assert(classFile.delete())  
+  }
+}