Merge pull request #1658 from dotty-staging/fix-#1639

Fix #1639: Changes around implicits and apply methods

5 files changed, 101 insertions, 56 deletions

diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala
index 8477cfe28..26c1e5ebc 100644
--- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala
+++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala
@@ -286,11 +286,16 @@ abstract class Reporter extends interfaces.ReporterResult {
   }
 
   /** Should this diagnostic not be reported at all? */
-  def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean = ctx.mode.is(Mode.Printing)
+  def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean =
+    ctx.mode.is(Mode.Printing)
 
   /** Does this reporter contain not yet reported errors or warnings? */
   def hasPending: Boolean = false
 
+  /** If this reporter buffers messages, remove and return all buffered messages. */
+  def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] = Nil
+
   /** Issue all error messages in this reporter to next outer one, or make sure they are written. */
-  def flush()(implicit ctx: Context): Unit = {}
+  def flush()(implicit ctx: Context): Unit =
+    removeBufferedMessages.foreach(ctx.reporter.report)
 }
diff --git a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala
index 586273c2e..34b109882 100644
--- a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala
+++ b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala
@@ -36,11 +36,9 @@ class StoreReporter(outer: Reporter) extends Reporter {
     }
   }
 
-  override def flush()(implicit ctx: Context) =
-    if (infos != null) {
-      infos.foreach(ctx.reporter.report(_))
-      infos = null
-    }
+  override def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] =
+    if (infos != null) try infos.toList finally infos = null
+    else Nil
 
   override def errorsReported = hasErrors || outer.errorsReported
 }
diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
index f3dceea71..1a9a8f64c 100644
--- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
@@ -24,6 +24,7 @@ import Constants._
 import Applications._
 import ProtoTypes._
 import ErrorReporting._
+import reporting.diagnostic.MessageContainer
 import Inferencing.fullyDefinedType
 import Trees._
 import Hashable._
