closes #2585: generate more precise Java generi...

closes #2585: generate more precise Java generic signatures for classes
nested in parametric outer classes fix based on review by Martin
baseType is your friend

5 files changed, 60 insertions, 9 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
index d420524fd6..c4408c661a 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
@@ -434,9 +434,11 @@ abstract class GenJVM extends SubComponent {
 
     def addGenericSignature(jmember: JMember, sym: Symbol, owner: Symbol) {
       if (!sym.hasFlag(Flags.EXPANDEDNAME | Flags.SYNTHETIC)
-          && !(sym.isMethod && sym.hasFlag(Flags.LIFTED))) {
+          && !(sym.isMethod && sym.hasFlag(Flags.LIFTED))
+          && !(sym.ownerChain exists (_.isImplClass))) {  // @M don't generate java generics sigs for (members of) implementation classes, as they are monomorphic (TODO: ok?)
         val memberTpe = atPhase(currentRun.erasurePhase)(owner.thisType.memberInfo(sym))
-//        println("sym: " + sym.fullNameString + " : " + memberTpe + " sym.info: " + sym.info)
+        // println("addGenericSignature sym: " + sym.fullNameString + " : " + memberTpe + " sym.info: " + sym.info)
+        // println("addGenericSignature: "+ (sym.ownerChain map (x => (x.name, x.isImplClass))))
         erasure.javaSig(sym, memberTpe) match {
           case Some(sig) =>
             val index = jmember.getConstantPool().addUtf8(sig).toShort
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index 7c57b2e16f..290abd0995 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -75,6 +75,13 @@ abstract class Erasure extends AddInterfaces with typechecker.Analyzer with ast.
     case _ => 0
   }
 
+  // @M #2585 when generating a java generic signature that includes a selection of an inner class p.I,  (p = `pre`, I = `cls`)
+  // must rewrite to p'.I, where p' refers to the class that directly defines the nested class I
+  // see also #2585 marker in javaSig: there, type arguments must be included (use pre.baseType(cls.owner))
+  // requires cls.isClass
+  @inline private def rebindInnerClass(pre: Type, cls: Symbol): Type =
+    if(cls.owner.isClass) cls.owner.tpe else pre // why not cls.isNestedClass?
+
   /** <p>
    *    The erasure <code>|T|</code> of a type <code>T</code>. This is:
    *  </p>
@@ -90,7 +97,7 @@ abstract class Erasure extends AddInterfaces with typechecker.Analyzer with ast.
    *   - For a typeref scala.Array+[T] where T is not an abstract type, scala.Array+[|T|].
    *   - For a typeref scala.Any or scala.AnyVal, java.lang.Object.
    *   - For a typeref scala.Unit, scala.runtime.BoxedUnit.
-   *   - For a typeref P.C[Ts] where C refers to a class, |P|.C.
+   *   - For a typeref P.C[Ts] where C refers to a class, |P|.C. (Where P is first rebound to the class that directly defines C.)
    *   - For a typeref P.C[Ts] where C refers to an alias type, the erasure of C's alias.
    *   - For a typeref P.C[Ts] where C refers to an abstract type, the
    *     erasure of C's upper bound.
@@ -122,9 +129,8 @@ abstract class Erasure extends AddInterfaces with typechecker.Analyzer with ast.
             else typeRef(apply(pre), sym, args map this)
           else if (sym == AnyClass || sym == AnyValClass || sym == SingletonClass) erasedTypeRef(ObjectClass)
           else if (sym == UnitClass) erasedTypeRef(BoxedUnitClass)
-          else if (sym.isClass)
-            typeRef(apply(if (sym.owner.isClass) sym.owner.tpe else pre), sym, List())
-          else apply(sym.info)
+          else if (sym.isClass) typeRef(apply(rebindInnerClass(pre, sym)), sym, List())  // #2585
+          else apply(sym.info) // alias type or abstract type
         case PolyType(tparams, restpe) =>
           apply(restpe)
         case ExistentialType(tparams, restpe) =>
@@ -164,6 +170,7 @@ abstract class Erasure extends AddInterfaces with typechecker.Analyzer with ast.
           case TypeRef(pre, sym, args) =>
             if (sym == ArrayClass) args foreach traverse
             else if (sym.isTypeParameterOrSkolem || sym.isExistential || !args.isEmpty) result = true
+            else if (sym.isClass) traverse(rebindInnerClass(pre, sym)) // #2585
             else if (!sym.owner.isPackageClass) traverse(pre)
           case PolyType(_, _) | ExistentialType(_, _) =>
             result = true
@@ -243,8 +250,9 @@ abstract class Erasure extends AddInterfaces with typechecker.Analyzer with ast.
             tagOfClass(sym).toString
           else if (sym.isClass)
             {
-              if (needsJavaSig(pre)) {
-                val s = jsig(pre)
+              val preRebound = pre.baseType(sym.owner) // #2585
+              if (needsJavaSig(preRebound)) {
+                val s = jsig(preRebound)
                 if (s.charAt(0) == 'L') s.substring(0, s.length - 1) + classSigSuffix
                 else classSig
               } else classSig
@@ -914,7 +922,7 @@ abstract class Erasure extends AddInterfaces with typechecker.Analyzer with ast.
                           List()),
                     isArrayTest(qual1()))
                 }
-              }
+            }
           case TypeApply(fun, args) if (fun.symbol.owner != AnyClass &&
                                         fun.symbol != Object_asInstanceOf &&
                                         fun.symbol != Object_isInstanceOf) =>
diff --git a/test/files/jvm/t2585.check b/test/files/jvm/t2585.check
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/files/jvm/t2585.check
diff --git a/test/files/jvm/t2585/Test.java b/test/files/jvm/t2585/Test.java
new file mode 100644
index 0000000000..51fe20d81e
--- /dev/null
+++ b/test/files/jvm/t2585/Test.java
@@ -0,0 +1,16 @@
+class J { S s ; }
+
+public class Test {
+  public static void main(String[] args) {
+    final X x = new X();
+    final OuterImpl o = new OuterImpl(x);
+
+    final OuterImpl.Inner i1 = o.newInner();
+    i1.getT().getI().getT().getI();  // <--- Error: "The method getI() is undefined for the type Object"
+
+    final Outer<X>.Inner i2 = o.newInner();
+    i2.getT().getI().getT().getI(); // <--- Error: "The method getI() is undefined for the type Object"
+
+    HashMap<String, String> map = new HashMap<String, String>();
+  }
+}
\ No newline at end of file
diff --git a/test/files/jvm/t2585/genericouter.scala b/test/files/jvm/t2585/genericouter.scala
new file mode 100644
index 0000000000..e06aa8101e
--- /dev/null
+++ b/test/files/jvm/t2585/genericouter.scala
@@ -0,0 +1,25 @@
+case class S(n:Int)
+
+trait TraversableLike[+A, +Repr] {
+  class WithFilter(p: A => Boolean)
+  def withFilter(p: A => Boolean): WithFilter = new WithFilter(p)
+}
+
+class HashMap[K, +V] extends TraversableLike[(K, V), HashMap[K, V]]
+
+class Outer[T](val t: T) {
+  class Inner {
+    def getT : T = t
+  }
+}
+
+class OuterImpl(x: X) extends Outer[X](x) {
+  def newInner = new Inner
+}
+
+class X {
+  def getI : Outer[X]#Inner = {
+    val oImpl = new OuterImpl(this)
+    new oImpl.Inner
+  }
+}
\ No newline at end of file