smarter bridges to unapplies

wraps the call to a bridged synthetic unapply(Seq) in a defensive if-test:

if (x.isInstanceOf[expectedType]) real.unapply(x.asInstanceOf[expectedType]) else None

NOTE: the test is WRONG, but it has to be due to #1697/#2337 -- once those are fixed, this one should generate the expected output

git-svn-id: http://lampsvn.epfl.ch/svn-repos/scala/scala/trunk@25966 5e8d7ff9-d8ef-0310-90f0-a4852d11357a

src/compiler/scala/tools/nsc/transform/Erasure.scala

@@ -755,7 +755,7 @@ abstract class Erasure extends AddInterfaces
       while (opc.hasNext) {
         val member = opc.overriding
         val other = opc.overridden
-        //Console.println("bridge? " + member + ":" + member.tpe + member.locationString + " to " + other + ":" + other.tpe + other.locationString)//DEBUG
+        //println("bridge? " + member + ":" + member.tpe + member.locationString + " to " + other + ":" + other.tpe + other.locationString)//DEBUG
         if (atPhase(currentRun.explicitouterPhase)(!member.isDeferred)) {
           val otpe = erasure(owner, other.tpe)
           val bridgeNeeded = atPhase(phase.next) (
@@ -790,8 +790,19 @@ abstract class Erasure extends AddInterfaces
                       member.tpe match {
                         case MethodType(List(), ConstantType(c)) => Literal(c)
                         case _ =>
-                          (((Select(This(owner), member): Tree) /: bridge.paramss)
+                          val bridgingCall = (((Select(This(owner), member): Tree) /: bridge.paramss)
                              ((fun, vparams) => Apply(fun, vparams map Ident)))
+                          // type checking ensures we can safely call `other`, but unless `member.tpe <:< other.tpe`, calling `member` is not guaranteed to succeed
+                          // in general, there's nothing we can do about this, except for an unapply: when this subtype test fails, return None without calling `member`
+                          if (  member.isSynthetic // TODO: should we do this for user-defined unapplies as well?
+                             && ((member.name == nme.unapply) || (member.name == nme.unapplySeq))
+                             // && (bridge.paramss.nonEmpty && bridge.paramss.head.nonEmpty && bridge.paramss.head.tail.isEmpty) // does the first argument list has exactly one argument -- for user-defined unapplies we can't be sure
+                             && !(atPhase(phase.next)(member.tpe <:< other.tpe))) { // no static guarantees (TODO: is the subtype test ever true?)
+                            import CODE._
+                            val typeTest = gen.mkIsInstanceOf(REF(bridge.paramss.head.head), member.tpe.params.head.tpe, any = true, wrapInApply = true) // any = true since we're before erasure (?), wrapInapply is true since we're after uncurry
+                            // println("unapp type test: "+ typeTest)
+                            IF (typeTest) THEN bridgingCall ELSE REF(NoneModule)
+                          } else bridgingCall
                       });
                   debuglog("generating bridge from " + other + "(" + Flags.flagsToString(bridge.flags)  + ")" + ":" + otpe + other.locationString + " to " + member + ":" + erasure(owner, member.tpe) + member.locationString + " =\n " + bridgeDef);
                   bridgeDef

test/files/run/patmat_unapp_abstype.check

@@ -0,0 +1,2 @@
+TypeRef
+none of the above

test/files/run/patmat_unapp_abstype.scala

@@ -0,0 +1,39 @@
+// abstract types and extractors, oh my!
+trait TypesAPI {
+  trait Type
+
+  // an alternative fix (implemented in the virtual pattern matcher, is to replace the isInstanceOf by a manifest-based run-time test)
+  // that's what typeRefMani is for
+  type TypeRef <: Type //; implicit def typeRefMani: Manifest[TypeRef]
+  val TypeRef: TypeRefExtractor; trait TypeRefExtractor {
+    def apply(x: Int): TypeRef
+    def unapply(x: TypeRef): Option[(Int)]
+  }
+
+  // just for illustration, should follow the same pattern as TypeRef
+  case class MethodType(n: Int) extends Type
+}
+
+// user should not be exposed to the implementation
+trait TypesUser extends TypesAPI {
+  def shouldNotCrash(tp: Type): Unit = {
+    tp match {
+      case TypeRef(x) => println("TypeRef") 
+      case MethodType(x) => println("MethodType")
+      case _ => println("none of the above")
+    }
+  }
+}
+
+trait TypesImpl extends TypesAPI {
+  object TypeRef extends TypeRefExtractor  // this will have a bridged unapply(x: Type) = unapply(x.asInstanceOf[TypeRef])
+  case class TypeRef(n: Int) extends Type // this has a bridge from TypesAPI#Type to TypesImpl#TypeRef 
+  // --> the cast in the bridge will fail because the pattern matcher can't type test against the abstract types in TypesUser
+  //lazy val typeRefMani = manifest[TypeRef]
+}
+
+object Test extends TypesImpl with TypesUser with App {
+  shouldNotCrash(TypeRef(10)) // should and does print "TypeRef"
+  // once  #1697/#2337 are fixed, this should generate the correct output
+  shouldNotCrash(MethodType(10)) // should print "MethodType" but prints "none of the above" -- good one, pattern matcher!
+}