Fix #1857: Interpolate type variables before implicit search

[odersky]

Interpolating type variables before resolving implicit parameters means that
more companion objects can become eligible for the implicit scope. An example
demonstrating this is i1857.scala. The spec mandates this behavior because it
says that type arguments are inferred before implicit parameter resolution takes place.

Review by @smarter?

[smarter]

I think this is a too early point to do the instantiation, for example it breaks:

object Test {
  def foo1[T](x: T)(implicit ev: T): Nothing = ???

  def test1: Unit = {
    implicit val ii: Int = 42
    implicit val ss: String = "foo"

    foo1(10) // ambiguous implicit because T=Any
  }
}

Whereas previously we correctly inferred T=Int.

We already have some logic to deal with type variables that appear in implicit parameters' types: https://github.com/lampepfl/dotty/blob/119725799675ee00d6d3374771765b88e4de67bc/compiler/src/dotty/tools/dotc/typer/Typer.scala#L1846

I've simplified the testcase to show where things stop working:

class Wrapper[Host]


class SubWrapper extends Wrapper[Foo]

class Foo
object Foo {
  implicit val fooWrapper: SubWrapper = new SubWrapper
}


object RunMe {
  def main(args: Array[String]): Unit = {
    def wrap1[A, W <: Wrapper[Foo]](host: A)(implicit w: W): W = w
    def wrap2[A, W <: Wrapper[A]](host: A)(implicit w: W): W = w

    val f: Foo = new Foo

    val a = wrap1(f) // works with master
    val b = wrap2(f) // doesn't work with master
  }
}

In wrap2(f) we end up calling instantiateSelected(W, List(A)), W is bounded by Wrapper[A] but IsFullyDefinedAccumulator will not look at the bounds of W but stop because force.appliesTo(W) returns false. I think the correct fix involves tweaking IsFullyDefinedAccumulator but I'm not sure how.

[odersky]

@smarter I have a fix based on tweaking interpolateUndetVars. I am not sure how to do it by tweaking isFullyDefinedAccumulator. The problem here is that in the original example DSL is not fully defined, (nor should it be), and Host does not even appear in the implicit search. Should Host be always inferred before doing the implicit search? I actually doubt that. In fact, I would argue that the implicit search will determine DSL, which will in turn determine Host, so inferring Host before the search is wrong!

With the last fix, we fix the incompatibility "by accident". Since Host does not appear anywhere in the type of the implicit method, it will be instantiated. By contrast, if the result type of the method contained Host in invariant position, it would not be instantiated, and the implicit would not be found.

I realize it is deeply unsatisfactory that the implicit scope depends on such subtleties of type inference. Maybe the right answer is to go the other way: Instance types of type variables never contribute to the implicit scope, only their bounds do. Then i1857.scala (and I suspect quite a few others) would give compile errors. We'd need to be sure we can catch them with @olafurpg 's scalafix.

WDYT?

[smarter]

Maybe the right answer is to go the other way: Instance types of type variables never contribute to the implicit scope, only their bounds do. Then i1857.scala (and I suspect quite a few others) would give compile errors. We'd need to be sure we can catch them with @olafurpg 's scalafix.

I would advise against going in this direction, this sound similar to what we had a year and a half ago which lead to issues when using the standard library like #553 and more generally #739. Being able to add the implicits to the code using scalafix would help, but it would be deeply unsatisfying if we can't get implicit search to work in some simple uses of the standard library.

I like the current approach because it is more flexible than what scalac does but in most common situations will produce the same result.

Should Host be always inferred before doing the implicit search? I actually doubt that. In fact, I would argue that the implicit search will determine DSL, which will in turn determine Host, so inferring Host before the search is wrong!

I would say that yes, Host should be instantiated before doing the implicit search because:

1. It has been constrained by a previous parameter list
2. It is referred to indirectly by the type DSL (since DSL <: CommandeerDSL[Host])

So we just need a way to collect all type variables which are referred to indirectly, and we already almost have this in the fixed point computation of variances: https://github.com/lampepfl/dotty/blob/119725799675ee00d6d3374771765b88e4de67bc/compiler/src/dotty/tools/dotc/typer/Inferencing.scala#L334-L335

I think we could replace IsFullyDefinedAccumulator by a call to variances and some processing of the result, this might require some tweaking of variances.

[smarter]

I did not manage to unify IsFullyDefinedAccumulator and variances but here's an alternative fix that shows what I had in mind, let me know what you think: #1861

[odersky]

I think #1861 is more promising than this one, so let's close it. I did not see how #739 would have been invalidated by the fix but #553 certainly does become invalidated. I still don't like the tight intergration between type variable integration and implicit scope but maybe that boat has already sailed.