@@ -212,6 +213,8 @@ object Implicits {
   /** A "no matching implicit found" failure */
   case object NoImplicitMatches extends SearchFailure
 
+  case object DivergingImplicit extends SearchFailure
+
   /** A search failure that can show information about the cause */
   abstract class ExplainedSearchFailure extends SearchFailure {
     protected def pt: Type
@@ -233,9 +236,35 @@ object Implicits {
       "\n " + explanation
   }
 
-  class NonMatchingImplicit(ref: TermRef, val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure {
-    def explanation(implicit ctx: Context): String =
-      em"${err.refStr(ref)} does not $qualify"
+  class NonMatchingImplicit(ref: TermRef,
+                            val pt: Type,
+                            val argument: tpd.Tree,
+                            trail: List[MessageContainer]) extends ExplainedSearchFailure {
+    private val separator = "\n**** because ****\n"
+
+    /** Replace repeated parts beginning with `separator` by ... */
+    private def elideRepeated(str: String): String = {
+      val startIdx = str.indexOfSlice(separator)
+      val nextIdx = str.indexOfSlice(separator, startIdx + separator.length)
+      if (nextIdx < 0) str
+      else {
+        val prefix = str.take(startIdx)
+        val first = str.slice(startIdx, nextIdx)
+        var rest = str.drop(nextIdx)
+        if (rest.startsWith(first)) {
+          rest = rest.drop(first.length)
+          val dots = "\n\n     ...\n"
+          if (!rest.startsWith(dots)) rest = dots ++ rest
+        }
+        prefix ++ first ++ rest
+      }
+    }
+
+    def explanation(implicit ctx: Context): String = {
+      val headMsg = em"${err.refStr(ref)} does not $qualify"
+      val trailMsg = trail.map(mc => i"$separator  ${mc.message}").mkString
+      elideRepeated(headMsg ++ trailMsg)
+    }
   }
 
   class ShadowedImplicit(ref: TermRef, shadowing: Type, val pt: Type, val argument: tpd.Tree) extends ExplainedSearchFailure {
@@ -273,9 +302,10 @@ trait ImplicitRunInfo { self: RunInfo =>
    *                      a type variable, we need the current context, the current
    *                      runinfo context does not do.
    */
-  def implicitScope(tp: Type, liftingCtx: Context): OfTypeImplicits = {
+  def implicitScope(rootTp: Type, liftingCtx: Context): OfTypeImplicits = {
 
     val seen: mutable.Set[Type] = mutable.Set()
+    val incomplete: mutable.Set[Type] = mutable.Set()
 
     /** Replace every typeref that does not refer to a class by a conjunction of class types
      *  that has the same implicit scope as the original typeref. The motivation for applying
@@ -309,16 +339,23 @@ trait ImplicitRunInfo { self: RunInfo =>
       }
     }
 
-    def iscopeRefs(tp: Type): TermRefSet =
-      if (seen contains tp) EmptyTermRefSet
-      else {
-        seen += tp
-        iscope(tp).companionRefs
-      }
-
     // todo: compute implicits directly, without going via companionRefs?
     def collectCompanions(tp: Type): TermRefSet = track("computeImplicitScope") {
       ctx.traceIndented(i"collectCompanions($tp)", implicits) {
+
+        def iscopeRefs(t: Type): TermRefSet = implicitScopeCache.get(t) match {
+          case Some(is) =>
+            is.companionRefs
+          case None =>
+            if (seen contains t) {
+              incomplete += tp  // all references to rootTo will be accounted for in `seen` so we return `EmptySet`.
+              EmptyTermRefSet   // on the other hand, the refs of `tp` are now not accurate, so `tp` is marked incomplete.
+            } else {
+              seen += t
+              iscope(t).companionRefs
+            }
+        }
+
         val comps = new TermRefSet
         tp match {
           case tp: NamedType =>
@@ -356,7 +393,8 @@ trait ImplicitRunInfo { self: RunInfo =>
      *  @param isLifted    Type `tp` is the result of a `liftToClasses` application
      */
     def iscope(tp: Type, isLifted: Boolean = false): OfTypeImplicits = {
-      def computeIScope(cacheResult: Boolean) = {
+      val canCache = Config.cacheImplicitScopes && tp.hash != NotCached
+      def computeIScope() = {
         val savedEphemeral = ctx.typerState.ephemeral
         ctx.typerState.ephemeral = false
         try {
@@ -367,33 +405,23 @@ trait ImplicitRunInfo { self: RunInfo =>
             else
               collectCompanions(tp)
           val result = new OfTypeImplicits(tp, refs)(ctx)
-          if (ctx.typerState.ephemeral) record("ephemeral cache miss: implicitScope")
-          else if (cacheResult) implicitScopeCache(tp) = result
+          if (ctx.typerState.ephemeral)
+            record("ephemeral cache miss: implicitScope")
+          else if (canCache &&
+                   ((tp eq rootTp) ||                  // first type traversed is always cached
+                    !incomplete.contains(tp) &&        // other types are cached if they are not incomplete
+                    result.companionRefs.forall(       // and all their companion refs are cached
+                      implicitScopeCache.contains)))
+            implicitScopeCache(tp) = result
           result
         }
         finally ctx.typerState.ephemeral |= savedEphemeral
       }
-
-      if (tp.hash == NotCached || !Config.cacheImplicitScopes)
-        computeIScope(cacheResult = false)
-      else implicitScopeCache get tp match {
-        case Some(is) => is
-        case None =>
-          // Implicit scopes are tricky to cache because of loops. For example
-          // in `tests/pos/implicit-scope-loop.scala`, the scope of B contains
-          // the scope of A which contains the scope of B. We break the loop
-          // by returning EmptyTermRefSet in `collectCompanions` for types
-          // that we have already seen, but this means that we cannot cache
-          // the computed scope of A, it is incomplete.
-          // Keeping track of exactly where these loops happen would require a
-          // lot of book-keeping, instead we choose to be conservative and only
-          // cache scopes before any type has been seen. This is unfortunate
-          // because loops are very common for types in scala.collection.
-          computeIScope(cacheResult = seen.isEmpty)
-      }
+      if (canCache) implicitScopeCache.getOrElse(tp, computeIScope())
+      else computeIScope()
     }
 
-    iscope(tp)
+    iscope(rootTp)
   }
 
   /** A map that counts the number of times an implicit ref was picked */
@@ -587,7 +615,7 @@ trait Implicits { self: Typer =>
     val wildProto = implicitProto(pt, wildApprox(_))
 
     /** Search failures; overridden in ExplainedImplicitSearch */
-    protected def nonMatchingImplicit(ref: TermRef): SearchFailure = NoImplicitMatches
+    protected def nonMatchingImplicit(ref: TermRef, trail: List[MessageContainer]): SearchFailure = NoImplicitMatches
     protected def divergingImplicit(ref: TermRef): SearchFailure = NoImplicitMatches
     protected def shadowedImplicit(ref: TermRef, shadowing: Type): SearchFailure = NoImplicitMatches
     protected def failedSearch: SearchFailure = NoImplicitMatches
@@ -628,7 +656,7 @@ trait Implicits { self: Typer =>
             { implicits.println(i"invalid eqAny[$tp1, $tp2]"); false }
         }
         if (ctx.reporter.hasErrors)
-          nonMatchingImplicit(ref)
+          nonMatchingImplicit(ref, ctx.reporter.removeBufferedMessages)
         else if (contextual && !ctx.mode.is(Mode.ImplicitShadowing) &&
                  !shadowing.tpe.isError && !refMatches(shadowing)) {
           implicits.println(i"SHADOWING $ref in ${ref.termSymbol.owner} is shadowed by $shadowing in ${shadowing.symbol.owner}")
@@ -637,7 +665,7 @@ trait Implicits { self: Typer =>
         else generated1 match {
           case TypeApply(fn, targs @ (arg1 :: arg2 :: Nil))
           if fn.symbol == defn.Predef_eqAny && !validEqAnyArgs(arg1.tpe, arg2.tpe) =>
-            nonMatchingImplicit(ref)
+            nonMatchingImplicit(ref, Nil)
           case _ =>
             SearchSuccess(generated1, ref, ctx.typerState)
         }
@@ -743,8 +771,8 @@ trait Implicits { self: Typer =>
       fail
     }
     def failures = myFailures.toList
-    override def nonMatchingImplicit(ref: TermRef) =
-      record(new NonMatchingImplicit(ref, pt, argument))
+    override def nonMatchingImplicit(ref: TermRef, trail: List[MessageContainer]) =
+      record(new NonMatchingImplicit(ref, pt, argument, trail))
     override def divergingImplicit(ref: TermRef) =
       record(new DivergingImplicit(ref, pt, argument))
     override def shadowedImplicit(ref: TermRef, shadowing: Type): SearchFailure =
diff --git a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala
index f0acdef12..3252ead47 100644
--- a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala
+++ b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala
@@ -74,8 +74,8 @@ class ReTyper extends Typer {
   override def index(trees: List[untpd.Tree])(implicit ctx: Context) = ctx
   override def annotate(trees: List[untpd.Tree])(implicit ctx: Context) = ()
 
-  override def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: (Tree, TyperState) => Tree)(implicit ctx: Context): Tree =
-    fallBack(tree, ctx.typerState)
+  override def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: => Tree)(implicit ctx: Context): Tree =
+    fallBack
 
   override def completeAnnotations(mdef: untpd.MemberDef, sym: Symbol)(implicit ctx: Context): Unit = ()
 
diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
index 4ec1c460f..94b13b22b 100644
--- a/compiler/src/dotty/tools/dotc/typer/Typer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -1613,18 +1613,34 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
    *  `fallBack`.
    *
    *  1st strategy: Try to insert `.apply` so that the result conforms to prototype `pt`.
+   *                This strategy is not tried if the prototype represents already
+   *                another `.apply` or `.apply()` selection.
    *  2nd strategy: If tree is a select `qual.name`, try to insert an implicit conversion
    *    around the qualifier part `qual` so that the result conforms to the expected type
    *    with wildcard result type.
    */
-  def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: (Tree, TyperState) => Tree)(implicit ctx: Context): Tree =
-    tryEither { implicit ctx =>
+  def tryInsertApplyOrImplicit(tree: Tree, pt: ProtoType)(fallBack: => Tree)(implicit ctx: Context): Tree = {
+
+    /** Is `pt` a prototype of an `apply` selection, or a parameterless function yielding one? */
+    def isApplyProto(pt: Type): Boolean = pt match {
+      case pt: SelectionProto => pt.name == nme.apply
+      case pt: FunProto => pt.args.isEmpty && isApplyProto(pt.resultType)
+      case pt: IgnoredProto => isApplyProto(pt.ignored)
+      case _ => false
+    }
+
+    def tryApply(implicit ctx: Context) = {
       val sel = typedSelect(untpd.Select(untpd.TypedSplice(tree), nme.apply), pt)
       if (sel.tpe.isError) sel else adapt(sel, pt)
-    } { (failedTree, failedState) =>
-      tryInsertImplicitOnQualifier(tree, pt).getOrElse(fallBack(failedTree, failedState))
     }
 
+    def tryImplicit =
+      tryInsertImplicitOnQualifier(tree, pt).getOrElse(fallBack)
+
+    if (isApplyProto(pt)) tryImplicit
+    else tryEither(tryApply(_))((_, _) => tryImplicit) 
+  }
+
   /** If this tree is a select node `qual.name`, try to insert an implicit conversion
    *  `c` around `qual` so that `c(qual).name` conforms to `pt`.
    */
@@ -1716,7 +1732,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
           def hasEmptyParams(denot: SingleDenotation) = denot.info.paramTypess == ListOfNil
           pt match {
             case pt: FunProto =>
-              tryInsertApplyOrImplicit(tree, pt)((_, _) => noMatches)
+              tryInsertApplyOrImplicit(tree, pt)(noMatches)
             case _ =>
               if (altDenots exists (_.info.paramTypess == ListOfNil))
                 typed(untpd.Apply(untpd.TypedSplice(tree), Nil), pt)
@@ -1755,7 +1771,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
           case Apply(_, _) => " more"
           case _ => ""
         }
-        (_, _) => errorTree(tree, em"$methodStr does not take$more parameters")
+        errorTree(tree, em"$methodStr does not take$more parameters")
       }
     }
 
@@ -1974,9 +1990,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
             case pt: FunProto =>
               adaptToArgs(wtp, pt)
             case pt: PolyProto =>
-              tryInsertApplyOrImplicit(tree, pt) {
-                (_, _) => tree // error will be reported in typedTypeApply
-              }
+              tryInsertApplyOrImplicit(tree, pt)(tree) // error will be reported in typedTypeApply
             case _ =>
               if (ctx.mode is Mode.Type) adaptType(tree.tpe)
               else adaptNoArgs(wtp)