closes #3281, #3866.

I don't understand why we had to clear() the undoLog in the first place,
since the undoXXX methods increase and decrease its size symmetrically,
so the log should always be empty once they have all unwound. Was it a
(premature) optimisation or was there some kind of semantic meaning to
it that I didn't see?

review by odersky

3 files changed, 36 insertions, 12 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/Types.scala b/src/compiler/scala/tools/nsc/symtab/Types.scala
index 8f72bdcde6..f4f0d1e4a7 100644
--- a/src/compiler/scala/tools/nsc/symtab/Types.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Types.scala
@@ -114,7 +114,7 @@ trait Types extends reflect.generic.Types { self: SymbolTable =>
     }
 
     private[Types] def record(tv: TypeVar) = {log = (tv, tv.constr.cloneInternal) :: log}
-    private[Types] def clear {log = List()}
+    private[Types] def clear() { log = List() } // TODO: what's the point of this method? -- we roll back the log (using undoTo) in the combinators below anyway, see comments at clear() calls below
 
     // `block` should not affect constraints on typevars
     def undo[T](block: => T): T = {
@@ -2309,23 +2309,28 @@ A type's typeSymbol should never be inspected directly.
       // only one of them is in the set of tvars that need to be solved, but
       // they share the same TypeConstraint instance
 
-
+    // <region name="constraint mutators + undoLog">
+    // invariant: before mutating constr, save old state in undoLog (undoLog is used to reset constraints to avoid piling up unrelated ones)
     def setInst(tp: Type) {
 //      assert(!(tp containsTp this), this)
+      undoLog record this
       constr.inst = tp
     }
 
     def addLoBound(tp: Type, numBound: Boolean = false) {
       assert(tp != this) // implies there is a cycle somewhere (?)
       //println("addLoBound: "+(safeToString, debugString(tp))) //DEBUG
+      undoLog record this
       constr.addLoBound(tp, numBound)
     }
 
     def addHiBound(tp: Type, numBound: Boolean = false) {
       // assert(tp != this)
       //println("addHiBound: "+(safeToString, debugString(tp))) //DEBUG
+      undoLog record this
       constr.addHiBound(tp, numBound)
     }
+    // </region>
 
     // ignore subtyping&equality checks while true -- see findMember
     private[TypeVar] var suspended = false
@@ -2342,8 +2347,6 @@ A type's typeSymbol should never be inspected directly.
     def registerBound(tp: Type, isLowerBound: Boolean, numBound: Boolean = false): Boolean = { //println("regBound: "+(safeToString, debugString(tp), isLowerBound)) //@MDEBUG
       if(isLowerBound) assert(tp != this)
 
-      undoLog record this
-
       def checkSubtype(tp1: Type, tp2: Type) =
         if (numBound)
           if (isLowerBound) tp1 weak_<:< tp2
@@ -2391,8 +2394,6 @@ A type's typeSymbol should never be inspected directly.
       if (suspended) tp =:= origin
       else if (constr.instValid) checkIsSameType(tp)
       else isRelatable(tp) && {
-        undoLog record this
-
         val newInst = wildcardToTypeVarMap(tp)
         if (constr.isWithinBounds(newInst)) {
           setInst(tp)
@@ -2444,7 +2445,7 @@ A type's typeSymbol should never be inspected directly.
                           origin+
                           (if(typeArgs.isEmpty) "" else (typeArgs map (_.safeToString)).mkString("[ ", ", ", " ]")) // +"#"+tid //DEBUG
       if (constr.inst eq null) "<null " + origin + ">"
-      else if (settings.debug.value) varString+"(@"+constr.## +")"+constr.toString
+      // else if (settings.debug.value) varString+"(@"+constr.## +")"+constr.toString
       else if (constr.inst eq NoType) varString
       else constr.inst.toString
     }
@@ -4027,7 +4028,9 @@ A type's typeSymbol should never be inspected directly.
     }
   } finally {
     subsametypeRecursions -= 1
-    if (subsametypeRecursions == 0) undoLog.clear
+    // XXX AM TODO: figure out when it is safe and needed to clear the log -- the commented approach below is too eager (it breaks #3281, #3866)
+    // it doesn't help to keep separate recursion counts for the three methods that now share it
+    // if (subsametypeRecursions == 0) undoLog.clear()
   }
 
   def isDifferentType(tp1: Type, tp2: Type): Boolean = try {
@@ -4037,7 +4040,9 @@ A type's typeSymbol should never be inspected directly.
     }
   } finally {
     subsametypeRecursions -= 1
-    if (subsametypeRecursions == 0) undoLog.clear
+    // XXX AM TODO: figure out when it is safe and needed to clear the log -- the commented approach below is too eager (it breaks #3281, #3866)
+    // it doesn't help to keep separate recursion counts for the three methods that now share it
+    // if (subsametypeRecursions == 0) undoLog.clear()
   }
 
   def isDifferentTypeConstructor(tp1: Type, tp2: Type): Boolean = tp1 match {
@@ -4366,7 +4371,9 @@ A type's typeSymbol should never be inspected directly.
     }
   } finally {
     subsametypeRecursions -= 1
-    if (subsametypeRecursions == 0) undoLog clear
+    // XXX AM TODO: figure out when it is safe and needed to clear the log -- the commented approach below is too eager (it breaks #3281, #3866)
+    // it doesn't help to keep separate recursion counts for the three methods that now share it
+    // if (subsametypeRecursions == 0) undoLog.clear()
   }
 
   /** Does this type have a prefix that begins with a type variable,
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index c494185133..e4ae8f680d 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -168,11 +168,11 @@ trait Typers { self: Analyzer =>
     import context0.unit
 
     val infer = new Inferencer(context0) {
-      override def isCoercible(tp: Type, pt: Type): Boolean =
+      override def isCoercible(tp: Type, pt: Type): Boolean = undoLog undo { // #3281
         tp.isError || pt.isError ||
         context0.implicitsEnabled && // this condition prevents chains of views
         inferView(EmptyTree, tp, pt, false) != EmptyTree
-    }
+      }}
 
     /** Find implicit arguments and pass them to given tree.
      */
diff --git a/test/files/pos/t3866.scala b/test/files/pos/t3866.scala
new file mode 100644
index 0000000000..5d366ccf13
--- /dev/null
+++ b/test/files/pos/t3866.scala
@@ -0,0 +1,17 @@
+abstract class ImplicitRepeated {
+  trait T[+A, +B]
+  trait X
+
+  def f[N, R <: List[_]](elems: T[N, R]*)                // alternative a)
+  def f[N, R <: List[_]](props: String, elems: T[N, R]*) // alternative b)
+
+  // the following implicit causes "cannot be applied" errors
+  implicit def xToRight(r: X): T[Nothing, X] = null
+  implicit def anyToN[N](x: N): T[N, Nothing] = null
+
+
+  f("A", 1, 2) // should be implicitly resolved to alternative b)
+  f( 1, 2 )    // should be implicitly resolved to alternative a)
+    // ImplicitRepeated.this.f[Int, Nothing]("A", ImplicitRepeated.this.anyToN[Int](1), ImplicitRepeated.this.anyToN[Int](2));
+    // ImplicitRepeated.this.f[Int, Nothing](ImplicitRepeated.this.anyToN[Int](1), ImplicitRepeated.this.anyToN[Int](2))
+}
\ No newline at end of file