Squashing the signature bugs.

Review by extempore.

3 files changed, 74 insertions, 76 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
index b8aa8d7a8f..2305e22ed2 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
@@ -549,7 +549,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
 
     // @M don't generate java generics sigs for (members of) implementation
     // classes, as they are monomorphic (TODO: ok?)
-    private def noGenericSignature(sym: Symbol) = (
+    private def needsGenericSignature(sym: Symbol) = !(
       // PP: This condition used to include sym.hasExpandedName, but this leads
       // to the total loss of generic information if a private member is
       // accessed from a closure: both the field and the accessor were generated
@@ -558,11 +558,11 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
       // unrelated change.
          sym.isSynthetic
       || sym.isLiftedMethod
+      || sym.isBridge
       || (sym.ownerChain exists (_.isImplClass))
     )
     def addGenericSignature(jmember: JMember, sym: Symbol, owner: Symbol) {
-      if (noGenericSignature(sym)) ()
-      else {
+      if (needsGenericSignature(sym)) {
         val memberTpe = atPhase(currentRun.erasurePhase)(owner.thisType.memberInfo(sym))
         // println("addGenericSignature sym: " + sym.fullName + " : " + memberTpe + " sym.info: " + sym.info)
         // println("addGenericSignature: "+ (sym.ownerChain map (x => (x.name, x.isImplClass))))
@@ -572,34 +572,37 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
            *  in which case we treat every signature as valid.  Medium term we
            *  should certainly write independent signature validation.
            */
-          if (SigParser.isParserAvailable && isValidSignature(sym, sig)) {
+          if (SigParser.isParserAvailable && !isValidSignature(sym, sig)) {
+            clasz.cunit.warning(sym.pos,
+                """|compiler bug: created invalid generic signature for %s in %s
+                   |signature: %s
+                   |if this is reproducible, please report bug at http://lampsvn.epfl.ch/trac/scala
+                """.trim.stripMargin.format(sym, sym.owner.skipPackageObject.fullName, sig))
+            return
+          }
+          if ((settings.check.value contains "genjvm")) {
             val normalizedTpe = atPhase(currentRun.erasurePhase)(erasure.prepareSigMap(memberTpe))
             val bytecodeTpe = owner.thisType.memberInfo(sym)
-            if (sym.isType || (erasure.erasure(normalizedTpe) =:= bytecodeTpe)) {
-              val index = jmember.getConstantPool.addUtf8(sig).toShort
-              if (opt.verboseDebug)
-                atPhase(currentRun.erasurePhase) {
-                  println("add generic sig "+sym+":"+sym.info+" ==> "+sig+" @ "+index)
-                }
-              val buf = ByteBuffer.allocate(2)
-              buf putShort index
-              addAttribute(jmember, tpnme.SignatureATTR, buf)
-            } else {
-              if (sym.hasFlag(Flags.MIXEDIN))
-                ()// println("suppressing signature for mixedin "+sym)
-              else
-                clasz.cunit.warning(sym.pos,
-              """|compiler bug: created generic signature for %s in %s that does not conform to its erasure
-                 |signature: %s
-                 |erasure type: %s
-                 |if this is reproducible, please report bug at http://lampsvn.epfl.ch/trac/scala
-              """.trim.stripMargin.format(sym, sym.owner.skipPackageObject.fullName, sig, bytecodeTpe))
+            if (!sym.isType && !sym.isConstructor && !(erasure.erasure(normalizedTpe) =:= bytecodeTpe)) {
+              clasz.cunit.warning(sym.pos,
+                  """|compiler bug: created generic signature for %s in %s that does not conform to its erasure
+                     |signature: %s
+                     |original type: %s
+                     |normalized type: %s
+                     |erasure type: %s
+                     |if this is reproducible, please report bug at http://lampsvn.epfl.ch/trac/scala
+                  """.trim.stripMargin.format(sym, sym.owner.skipPackageObject.fullName, sig, memberTpe, normalizedTpe, bytecodeTpe))
+               return
+            }
+          }
+          val index = jmember.getConstantPool.addUtf8(sig).toShort
+          if (opt.verboseDebug)
+            atPhase(currentRun.erasurePhase) {
+              println("add generic sig "+sym+":"+sym.info+" ==> "+sig+" @ "+index)
             }
-          } else clasz.cunit.warning(sym.pos,
-            """|compiler bug: created invalid generic signature for %s in %s
-               |signature: %s
-               |if this is reproducible, please report bug at http://lampsvn.epfl.ch/trac/scala
-            """.trim.stripMargin.format(sym, sym.owner.skipPackageObject.fullName, sig))
+          val buf = ByteBuffer.allocate(2)
+          buf putShort index
+          addAttribute(jmember, tpnme.SignatureATTR, buf)
         }
       }
     }
@@ -1858,7 +1861,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
       if (sym.isInterface) ACC_INTERFACE else 0,
       if (sym.isFinal && !sym.enclClass.isInterface && !sym.isClassConstructor) ACC_FINAL else 0,
       if (sym.isStaticMember) ACC_STATIC else 0,
-      if (sym.isBridge) ACC_BRIDGE else 0,
+      if (sym.isBridge || sym.hasFlag(Flags.MIXEDIN) && sym.isMethod) ACC_BRIDGE else 0,
       if (sym.isClass && !sym.isInterface) ACC_SUPER else 0,
       if (sym.isVarargsMethod) ACC_VARARGS else 0
     )
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index bdc0a1bfac..cef22dbbfd 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -231,12 +231,21 @@ abstract class Erasure extends AddInterfaces
   // for debugging signatures: traces logic given system property
   private val traceSig = util.Tracer(sys.props contains "scalac.sigs.trace")
 
+  /** 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
+   *  is the same as the erased type that's generated. Normalization means
+   *  unboxing some primitive types and further simplifications as they are done in jsig.
+   */
   val prepareSigMap = new TypeMap {
     def squashBoxed(tp: Type): Type = tp.normalize match {
       case t @ RefinedType(parents, decls) =>
         val parents1 = parents mapConserve squashBoxed
         if (parents1 eq parents) tp
         else RefinedType(parents1, decls)
+      case t @ ExistentialType(tparams, tpe) =>
+        val tpe1 = squashBoxed(tpe)
+        if (tpe1 eq tpe) t
+        else ExistentialType(tparams, tpe1)
       case t =>
         if (boxedClass contains t.typeSymbol) ObjectClass.tpe
         else tp
@@ -279,6 +288,10 @@ abstract class Erasure extends AddInterfaces
         val parents1 = parents mapConserve apply
         if (parents1 eq parents) tp1
         else RefinedType(parents1, decls)
+      case t @ ExistentialType(tparams, tpe) =>
+        val tpe1 = apply(tpe)
+        if (tpe1 eq tpe) t
+        else ExistentialType(tparams, tpe1)
       case tp1: ClassInfoType =>
         tp1
       case tp1 =>
@@ -290,56 +303,37 @@ abstract class Erasure extends AddInterfaces
    *  type for constructors.
    */
   def javaSig(sym0: Symbol, info: Type): Option[String] = atPhase(currentRun.erasurePhase) {
-    def jsig(tp: Type): String = jsig2(toplevel = false, Nil, tp)
-
-    // Unit in return position is 'V', but that cannot appear elsewhere.
-    def unboxedSig(tpe: Type, isReturnPosition: Boolean) = {
-      val tsym = tpe.typeSymbol
-      if (isReturnPosition && (tsym == UnitClass || sym0.isConstructor)) VOID_TAG
-      else if (isNonUnitValueClass(tsym)) abbrvTag(tsym)
-      else jsig(tpe)
-    }
-    def boxedSig(tp: Type) = jsig(squashBoxed(tp))
-    def squashBoxed(tp: Type) =
-      if (boxedClass contains tp.typeSymbol) ObjectClass.tpe
-      else tp
+    def boxedSig(tp: Type) = jsig(tp, primitiveOK = false)
 
-    def hiBounds(bounds: TypeBounds): List[Type] = (bounds.hi.normalize match {
+    def hiBounds(bounds: TypeBounds): List[Type] = bounds.hi.normalize match {
       case RefinedType(parents, _) => parents map normalize
       case tp                      => tp :: Nil
-    }) map squashBoxed
+    }
 
-    def jsig2(toplevel: Boolean, existentiallyBound: List[Symbol], tp0: Type): String = {
+    def jsig(tp0: Type, existentiallyBound: List[Symbol] = Nil, toplevel: Boolean = false, primitiveOK: Boolean = true): String = {
       val tp = tp0.dealias
       tp match {
         case st: SubType =>
-          jsig2(toplevel, existentiallyBound, st.supertype)
+          jsig(st.supertype, existentiallyBound, toplevel, primitiveOK)
         case ExistentialType(tparams, tpe) =>
-          jsig2(toplevel, tparams, tpe)
+          jsig(tpe, tparams, toplevel, primitiveOK)
         case TypeRef(pre, sym, args) =>
           def argSig(tp: Type) =
             if (existentiallyBound contains tp.typeSymbol) {
               val bounds = tp.typeSymbol.info.bounds
-              if (AnyRefClass.tpe <:< bounds.hi) {
-                if (bounds.lo <:< NullClass.tpe) "*"
-                else "-" + boxedSig(bounds.lo)
-              }
-              else "+" + boxedSig(bounds.hi)
-            }
-            else if (tp.typeSymbol == UnitClass) {
-              jsig(ObjectClass.tpe)
+              if (!(AnyRefClass.tpe <:< bounds.hi)) "+" + boxedSig(bounds.hi)
+              else if (!(bounds.lo <:< NullClass.tpe)) "-" + boxedSig(bounds.lo)
+              else "*"
             } else {
               boxedSig(tp)
             }
-          def classSig = (
+          def classSig: String =
             "L"+atPhase(currentRun.icodePhase)(sym.fullName + global.genJVM.moduleSuffix(sym)).replace('.', '/')
-          )
-          def classSigSuffix = "." + sym.name
-
-          // If args isEmpty, Array is being used as a higher-kinded type
-          if (sym == ArrayClass && args.nonEmpty) {
+          def classSigSuffix: String =
+            "."+sym.name
+          if (sym == ArrayClass) {
             if (unboundedGenericArrayLevel(tp) == 1) jsig(ObjectClass.tpe)
-            else ARRAY_TAG.toString+(args map jsig).mkString
+            else ARRAY_TAG.toString+(args map (jsig(_))).mkString
           }
           else if (isTypeParameterInSig(sym, sym0)) {
             assert(!sym.isAliasType, "Unexpected alias type: " + sym)
@@ -353,13 +347,18 @@ abstract class Erasure extends AddInterfaces
             jsig(RuntimeNothingClass.tpe)
           else if (sym == NullClass)
             jsig(RuntimeNullClass.tpe)
+          else if (isValueClass(sym)) {
+            if (!primitiveOK) jsig(ObjectClass.tpe)
+            else if (sym == UnitClass) jsig(BoxedUnitClass.tpe)
+            else abbrvTag(sym).toString
+          }
           else if (sym.isClass) {
             val preRebound = pre.baseType(sym.owner) // #2585
             traceSig.seq("sym.isClass", Seq(sym.ownerChain, preRebound, sym0.enclClassChain)) {
               dotCleanup(
                 (
                   if (needsJavaSig(preRebound)) {
-                    val s = jsig(preRebound)
+                    val s = jsig(preRebound, existentiallyBound)
                     if (s.charAt(0) == 'L') s.substring(0, s.length - 1) + classSigSuffix
                     else classSig
                   }
@@ -373,7 +372,7 @@ abstract class Erasure extends AddInterfaces
               )
             }
           }
-          else jsig(erasure(tp))
+          else jsig(erasure(tp), existentiallyBound, toplevel, primitiveOK)
         case PolyType(tparams, restpe) =>
           assert(tparams.nonEmpty)
           def boundSig(bounds: List[Type]) = {
@@ -390,21 +389,17 @@ abstract class Erasure extends AddInterfaces
           val paramString = if (toplevel) tparams map paramSig mkString ("<", "", ">") else ""
           traceSig.seq("PolyType", Seq(tparams, restpe))(paramString + jsig(restpe))
         case MethodType(params, restpe) =>
-          traceSig.seq("MethodType", Seq(params, restpe)) {
-            "(%s)%s".format(
-              params map (p => unboxedSig(p.tpe, false)) mkString,
-              unboxedSig(restpe, true)
-            )
-          }
+          "("+(params map (_.tpe) map (jsig(_))).mkString+")"+
+          (if (restpe.typeSymbol == UnitClass || sym0.isConstructor) VOID_TAG.toString else jsig(restpe))
         case RefinedType(parent :: _, decls) =>
-          jsig(parent)
+          boxedSig(parent)
         case ClassInfoType(parents, _, _) =>
-          (parents map jsig).mkString
+          (parents map (boxedSig(_))).mkString
         case AnnotatedType(_, atp, _) =>
-          jsig(atp)
+          jsig(atp, existentiallyBound, toplevel, primitiveOK)
         case BoundedWildcardType(bounds) =>
           println("something's wrong: "+sym0+":"+sym0.tpe+" has a bounded wildcard type")
-          jsig(bounds.hi)
+          jsig(bounds.hi, existentiallyBound, toplevel, primitiveOK)
         case _ =>
           val etp = erasure(tp)
           if (etp eq tp) throw new UnknownSig
@@ -413,7 +408,7 @@ abstract class Erasure extends AddInterfaces
     }
     traceSig.seq("javaSig", Seq(sym0, info)) {
       if (needsJavaSig(info)) {
-        try Some(jsig2(true, Nil, info))
+        try Some(jsig(info, toplevel = true))
         catch { case ex: UnknownSig => None }
       }
       else None
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index caba8f9bd1..49f5907245 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -134,7 +134,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
   def addMember(clazz: Symbol, member: Symbol): Symbol = {
     if (settings.debug.value) log("new member of " + clazz + ":" + member.defString)
     clazz.info.decls enter member
-    member setFlag MIXEDIN
+    member.setFlag(MIXEDIN)
   }
 
   def needsExpandedSetterName(field: Symbol) = !field.isLazy && (