Fix bridge generation for implicit shortcut methods

Author: odersky

compiler/src/dotty/tools/dotc/transform/Bridges.scala

@@ -9,9 +9,10 @@ import ast.untpd
 import collection.{mutable, immutable}
 import TypeErasure._
 import ValueClasses.isDerivedValueClass
+import ShortcutImplicits._
 
 /** A helper class for generating bridge methods in class `root`. */
-class Bridges(root: ClassSymbol)(implicit ctx: Context) {
+class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Context) {
   import ast.tpd._
 
   assert(ctx.phase == ctx.erasurePhase.next)
@@ -26,7 +27,11 @@ class Bridges(root: ClassSymbol)(implicit ctx: Context) {
      *  only in classes, never in traits.
      */
     override def parents = Array(root.superClass)
-    override def exclude(sym: Symbol) = !sym.is(MethodOrModule) || super.exclude(sym)
+    
+    override def exclude(sym: Symbol) =
+      !sym.is(MethodOrModule) ||
+      isImplicitShortcut(sym) ||
+      super.exclude(sym)
   }
 
   //val site = root.thisType
@@ -81,8 +86,7 @@ class Bridges(root: ClassSymbol)(implicit ctx: Context) {
       owner = root,
       flags = (member.flags | Method | Bridge | Artifact) &~
         (Accessor | ParamAccessor | CaseAccessor | Deferred | Lazy | Module),
-      coord = bridgePosFor(member))
-      .enteredAfter(ctx.erasurePhase.asInstanceOf[DenotTransformer]).asTerm
+      coord = bridgePosFor(member)).enteredAfter(thisPhase).asTerm
 
     ctx.debuglog(
       i"""generating bridge from ${other.showLocated}: ${other.info}
@@ -111,8 +115,19 @@ class Bridges(root: ClassSymbol)(implicit ctx: Context) {
    */
   def add(stats: List[untpd.Tree]): List[untpd.Tree] = {
     val opc = new BridgesCursor()(preErasureCtx)
+    val ectx = ctx.withPhase(thisPhase)
     while (opc.hasNext) {
-      if (!opc.overriding.is(Deferred)) addBridgeIfNeeded(opc.overriding, opc.overridden)
+      if (!opc.overriding.is(Deferred)) {
+        addBridgeIfNeeded(opc.overriding, opc.overridden)
+        
+        if (needsImplicitShortcut(opc.overriding)(ectx))
+          // implicit shortcuts do not show up in the Bridges cursor, since they
+          // are created only when referenced. Therefore we need to generate a bridge
+          // for them specifically, if one is needed for the original methods.
+          addBridgeIfNeeded(
+            shortcutMethod(opc.overriding, thisPhase)(ectx),
+            shortcutMethod(opc.overridden, thisPhase)(ectx))
+      }
       opc.next()
     }
     if (bridges.isEmpty) stats

compiler/src/dotty/tools/dotc/transform/Erasure.scala

@@ -91,7 +91,7 @@ class Erasure extends Phase with DenotTransformer {
       ref.derivedSingleDenotation(ref.symbol, transformInfo(ref.symbol, ref.symbol.info))
   }
 
-  val eraser = new Erasure.Typer
+  val eraser = new Erasure.Typer(this)
 
   def run(implicit ctx: Context): Unit = {
     val unit = ctx.compilationUnit
@@ -314,7 +314,7 @@ object Erasure {
       }
   }
 
-  class Typer extends typer.ReTyper with NoChecking {
+  class Typer(erasurePhase: DenotTransformer) extends typer.ReTyper with NoChecking {
     import Boxing._
 
     def erasedType(tree: untpd.Tree)(implicit ctx: Context): Type = {
@@ -672,7 +672,7 @@ object Erasure {
 
     override def typedStats(stats: List[untpd.Tree], exprOwner: Symbol)(implicit ctx: Context): List[Tree] = {
       val stats1 =
-        if (takesBridges(ctx.owner)) new Bridges(ctx.owner.asClass).add(stats)
+        if (takesBridges(ctx.owner)) new Bridges(ctx.owner.asClass, erasurePhase).add(stats)
         else stats
       super.typedStats(stats1, exprOwner).filter(!_.isEmpty)
     }

compiler/src/dotty/tools/dotc/transform/ShortcutImplicits.scala

@@ -73,29 +73,10 @@ class ShortcutImplicits extends MiniPhase with IdentityDenotTransformer { thisPh
   override def initContext(ctx: FreshContext) =
     DirectMeth = ctx.addLocation[MutableSymbolMap[Symbol]]()
 
-  /** If this option is true, we don't specialize symbols that are known to be only
-   *  targets of monomorphic calls.
-   *  The reason for this option is that benchmarks show that on the JVM for monomorphic dispatch
-   *  scenarios inlining and escape analysis can often remove all calling overhead, so we might as
-   *  well not duplicate the code. We need more experience to decide on the best setting of this option.
-   */
-  final val specializeMonoTargets = true
 
   override def prepareForUnit(tree: Tree)(implicit ctx: Context) =
     ctx.fresh.updateStore(DirectMeth, newMutableSymbolMap[Symbol])
 
-  /** Should `sym` get a ..$direct companion?
-    *  This is the case if `sym` is a method with a non-nullary implicit function type as final result type.
-    *  However if `specializeMonoTargets` is false, we exclude symbols that are known
-    *  to be only targets of monomorphic calls because they are effectively
-    *  final and don't override anything.
-    */
-  private def shouldBeSpecialized(sym: Symbol)(implicit ctx: Context) =
-    sym.is(Method, butNot = Accessor) &&
-    defn.isImplicitFunctionType(sym.info.finalResultType) &&
-    defn.functionArity(sym.info.finalResultType) > 0 &&
-    !sym.isAnonymousFunction &&
-    (specializeMonoTargets || !sym.isEffectivelyFinal || sym.allOverriddenSymbols.nonEmpty)
 
   /** The direct method `m$direct` that accompanies the given method `m`.
     *  Create one if it does not exist already.
@@ -108,7 +89,7 @@ class ShortcutImplicits extends MiniPhase with IdentityDenotTransformer { thisPh
   override def transformSelect(tree: Select)(implicit ctx: Context) =
     if (tree.name == nme.apply &&
         defn.isImplicitFunctionType(tree.qualifier.tpe.widen) &&
-        shouldBeSpecialized(tree.qualifier.symbol)) {
+        needsImplicitShortcut(tree.qualifier.symbol)) {
       def directQual(tree: Tree): Tree = tree match {
         case Apply(fn, args)     => cpy.Apply(tree)(directQual(fn), args)
         case TypeApply(fn, args) => cpy.TypeApply(tree)(directQual(fn), args)
@@ -127,7 +108,7 @@ class ShortcutImplicits extends MiniPhase with IdentityDenotTransformer { thisPh
   /** Transform methods with implicit function type result according to rewrite rule (1) above */
   override def transformDefDef(mdef: DefDef)(implicit ctx: Context): Tree = {
     val original = mdef.symbol
-    if (shouldBeSpecialized(original)) {
+    if (needsImplicitShortcut(original)) {
       val direct = directMethod(original)
 
       // Move @tailrec to the direct method
@@ -171,6 +152,27 @@ class ShortcutImplicits extends MiniPhase with IdentityDenotTransformer { thisPh
 object ShortcutImplicits {
   val name = "shortcutImplicits"
 
+  /** If this option is true, we don't specialize symbols that are known to be only
+   *  targets of monomorphic calls.
+   *  The reason for this option is that benchmarks show that on the JVM for monomorphic dispatch
+   *  scenarios inlining and escape analysis can often remove all calling overhead, so we might as
+   *  well not duplicate the code. We need more experience to decide on the best setting of this option.
+   */
+  final val specializeMonoTargets = true
+
+  /** Should `sym` get a ..$direct companion?
+    *  This is the case if `sym` is a method with a non-nullary implicit function type as final result type.
+    *  However if `specializeMonoTargets` is false, we exclude symbols that are known
+    *  to be only targets of monomorphic calls because they are effectively
+    *  final and don't override anything.
+    */
+  def needsImplicitShortcut(sym: Symbol)(implicit ctx: Context) =
+    sym.is(Method, butNot = Accessor) &&
+    defn.isImplicitFunctionType(sym.info.finalResultType) &&
+    defn.functionArity(sym.info.finalResultType) > 0 &&
+    !sym.isAnonymousFunction &&
+    (specializeMonoTargets || !sym.isEffectivelyFinal || sym.allOverriddenSymbols.nonEmpty)
+
   /** @pre    The type's final result type is an implicit function type `implicit Ts => R`.
     *  @return The type of the `apply` member of `implicit Ts => R`.
     */
@@ -181,8 +183,6 @@ object ShortcutImplicits {
     case info             => info.member(nme.apply).info
   }
 
-  def isImplicitShortcut(sym: Symbol)(implicit ctx: Context) = sym.name.is(DirectMethodName)
-
   /** A new `m$direct` method to accompany the given method `m` */
   private def newShortcutMethod(sym: Symbol)(implicit ctx: Context): Symbol =
     sym.copy(
@@ -195,6 +195,8 @@ object ShortcutImplicits {
     sym.owner.info.decl(DirectMethodName(sym.name.asTermName))
       .suchThat(_.info matches directInfo(sym.info)).symbol
       .orElse(newShortcutMethod(sym).enteredAfter(phase))
+
+  def isImplicitShortcut(sym: Symbol)(implicit ctx: Context) = sym.name.is(DirectMethodName)
 }
 
 

tests/pos/i4753.scala

@@ -0,0 +1,13 @@
+class A
+
+trait Foo {
+  def foo: implicit A => Int
+}
+
+class Test {
+  new FooI{}
+}
+
+class FooI extends Foo {
+  def foo: implicit A => Int = 3
+}

tests/run/implicitShortcut/Base_1.scala

@@ -0,0 +1,8 @@
+package implicitShortcut
+
+class C
+abstract class Base[T] {
+
+  def foo(x: T): implicit C => T = x
+
+}

tests/run/implicitShortcut/Derived_2.scala

@@ -0,0 +1,5 @@
+package implicitShortcut
+
+class Derived extends Base[Int] {
+  override def foo(x: Int): implicit C => Int = 42
+}

tests/run/implicitShortcut/Test_3.scala

@@ -0,0 +1,10 @@
+import implicitShortcut._
+object Test extends App {
+
+  val d = new Derived
+  val b: Base[Int] = d
+  implicit val c: C = new C
+
+  assert(b.foo(1) == 42)
+  assert(d.foo(1) == 42)
+}