Merge pull request scala-js#2139 from sjrd/no-refl-proxy-in-sjsir

Synthesize reflective proxies at link time.

Author: sjrd

cli/src/main/scala/org/scalajs/cli/Scalajsp.scala

@@ -31,7 +31,6 @@ object Scalajsp {
 
   private case class Options(
     infos: Boolean = false,
-    showReflProxy: Boolean = false,
     jar: Option[File] = None,
     fileNames: Seq[String] = Seq.empty)
 
@@ -49,9 +48,6 @@ object Scalajsp {
       opt[Unit]('i', "infos")
         .action { (_, c) => c.copy(infos = true) }
         .text("Show DCE infos instead of trees")
-      opt[Unit]('p', "reflProxies")
-        .action { (_, c) => c.copy(showReflProxy = true) }
-        .text("Show reflective call proxies")
       opt[Unit]('s', "supported")
         .action { (_,_) => printSupported(); sys.exit() }
         .text("Show supported Scala.js IR versions")
@@ -90,15 +86,8 @@ object Scalajsp {
       opts: Options): Unit = {
     if (opts.infos)
       new InfoPrinter(stdout).print(vfile.info)
-    else {
-      val (info, tree) = vfile.infoAndTree
-      val outTree = {
-        if (opts.showReflProxy) tree
-        else filterOutReflProxies(tree)
-      }
-
-      new IRTreePrinter(stdout).printTopLevelTree(outTree)
-    }
+    else
+      new IRTreePrinter(stdout).printTopLevelTree(vfile.tree)
 
     stdout.flush()
   }
@@ -141,15 +130,4 @@ object Scalajsp {
   private val stdout =
     new BufferedWriter(new OutputStreamWriter(Console.out, "UTF-8"))
 
-  // !!! CODE DUPLICATION with ScalaJSPluginInternal.filterOutReflProxies
-  private def filterOutReflProxies(tree: ClassDef): ClassDef = {
-    import ir.Trees._
-    import ir.Definitions.isReflProxyName
-    val newDefs = tree.defs.filter {
-      case MethodDef(_, Ident(name, _), _, _, _) => !isReflProxyName(name)
-      case _ => true
-    }
-    tree.copy(defs = newDefs)(tree.optimizerHints)(tree.pos)
-  }
-
 }

compiler/src/main/scala/org/scalajs/core/compiler/GenJSCode.scala

@@ -367,14 +367,9 @@ abstract class GenJSCode extends plugins.PluginComponent
         memberExports ++ exportedConstructorsOrAccessors
       }
 
-      // Generate the reflective call proxies (where required)
-      val reflProxies =
-        if (isHijacked) Nil
-        else genReflCallProxies(sym)
-
       // Hashed definitions of the class
       val hashedDefs =
-        Hashers.hashDefs(generatedMembers ++ exports ++ reflProxies)
+        Hashers.hashDefs(generatedMembers ++ exports)
 
       // The complete class definition
       val kind =
@@ -890,120 +885,6 @@ abstract class GenJSCode extends plugins.PluginComponent
           afterSuper)(body.pos)
     }
 
