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.
scala · Mar 11, 2014 · 7b72f95 · 7b72f95
1 parent 5720e97
commit 7b72f95
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 4 deletions.
diff --git 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
@@ -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
@@ -0,0 +1,3 @@
+Scope{
+  final private val f1: Int
+}
diff --git a/test/files/run/t8196.scala 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()  
+}