Merge pull request #3263 from retronym/ticket/6780

SI-6780 Better handling of cycles in in-scope implicit search

3 files changed, 67 insertions, 28 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
index 2be6d92ed0..2b4417a80d 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -773,7 +773,7 @@ trait Contexts { self: Analyzer =>
     // Implicit collection
     //
 
-    private var implicitsCache: List[List[ImplicitInfo]] = null
+    private var implicitsCache: List[ImplicitInfo] = null
     private var implicitsRunId = NoRunId
 
     def resetCache() {
@@ -832,35 +832,53 @@ trait Contexts { self: Analyzer =>
      * filtered out later by `eligibleInfos` (SI-4270 / 9129cfe9), as they don't type-check.
      */
     def implicitss: List[List[ImplicitInfo]] = {
-      val imports = this.imports
       val nextOuter = this.nextOuter
-      if (implicitsRunId != currentRunId) {
-        implicitsRunId = currentRunId
-        implicitsCache = List()
-        val newImplicits: List[ImplicitInfo] =
-          if (owner != nextOuter.owner && owner.isClass && !owner.isPackageClass && !inSelfSuperCall) {
-            if (!owner.isInitialized) return nextOuter.implicitss
-            // debuglog("collect member implicits " + owner + ", implicit members = " + owner.thisType.implicitMembers)//DEBUG
-            savingEnclClass(this) {
-              // !!! In the body of `class C(implicit a: A) { }`, `implicitss` returns `List(List(a), List(a), List(<predef..)))`
-              //     it handled correctly by implicit search, which considers the second `a` to be shadowed, but should be
-              //     remedied nonetheless.
-              collectImplicits(owner.thisType.implicitMembers, owner.thisType)
-            }
-          } else if (scope != nextOuter.scope && !owner.isPackageClass) {
-            debuglog("collect local implicits " + scope.toList)//DEBUG
-            collectImplicits(scope, NoPrefix)
-          } else if (firstImport != nextOuter.firstImport) {
-            assert(imports.tail.headOption == nextOuter.firstImport, (imports, nextOuter.imports))
-            collectImplicitImports(imports.head)
-          } else if (owner.isPackageClass) {
-            // the corresponding package object may contain implicit members.
-            collectImplicits(owner.tpe.implicitMembers, owner.tpe)
-          } else List()
-        implicitsCache = if (newImplicits.isEmpty) nextOuter.implicitss
-                         else newImplicits :: nextOuter.implicitss
+      def withOuter(is: List[ImplicitInfo]): List[List[ImplicitInfo]] =
+        is match {
+          case Nil => nextOuter.implicitss
+          case _   => is :: nextOuter.implicitss
+        }
+
+      val CycleMarker = NoRunId - 1
+      if (implicitsRunId == CycleMarker) {
+        debuglog(s"cycle while collecting implicits at owner ${owner}, probably due to an implicit without an explicit return type. Continuing with implicits from enclosing contexts.")
+        withOuter(Nil)
+      } else if (implicitsRunId != currentRunId) {
+        implicitsRunId = CycleMarker
+        implicits(nextOuter) match {
+          case None =>
+            implicitsRunId = NoRunId
+            withOuter(Nil)
+          case Some(is) =>
+            implicitsRunId = currentRunId
+            implicitsCache = is
+            withOuter(is)
+        }
       }
-      implicitsCache
+      else withOuter(implicitsCache)
+    }
+
+    /** @return None if a cycle is detected, or Some(infos) containing the in-scope implicits at this context */
+    private def implicits(nextOuter: Context): Option[List[ImplicitInfo]] = {
+      val imports = this.imports
+      if (owner != nextOuter.owner && owner.isClass && !owner.isPackageClass && !inSelfSuperCall) {
+        if (!owner.isInitialized) None
+        else savingEnclClass(this) {
+          // !!! In the body of `class C(implicit a: A) { }`, `implicitss` returns `List(List(a), List(a), List(<predef..)))`
+          //     it handled correctly by implicit search, which considers the second `a` to be shadowed, but should be
+          //     remedied nonetheless.
+          Some(collectImplicits(owner.thisType.implicitMembers, owner.thisType))
+        }
+      } else if (scope != nextOuter.scope && !owner.isPackageClass) {
+        debuglog("collect local implicits " + scope.toList)//DEBUG
+        Some(collectImplicits(scope, NoPrefix))
+      } else if (firstImport != nextOuter.firstImport) {
+        assert(imports.tail.headOption == nextOuter.firstImport, (imports, nextOuter.imports))
+        Some(collectImplicitImports(imports.head))
+      } else if (owner.isPackageClass) {
+        // the corresponding package object may contain implicit members.
+        Some(collectImplicits(owner.tpe.implicitMembers, owner.tpe))
+      } else Some(Nil)
     }
 
     //
diff --git a/test/files/neg/t712.check b/test/files/neg/t712.check
index 6819dc0ce9..831e943063 100644
--- a/test/files/neg/t712.check
+++ b/test/files/neg/t712.check
@@ -1,4 +1,5 @@
 t712.scala:10: error: value self is not a member of B.this.ParentImpl
+ Note: implicit method coerce is not applicable here because it comes after the application point and it lacks an explicit result type
   implicit def coerce(p : ParentImpl) = p.self;
                                           ^
 one error found
diff --git a/test/files/pos/t6780.scala b/test/files/pos/t6780.scala
new file mode 100644
index 0000000000..4a358046c6
--- /dev/null
+++ b/test/files/pos/t6780.scala
@@ -0,0 +1,20 @@
+object O {
+  implicit def i: Int = 0
+}
+
+import O._
+
+trait Foo {
+  implicit val v1: Any
+  implicit def d1: Any
+           val v2: Any
+  implicit val v3: Any
+}
+
+trait Bar1 extends Foo {
+  implicit val v1      = {implicitly[Int]; ()} // failed due to cycle in Context#implicits being broken with Nil.
+           def d1      = {implicitly[Int]; ()} // okay
+  implicit val v2      = {implicitly[Int]; ()} // okay
+  implicit val v3: Any = {implicitly[Int]; ()} // okay
+
+}