diff options
-rwxr-xr-x | compare-signatures | 13 | ||||
-rw-r--r-- | src/compiler/scala/reflect/internal/Types.scala | 12 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala | 17 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/Erasure.scala | 87 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Typers.scala | 21 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/util/Tracer.scala | 12 | ||||
-rw-r--r-- | test/files/run/bug4891.check | 9 | ||||
-rw-r--r-- | test/files/run/bug4891/J_2.java | 13 | ||||
-rw-r--r-- | test/files/run/bug4891/S_1.scala | 26 | ||||
-rw-r--r-- | test/files/run/bug4891/S_3.scala | 5 |
10 files changed, 165 insertions, 50 deletions
diff --git a/compare-signatures b/compare-signatures new file mode 100755 index 0000000000..17e9ace476 --- /dev/null +++ b/compare-signatures @@ -0,0 +1,13 @@ +#!/usr/bin/env bash +# + +file=${1:-test/files/run/bug4891/S_1.scala} +# src/library/scala/collection/immutable/MapLike.scala + +rm -rf /tmp/o1 /tmp/o2 +mkdir /tmp/o1 /tmp/o2 + +scalac3 -d /tmp/o1 $file +pscalac -d /tmp/o2 $file + +jpdiff /tmp/o1 /tmp/o2 diff --git a/src/compiler/scala/reflect/internal/Types.scala b/src/compiler/scala/reflect/internal/Types.scala index 4a2d8e9f08..7bcc0f8913 100644 --- a/src/compiler/scala/reflect/internal/Types.scala +++ b/src/compiler/scala/reflect/internal/Types.scala @@ -3070,6 +3070,18 @@ A type's typeSymbol should never be inspected directly. } } + /** Substitutes the empty scope for any non-empty decls in the type. */ + object dropAllRefinements extends TypeMap { + def apply(tp: Type): Type = tp match { + case rt @ RefinedType(parents, decls) if !decls.isEmpty => + mapOver(copyRefinedType(rt, parents, EmptyScope)) + case ClassInfoType(parents, decls, clazz) if !decls.isEmpty => + mapOver(ClassInfoType(parents, EmptyScope, clazz)) + case _ => + mapOver(tp) + } + } + // Set to true for A* => Seq[A] // (And it will only rewrite A* in method result types.) // This is the pre-existing behavior. diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala index be98b314a4..c0a9192c86 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala @@ -11,7 +11,6 @@ import java.io.{ DataOutputStream, OutputStream } import java.nio.ByteBuffer import scala.collection.{ mutable, immutable } import scala.reflect.internal.pickling.{ PickleFormat, PickleBuffer } -import scala.tools.reflect.SigParser import scala.tools.nsc.util.ScalaClassLoader import scala.tools.nsc.symtab._ import scala.reflect.internal.ClassfileConstants._ @@ -603,14 +602,6 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with nannots } - /** Run the signature parser to catch bogus signatures. - */ - def isValidSignature(sym: Symbol, sig: String) = ( - if (sym.isMethod) SigParser verifyMethod sig - else if (sym.isTerm) SigParser verifyType sig - else SigParser verifyClass sig - ) - // @M don't generate java generics sigs for (members of) implementation // classes, as they are monomorphic (TODO: ok?) private def needsGenericSignature(sym: Symbol) = !( @@ -632,12 +623,8 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with // println("addGenericSignature: "+ (sym.ownerChain map (x => (x.name, x.isImplClass)))) erasure.javaSig(sym, memberTpe) foreach { sig => debuglog("sig(" + jmember.getName + ", " + sym + ", " + owner + ") " + sig) - /** 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. - */ - if (settings.Xverify.value && SigParser.isParserAvailable && !isValidSignature(sym, sig)) { + + if (settings.Xverify.value && !erasure.isValidSignature(sym, sig)) { clasz.cunit.warning(sym.pos, """|compiler bug: created invalid generic signature for %s in %s |signature: %s 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 diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 149763cf44..8dd6855464 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -186,6 +186,16 @@ trait Typers extends Modes with Adaptations { var context = context0 def context1 = context + def dropExistential(tp: Type): Type = tp match { + case ExistentialType(tparams, tpe) => + new SubstWildcardMap(tparams).apply(tp) + case TypeRef(_, sym, _) if sym.isAliasType => + val tp0 = tp.normalize + val tp1 = dropExistential(tp0) + if (tp1 eq tp0) tp else tp1 + case _ => tp + } + /** Check that <code>tree</code> is a stable expression. * * @param tree ... @@ -4263,17 +4273,6 @@ trait Typers extends Modes with Adaptations { def typed(tree: Tree, mode: Int, pt: Type): Tree = { indentTyping() - def dropExistential(tp: Type): Type = tp match { - case ExistentialType(tparams, tpe) => - debuglog("Dropping existential: " + tree + " " + tp) - new SubstWildcardMap(tparams).apply(tp) - case TypeRef(_, sym, _) if sym.isAliasType => - val tp0 = tp.normalize - val tp1 = dropExistential(tp0) - if (tp1 eq tp0) tp else tp1 - case _ => tp - } - var alreadyTyped = false try { if (Statistics.enabled) { diff --git a/src/compiler/scala/tools/nsc/util/Tracer.scala b/src/compiler/scala/tools/nsc/util/Tracer.scala index ec1eaa13ea..acbf60da5b 100644 --- a/src/compiler/scala/tools/nsc/util/Tracer.scala +++ b/src/compiler/scala/tools/nsc/util/Tracer.scala @@ -11,8 +11,6 @@ import scala.runtime.ScalaRunTime class Tracer(enabled: () => Boolean) { def out: PrintStream = System.out - def intoString(x: Any): String = "" + x - def stringify(x: Any) = ScalaRunTime stringOf x // So can pass tuples, lists, whatever as arguments and don't @@ -52,14 +50,18 @@ class Tracer(enabled: () => Boolean) { if (enabled()) { passign(name, stringifyArgs(args)) + val resultToPrint = result match { + case Some(x) => x + case _ => result + } // concise output optimization - val isOneliner = result match { + val isOneliner = resultToPrint match { case _: Boolean | _: None.type => true case s: String => s.length < 40 case _ => false } - if (isOneliner) p(stringify(result) + "\n") - else pblock(result) + if (isOneliner) p(stringify(resultToPrint) + "\n") + else pblock(resultToPrint) } result diff --git a/test/files/run/bug4891.check b/test/files/run/bug4891.check new file mode 100644 index 0000000000..2e84168ed4 --- /dev/null +++ b/test/files/run/bug4891.check @@ -0,0 +1,9 @@ +test.generic.T1 + (m) public abstract A test.generic.T1.t1(A) +test.generic.C1 + (m) public void test.generic.C1.m1() +test.generic.C2 + (m) public void test.generic.C1.m1() + (m) public A test.generic.C2.t1(A) (bridge) +null +interface scala.ScalaObject diff --git a/test/files/run/bug4891/J_2.java b/test/files/run/bug4891/J_2.java new file mode 100644 index 0000000000..db1cc52b13 --- /dev/null +++ b/test/files/run/bug4891/J_2.java @@ -0,0 +1,13 @@ +import test.generic.*; + +public class J_2 { + public static <A> void foo(T1<A> x) { + // x.m1(); + } + + public static void main(String[] args) { + Bug4891.main(null); + T1<Object> x = new C2<Object>(); + foo(x); + } +} diff --git a/test/files/run/bug4891/S_1.scala b/test/files/run/bug4891/S_1.scala new file mode 100644 index 0000000000..927c3ad93d --- /dev/null +++ b/test/files/run/bug4891/S_1.scala @@ -0,0 +1,26 @@ +package test.generic { + class C1[A] { + def m1(): Unit = () + } + + trait T1[A] extends C1[A] { + def t1(x: A) = x + } + + class C2[A] extends T1[A] +} + +import scala.tools.partest._ + +object Bug4891 extends SigTest { + import test.generic._ + + def main(args: Array[String]): Unit = { + show[T1[_]]() + show[C1[_]]() + show[C2[_]]() + + println(classOf[T1[_]].getGenericSuperclass) + classOf[T1[_]].getGenericInterfaces foreach println + } +} diff --git a/test/files/run/bug4891/S_3.scala b/test/files/run/bug4891/S_3.scala new file mode 100644 index 0000000000..0da4912bc5 --- /dev/null +++ b/test/files/run/bug4891/S_3.scala @@ -0,0 +1,5 @@ +object Test { + def main(args: Array[String]): Unit = { + J_2 main null + } +} |