-    /**
-     * Generates reflective proxy methods for methods in sym
-     *
-     * Reflective calls don't depend on the return type, so it's hard to
-     * generate calls without using runtime reflection to list the methods. We
-     * generate a method to be used for reflective calls (without return
-     * type in the name).
-     *
-     * There are cases where non-trivial overloads cause ambiguous situations:
-     *
-     * {{{
-     * object A {
-     *   def foo(x: Option[Int]): String
-     *   def foo(x: Option[String]): Int
-     * }
-     * }}}
-     *
-     * This is completely legal code, but due to the same erased parameter
-     * type of the {{{foo}}} overloads, they cannot be disambiguated in a
-     * reflective call, as the exact return type is unknown at the call site.
-     *
-     * Cases like the upper currently fail on the JVM backend at runtime. The
-     * Scala.js backend uses the following rules for selection (which will
-     * also cause runtime failures):
-     *
-     * - If a proxy with the same signature (method name and parameters)
-     *   exists in the superclass, no proxy is generated (proxy is inherited)
-     * - If no proxy exists in the superclass, a proxy is generated for the
-     *   first method with matching signatures.
-     */
-    def genReflCallProxies(sym: Symbol): List[js.MethodDef] = {
-      import scala.reflect.internal.Flags
-
-      // Flags of members we do not want to consider for reflective call proxys
-      val excludedFlags = (
-          Flags.BRIDGE  |
-          Flags.PRIVATE |
-          Flags.MACRO
-      )
-
-      /** Check if two method symbols conform in name and parameter types */
-      def weakMatch(s1: Symbol)(s2: Symbol) = {
-        val p1 = s1.tpe.params
-        val p2 = s2.tpe.params
-        s1 == s2 || // Shortcut
-        s1.name == s2.name &&
-        p1.size == p2.size &&
-        (p1 zip p2).forall { case (s1,s2) =>
-          s1.tpe =:= s2.tpe
-        }
-      }
-
-      /** Check if the symbol's owner's superclass has a matching member (and
-       *  therefore an existing proxy).
-       */
-      def superHasProxy(s: Symbol) = {
-        val alts = sym.superClass.tpe.findMember(
-            name = s.name,
-            excludedFlags = excludedFlags,
-            requiredFlags = Flags.METHOD,
-            stableOnly    = false).alternatives
-        alts.exists(weakMatch(s) _)
-      }
-
-      // Query candidate methods
-      val methods = sym.tpe.findMembers(
-          excludedFlags = excludedFlags,
-          requiredFlags = Flags.METHOD)
-
-      val candidates = methods filterNot { s =>
-        s.isConstructor  ||
-        superHasProxy(s) ||
-        jsInterop.isExport(s)
-      }
-
-      val proxies = candidates filter {
-        c => candidates.find(weakMatch(c) _).get == c
-      }
-
-      proxies.map(genReflCallProxy _).toList
-    }
-
-    /** actually generates reflective call proxy for the given method symbol */
-    private def genReflCallProxy(sym: Symbol): js.MethodDef = {
-      implicit val pos = sym.pos
-
-      val proxyIdent = encodeMethodSym(sym, reflProxy = true)
-
-      withNewLocalNameScope {
-        val jsParams = for (param <- sym.tpe.params) yield {
-          implicit val pos = param.pos
-          js.ParamDef(encodeLocalSym(param), toIRType(param.tpe),
-              mutable = false, rest = false)
-        }
-
-        val call = genApplyMethod(genThis(), sym, jsParams.map(_.ref))
-        val resTpeEnteringPosterasure = enteringPhase(currentRun.posterasurePhase) {
-          sym.tpe match {
-            case _: ExistentialType =>
-              /* We should not see an ExistentialType here. This is a
-               * scalac 2.10 bug. We assume no boxing is required. See #1581.
-               */
-              ObjectTpe
-            case symTpe =>
-              symTpe.resultType
-          }
-        }
-        val body = ensureBoxed(call, resTpeEnteringPosterasure)
-
-        js.MethodDef(static = false, proxyIdent, jsParams, jstpe.AnyType,
-            body)(OptimizerHints.empty, None)
-      }
-    }
-
     /** Generates the MethodDef of a (non-constructor) method
      *
      *  Most normal methods are emitted straightforwardly. If the result

ir/src/main/scala/org/scalajs/core/ir/InfoSerializers.scala

@@ -101,6 +101,9 @@ object InfoSerializers {
 
       import input._
 
+      val useHacks065 =
+        Set("0.6.0", "0.6.3", "0.6.4", "0.6.5").contains(version)
+
       val encodedName = readUTF()
       val isExported = readBoolean()
       val kind = ClassKind.fromByte(readByte())
@@ -126,7 +129,12 @@ object InfoSerializers {
             accessedClassData)
       }
 
-      val methods = readList(readMethod())
+      val methods0 = readList(readMethod())
+      val methods = if (useHacks065) {
+        methods0.filter(m => !Definitions.isReflProxyName(m.encodedName))
+      } else {
+        methods0
+      }
 
       val info = ClassInfo(encodedName, isExported, kind,
           superClass, interfaces, methods)

ir/src/main/scala/org/scalajs/core/ir/Serializers.scala

@@ -712,7 +712,17 @@ object Serializers {
           val superClass = readOptIdent()
           val parents = readIdents()
           val jsName = Some(readString()).filter(_ != "")
-          val defs = readTrees()
+          val defs0 = readTrees()
+          val defs = if (useHacks065) {
+            defs0.filter {
+              case MethodDef(_, Ident(name, _), _, _, _) =>
+                !Definitions.isReflProxyName(name)
+              case _ =>
+                true
+            }
+          } else {
+            defs0
+          }
           val optimizerHints = new OptimizerHints(readInt())
           ClassDef(name, kind, superClass, parents, jsName, defs)(optimizerHints)
 

project/BinaryIncompatibilities.scala

@@ -12,12 +12,44 @@ object BinaryIncompatibilities {
   )
 
   val SbtPlugin = Seq(
+      // private, not an issue
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.sbtplugin.ScalaJSPluginInternal.org$scalajs$sbtplugin$ScalaJSPluginInternal$$filterOutReflProxies")
   )
 
   val TestAdapter = Seq(
   )
 
   val CLI = Seq(
+      // private, not an issue
+      ProblemFilters.exclude[MissingTypesProblem](
+          "org.scalajs.cli.Scalajsp$Options$"),
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.cli.Scalajsp#Options.this"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem](
+          "org.scalajs.cli.Scalajsp#Options.<init>$default$2"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem](
+          "org.scalajs.cli.Scalajsp#Options.<init>$default$3"),
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.cli.Scalajsp#Options.<init>$default$4"),
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.cli.Scalajsp#Options.apply"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem](
+          "org.scalajs.cli.Scalajsp#Options.apply$default$2"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem](
+          "org.scalajs.cli.Scalajsp#Options.apply$default$3"),
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.cli.Scalajsp#Options.apply$default$4"),
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.cli.Scalajsp#Options.copy"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem](
+          "org.scalajs.cli.Scalajsp#Options.copy$default$2"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem](
+          "org.scalajs.cli.Scalajsp#Options.copy$default$3"),
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.cli.Scalajsp#Options.copy$default$4"),
+      ProblemFilters.exclude[MissingMethodProblem](
+          "org.scalajs.cli.Scalajsp#Options.showReflProxy")
   )
 
   val Library = Seq(

sbt-plugin/src/main/scala/scala/scalajs/sbtplugin/ScalaJSPluginInternal.scala

@@ -143,14 +143,12 @@ object ScalaJSPluginInternal {
 
   private def scalajspSettings: Seq[Setting[_]] = {
     case class Options(
-        infos: Boolean = false,
-        showReflProxy: Boolean = false
+        infos: Boolean = false
     )
 
     val optionsParser: Parser[Options] = {
       token(OptSpace ~> (
           (literal("-i") | "--infos") ^^^ ((_: Options).copy(infos = true))
-        | (literal("-p") | "--reflProxies") ^^^ ((_: Options).copy(showReflProxy = true))
       )).* map {
         fns => Function.chain(fns)(Options())
       }
@@ -182,32 +180,15 @@ object ScalaJSPluginInternal {
           val vfile = ScalajspUtils.loadIRFile(cp, relPath)
 
           val stdout = new java.io.PrintWriter(System.out)
-          if (options.infos) {
+          if (options.infos)
             new InfoPrinter(stdout).print(vfile.info)
-          } else {
-            val (info, tree) = vfile.infoAndTree
-            val outTree = {
-              if (options.showReflProxy) tree
-              else filterOutReflProxies(tree)
-            }
-            new IRTreePrinter(stdout).printTopLevelTree(outTree)
-          }
+          else
+            new IRTreePrinter(stdout).printTopLevelTree(vfile.tree)
           stdout.flush()
         }
     )
   }
 
-  // !!! CODE DUPLICATION with Scalajsp.filterOutReflProxies
-  private def filterOutReflProxies(tree: ir.Trees.ClassDef): ir.Trees.ClassDef = {
-    import ir.Trees._
-    import ir.Definitions.isReflProxyName
-    val newDefs = tree.defs.filter {
-      case MethodDef(_, Ident(name, _), _, _, _) => !isReflProxyName(name)
-      case _ => true
-    }
-    tree.copy(defs = newDefs)(tree.optimizerHints)(tree.pos)
-  }
-
   val scalaJSConfigSettings: Seq[Setting[_]] = Seq(
       incOptions ~= scalaJSPatchIncOptions
   ) ++ (

test-suite/js/src/test/scala/org/scalajs/testsuite/compiler/ReflectiveCallTest.scala

@@ -221,6 +221,24 @@ object ReflectiveCallTest extends JasmineTest {
       expect(call(new C)).toEqual(1)
     }
 
+    it("should be bug-compatible with Scala/JVM for inherited overloads") {
+      class Base {
+        def foo(x: Option[Int]): String = "a"
+      }
+
+      class Sub extends Base {
+        def foo(x: Option[String]): Int = 1
+      }
+
+      val sub = new Sub
+
+      val x: { def foo(x: Option[Int]): Any } = sub
+      expect(x.foo(Some(1)).asInstanceOf[js.Any]).toEqual(1) // here is the "bug"
+
+      val y: { def foo(x: Option[String]): Any } = sub
+      expect(y.foo(Some("hello")).asInstanceOf[js.Any]).toEqual(1)
+    }
+
     it("should work on java.lang.Object.{ notify, notifyAll } - #303") {
       type ObjNotifyLike = Any {
         def notify(): Unit

tools/shared/src/main/scala/org/scalajs/core/tools/optimizer/Analysis.scala

@@ -75,6 +75,7 @@ object Analysis {
     final case object None extends MethodSyntheticKind
     // TODO Get rid of InheritedConstructor when we can break binary compat
     final case object InheritedConstructor extends MethodSyntheticKind
+    final case class ReflectiveProxy(target: String) extends MethodSyntheticKind
   }
 
   sealed trait Error {