Fix java signature generation for traits: no cl...

Fix java signature generation for traits: no classes as parents. Closes
SI-4891, review by grek.

1 files changed, 68 insertions, 19 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index fc3aae9046..dbceb1f721 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -6,6 +6,7 @@
 package scala.tools.nsc
 package transform
 
+import scala.tools.reflect.SigParser
 import scala.reflect.internal.ClassfileConstants._
 import scala.collection.{ mutable, immutable }
 import symtab._
@@ -101,6 +102,12 @@ abstract class Erasure extends AddInterfaces
   // for debugging signatures: traces logic given system property
   // performance: get the value here
   val traceSignatures = (sys.BooleanProp keyExists "scalac.sigs.trace").value
+  private object traceSig extends util.Tracer(() => traceSignatures) {
+    override def stringify(x: Any) = x match {
+      case tp: Type   => super.stringify(dropAllRefinements(tp))
+      case _          => super.stringify(x)
+    }
+  }
 
   override protected def verifyJavaErasure = settings.Xverify.value || settings.debug.value
   private def needsJavaSig(tp: Type) = !settings.Ynogenericsig.value && NeedsSigCollector.collect(tp)
@@ -130,7 +137,6 @@ abstract class Erasure extends AddInterfaces
       case ch                 => last = ch ; ch
     }
   }
-  private val traceSig  = util.Tracer(traceSignatures)
 
   /** This object is only used for sanity testing when -check:genjvm is set.
    *  In that case we make sure that the erasure of the `normalized` type
@@ -200,16 +206,60 @@ abstract class Erasure extends AddInterfaces
     }
   }
 
+  /** Run the signature parser to catch bogus signatures.
+   */
+  def isValidSignature(sym: Symbol, sig: String) = (
+    /** Since we're using a sun internal class for signature validation,
+     *  we have to allow for it not existing or otherwise malfunctioning:
+     *  in which case we treat every signature as valid.  Medium term we
+     *  should certainly write independent signature validation.
+     */
+    SigParser.isParserAvailable && (
+      if (sym.isMethod) SigParser verifyMethod sig
+      else if (sym.isTerm) SigParser verifyType sig
+      else SigParser verifyClass sig
+    )
+  )
+
+  private def hiBounds(bounds: TypeBounds): List[Type] = bounds.hi.normalize match {
+    case RefinedType(parents, _) => parents map (_.normalize)
+    case tp                      => tp :: Nil
+  }
+
   /** The Java signature of type 'info', for symbol sym. The symbol is used to give the right return
    *  type for constructors.
    */
   def javaSig(sym0: Symbol, info: Type): Option[String] = atPhase(currentRun.erasurePhase) {
+    val isTraitSignature = sym0.enclClass.isTrait
+
+    def superSig(parents: List[Type]) = traceSig("superSig", parents) {
+      val ps = (
+        if (isTraitSignature) {
+          // java is unthrilled about seeing interfaces inherit from classes
+          val ok = parents filter (p => p.typeSymbol.isTrait || p.typeSymbol.isInterface)
+          // traits should always list Object.
+          if (ok.isEmpty || ok.head.typeSymbol != ObjectClass) ObjectClass.tpe :: ok
+          else ok
+        }
+        else parents
+      )
+      ps map boxedSig mkString
+    }
     def boxedSig(tp: Type) = jsig(tp, primitiveOK = false)
-
-    def hiBounds(bounds: TypeBounds): List[Type] = bounds.hi.normalize match {
-      case RefinedType(parents, _) => parents map normalize
-      case tp                      => tp :: Nil
+    def boundsSig(bounds: List[Type]) = traceSig("boundsSig", bounds) {
+      val (isTrait, isClass) = bounds partition (_.typeSymbol.isTrait)
+      val classPart = isClass match {
+        case Nil    => ":" // + boxedSig(ObjectClass.tpe)
+        case x :: _ => ":" + boxedSig(x)
+      }
+      classPart :: (isTrait map boxedSig) mkString ":"
     }
+    def paramSig(tsym: Symbol) = tsym.name + boundsSig(hiBounds(tsym.info.bounds))
+    def polyParamSig(tparams: List[Symbol]) = traceSig("polyParamSig", tparams) (
+      if (tparams.isEmpty) ""
+      else tparams map paramSig mkString ("<", "", ">")
+    )
+
     // Anything which could conceivably be a module (i.e. isn't known to be
     // a type parameter or similar) must go through here or the signature is
     // likely to end up with Foo<T>.Empty where it needs Foo<T>.Empty$.
@@ -276,19 +326,9 @@ abstract class Erasure extends AddInterfaces
           else jsig(erasure(sym0, tp), existentiallyBound, toplevel, primitiveOK)
         case PolyType(tparams, restpe) =>
           assert(tparams.nonEmpty)
-          def boundSig(bounds: List[Type]) = {
-            val (isTrait, isClass) = bounds partition (_.typeSymbol.isTrait)
+          val poly = if (toplevel) polyParamSig(tparams) else ""
+          poly + jsig(restpe)
 
-            ":" + (
-              if (isClass.isEmpty) "" else boxedSig(isClass.head)
-            ) + (
-              isTrait map (x => ":" + boxedSig(x)) mkString
-            )
-          }
-          def paramSig(tsym: Symbol) = tsym.name + boundSig(hiBounds(tsym.info.bounds))
-
-          val paramString = if (toplevel) tparams map paramSig mkString ("<", "", ">") else ""
-          paramString + jsig(restpe)
         case MethodType(params, restpe) =>
           "("+(params map (_.tpe) map (jsig(_))).mkString+")"+
           (if (restpe.typeSymbol == UnitClass || sym0.isConstructor) VOID_TAG.toString else jsig(restpe))
@@ -296,7 +336,7 @@ abstract class Erasure extends AddInterfaces
         case RefinedType(parent :: _, decls) =>
           boxedSig(parent)
         case ClassInfoType(parents, _, _) =>
-          (parents map (boxedSig(_))).mkString
+          superSig(parents)
         case AnnotatedType(_, atp, _) =>
           jsig(atp, existentiallyBound, toplevel, primitiveOK)
         case BoundedWildcardType(bounds) =>
@@ -308,13 +348,22 @@ abstract class Erasure extends AddInterfaces
           else jsig(etp)
       }
     }
-    traceSig("javaSig", (sym0, info)) {
+    val result = traceSig("javaSig", (sym0, info)) {
       if (needsJavaSig(info)) {
         try Some(jsig(info, toplevel = true))
         catch { case ex: UnknownSig => None }
       }
       else None
     }
+    // Debugging: immediately verify signatures when tracing.
+    if (traceSignatures) {
+      result foreach { sig =>
+        if (!isValidSignature(sym0, sig))
+          println("**** invalid signature for " + sym0 + ": " + sig)
+      }
+    }
+
+    result
   }
 
   class UnknownSig extends Exception