SI-8196 Runtime reflection robustness for STATIC impl details

Scala's runtime reflection works in few modes. The primary mode reads
reads out the pickled signatures from ScalaSig annotations, if
avaialable. However, these aren't available for Java-defined classes
(obviously) nor for local Scala-defined classes (less obviously.),
and the Scala `Symbol`s and `Types` must be reconstructed from
the Java generic reflection metadata.

This bug occurs in the last case, and is centered in
`FromJavaClassCompleter`.

In that completer, member fields and methods are given an owner
based on the STATIC modifier. That makes sense for Java defined
classes. I'm not 100% if it makes sense for Scala defined classes;
maybe we should just skip them entirely?

This patch still includes them, but makes the ownership-assignment
more robust in the face of STATIC members emitted by the Scala
compiler backend, such as the cache fields for structural calls.
(It's reflection all the way down!). We might not have a companion
module at all, so before we ended up owning those by `NoSymbol`,
and before too long hit the dreaded NSDHNAO crash.

That crash doesn't exist any more on 2.11 (it is demoted to a
-Xdev warning), but this patch still makes sense on that branch.

This commit makes `followStatic` and `enter` more robust when
finding a suitable owner for static members. I've also factored
out the duplicated logic between the two.

4 files changed, 45 insertions, 4 deletions

diff --git a/bincompat-forward.whitelist.conf b/bincompat-forward.whitelist.conf
index 087fa07b37..7879ecfd84 100644
--- a/bincompat-forward.whitelist.conf
+++ b/bincompat-forward.whitelist.conf
@@ -173,6 +173,11 @@ filter {
         {
             matchName="scala.reflect.runtime.SymbolLoaders.isInvalidClassName"
             problemName=MissingMethodProblem
+        },
+        {
+            matchName="scala.reflect.runtime.JavaMirrors#JavaMirror.scala$reflect$runtime$JavaMirrors$JavaMirror$$followStatic"
+            problemName=MissingMethodProblem
         }
+
     ]
 }
diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index 6fdb238462..9c0781ca06 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -685,8 +685,7 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
           module.moduleClass setInfo new ClassInfoType(List(), newScope, module.moduleClass)
         }
 
-        def enter(sym: Symbol, mods: Int) =
-          (if (jModifier.isStatic(mods)) module.moduleClass else clazz).info.decls enter sym
+        def enter(sym: Symbol, mods: Int) = followStatic(clazz, module, mods).info.decls enter sym
 
         def enterEmptyCtorIfNecessary(): Unit = {
           if (jclazz.getConstructors.isEmpty)
@@ -733,8 +732,12 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
      * If Java modifiers `mods` contain STATIC, return the module class
      *  of the companion module of `clazz`, otherwise the class `clazz` itself.
      */
-    private def followStatic(clazz: Symbol, mods: Int) =
-      if (jModifier.isStatic(mods)) clazz.companionModule.moduleClass else clazz
+    private def followStatic(clazz: Symbol, mods: Int): Symbol = followStatic(clazz, clazz.companionModule, mods)
+
+    private def followStatic(clazz: Symbol, module: Symbol, mods: Int): Symbol =
+      // SI-8196 `orElse(clazz)` needed for implementation details of the backend, such as the static
+      //         field containing the cache for structural calls.
+      if (jModifier.isStatic(mods)) module.moduleClass.orElse(clazz) else clazz
 
   /** Methods which need to be treated with care
    *  because they either are getSimpleName or call getSimpleName:
diff --git a/test/files/run/t8196.check b/test/files/run/t8196.check
new file mode 100644
index 0000000000..3286c15c91
--- /dev/null
+++ b/test/files/run/t8196.check
@@ -0,0 +1,3 @@
+Scope{
+  final private val f1: Int
+}
diff --git a/test/files/run/t8196.scala b/test/files/run/t8196.scala
new file mode 100644
index 0000000000..d526eafbb3
--- /dev/null
+++ b/test/files/run/t8196.scala
@@ -0,0 +1,30 @@
+object Test extends App {
+ 
+  trait FormTrait {
+    import scala.reflect.runtime.{ universe => ru }
+
+    val runtimeMirror = ru.runtimeMirror(this.getClass.getClassLoader)
+    val instanceMirror = runtimeMirror.reflect(this)
+    val members = instanceMirror.symbol.typeSignature.members
+    def fields = members.filter(_.typeSignature <:< ru.typeOf[Int])
+  }
+ 
+  val f = () => {
+ 
+    class Form1 extends FormTrait {
+      val f1 = 5
+    }
+    val form1 = new Form1
+ 
+    println(form1.fields)
+ 
+    val form2 = new FormTrait {
+      val g1 = new Form1
+    }
+ 
+    form2.g1 // comment this line in order to make the test pass
+    ()
+  }
+
+  f()  
+}