Merge pull request #4772 from dotty-staging/fix-#4753

Fix #4753: Ignore implicit shortcuts in RefChecks

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

@@ -2,7 +2,7 @@ package dotty.tools.dotc
 package transform
 
 import MegaPhase._
-import core.DenotTransformers.IdentityDenotTransformer
+import core.DenotTransformers.{DenotTransformer, IdentityDenotTransformer}
 import core.Symbols._
 import core.Contexts._
 import core.Types._
@@ -44,8 +44,21 @@ import collection.mutable
  *  (2) A reference `qual.apply` where `qual` has implicit function type and
  *  `qual` refers to a method `m` is rewritten to a reference to `m$direct`,
  *  keeping the same type and value arguments as they are found in `qual`.
+ *
+ *  Note: The phase adds direct methods for all methods with IFT results that
+ *        are defined in the transformed compilation unit, as well as for all
+ *        methods that are referenced from inside the unit. It does NOT do an
+ *        info transformer that adds these methods everywhere where an IFT returning
+ *        method exists (including in separately compiled classes).
+ *        Adding such an info transformer is impractical because it would mean
+ *        that we have to force the types of all members of classes that are referenced.
+ *        But not adding an info transformer can lead to inconsistencies in RefChecks.
+ *        We solve that by ignoring direct methods in Refchecks.
+ *        Another, related issue is bridge generation, where we also generate
+ *        shortcut methods on the fly.
  */
 class ShortcutImplicits extends MiniPhase with IdentityDenotTransformer { thisPhase =>
+  import ShortcutImplicits._
   import tpd._
 
   override def phaseName: String = ShortcutImplicits.name
@@ -61,82 +74,38 @@ 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)
-
-  /** @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`.
-    */
-  private def directInfo(info: Type)(implicit ctx: Context): Type = info match {
-    case info: PolyType   => info.derivedLambdaType(resType = directInfo(info.resultType))
-    case info: MethodType => info.derivedLambdaType(resType = directInfo(info.resultType))
-    case info: ExprType   => directInfo(info.resultType)
-    case info             => info.member(nme.apply).info
-  }
-
-  /** A new `m$direct` method to accompany the given method `m` */
-  private def newDirectMethod(sym: Symbol)(implicit ctx: Context): Symbol = {
-    val direct = sym.copy(
-      name = DirectMethodName(sym.name.asTermName).asInstanceOf[sym.ThisName],
-      flags = sym.flags | Synthetic,
-      info = directInfo(sym.info))
-    if (direct.allOverriddenSymbols.isEmpty) direct.resetFlag(Override)
-    direct
-  }
 
   /** The direct method `m$direct` that accompanies the given method `m`.
     *  Create one if it does not exist already.
     */
   private def directMethod(sym: Symbol)(implicit ctx: Context): Symbol =
-    if (sym.owner.isClass) {
-      val direct = sym.owner.info.member(DirectMethodName(sym.name.asTermName))
-        .suchThat(_.info matches directInfo(sym.info)).symbol
-      if (direct.maybeOwner == sym.owner) direct
-      else newDirectMethod(sym).enteredAfter(thisPhase)
-    }
-    else directMeth.getOrElseUpdate(sym, newDirectMethod(sym))
+    if (sym.owner.isClass) shortcutMethod(sym, thisPhase)
+    else directMeth.getOrElseUpdate(sym, newShortcutMethod(sym))
 
   /** Transform `qual.apply` occurrences according to rewrite rule (2) above */
   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)
         case Block(stats, expr)  => cpy.Block(tree)(stats, directQual(expr))
         case tree: RefTree =>
           cpy.Ref(tree)(DirectMethodName(tree.name.asTermName))
-            .withType(directMethod(tree.symbol).termRef)
+            .withType(tree.tpe.asInstanceOf[NamedType].prefix.select(directMethod(tree.symbol)))
       }
       directQual(tree.qualifier)
     } else tree
 
   /** 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
@@ -179,4 +148,51 @@ 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`.
+    */
+  private def directInfo(info: Type)(implicit ctx: Context): Type = info match {
+    case info: PolyType   => info.derivedLambdaType(resType = directInfo(info.resultType))
+    case info: MethodType => info.derivedLambdaType(resType = directInfo(info.resultType))
+    case info: ExprType   => directInfo(info.resultType)
+    case info             => info.member(nme.apply).info
+  }
+
+  /** A new `m$direct` method to accompany the given method `m` */
+  private def newShortcutMethod(sym: Symbol)(implicit ctx: Context): Symbol =
+    sym.copy(
+      name = DirectMethodName(sym.name.asTermName).asInstanceOf[sym.ThisName],
+      flags = sym.flags &~ Override | Synthetic,
+      info = directInfo(sym.info))
+
+  def shortcutMethod(sym: Symbol, phase: DenotTransformer)(implicit ctx: Context) =
+    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)
 }
+
+

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -336,8 +336,9 @@ object RefChecks {
       } else if (!other.is(Deferred) &&
                  !other.name.is(DefaultGetterName) &&
                  !member.isAnyOverride) {
-        // (*) Exclusion for default getters, fixes SI-5178. We cannot assign the Override flag to
+        // Exclusion for default getters, fixes SI-5178. We cannot assign the Override flag to
         // the default getter: one default getter might sometimes override, sometimes not. Example in comment on ticket.
+        // Also exclusion for implicit shortcut methods
         // Also excluded under Scala2 mode are overrides of default methods of Java traits.
         if (autoOverride(member) ||
             other.owner.is(JavaTrait) && ctx.testScala2Mode("`override' modifier required when a Java 8 default method is re-implemented", member.pos))
@@ -446,9 +447,12 @@ object RefChecks {
           }
 
       def ignoreDeferred(member: SingleDenotation) =
-        member.isType ||
-          member.symbol.isSuperAccessor || // not yet synthesized
-          member.symbol.is(JavaDefined) && hasJavaErasedOverriding(member.symbol)
+        member.isType || {
+          val mbr = member.symbol
+          mbr.isSuperAccessor || // not yet synthesized
+          ShortcutImplicits.isImplicitShortcut(mbr) || // only synthesized when referenced, see Note in ShortcutImplicits
+          mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)
+        }
 
       // 2. Check that only abstract classes have deferred members
       def checkNoAbstractMembers(): Unit = {

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)
+}