scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.8k stars 1.04k forks source link

Implicit nesting rules breaks usage of some existing libraries #7069

Open smarter opened 5 years ago

smarter commented 5 years ago

Given:

trait HasApply {
  def apply(a: Int, b: String): Int = a
}
object HasApply {
  implicit def foo[T](x: T): HasApply = ???
}

import HasApply._

object Test {
  val str = "abc"
  str.apply(0)
}

Dotty by default will complain:

-- Error: try/impany.scala:12:5 ------------------------------------------------
12 |  str(0)
   |  ^^^^^^
   |missing argument for parameter b of method apply: (a: Int, b: String): Int

On the other hand, if we run with -language:Scala2, then Dotty will find Predef.augmentString, like Scala 2 does, because of this check: https://github.com/lampepfl/dotty/blob/1f22a403680094659f4013811b971e74b1b32160/compiler/src/dotty/tools/dotc/typer/Implicits.scala#L270

It would be nicer if we were able to select the correct implicit conversion without using the Scala 2 mode, e.g. because the type of the argument of augmentString is more precise than the type of the argument of foo, or because the apply method we want to call only takes one parameter of type Int which does not match the signature of HasApply#apply. Otherwise, some Scala 2 libraries which (ab)use implicit conversions become hard to use from Dotty because of definitions such as:

https://github.com/typelevel/scalacheck/blob/3fc537dde9d8fdf951503a8d8b027a568d52d055/src/main/scala/org/scalacheck/util/Pretty.scala#L109

https://github.com/typelevel/scalacheck/blob/3fc537dde9d8fdf951503a8d8b027a568d52d055/src/main/scala/org/scalacheck/Gen.scala#L431-L432

Simply removing these implicit conversions is usually not an option, because it would likely break a lot of code that relies on it, with no easy migration strategy.

WDYT @odersky ?

smarter commented 5 years ago

To give a more concrete proposal, right now the logic is https://github.com/lampepfl/dotty/blob/6f6751d5eb7f4b667f499f6326970d55b0ae62af/compiler/src/dotty/tools/dotc/typer/Implicits.scala#L1444-L1450

So if we have two matching candidates at different levels, we always prefer the more nested one. Instead, we could do:

def compareCandidate(prev: SearchSuccess, ref: TermRef, level: Int): Int =
  if (prev.ref eq ref) 0
  else {
    val specificity = nestedContext().test(compare(prev.ref, ref))
    if (specificity == 0)
      prev.level - level
    else
      specificity
  }

So, only use the nesting level as a tie-break when the candidates are equally specific. This algorithm has two interesting properties:

  1. It matches how Scala 2 works more often
  2. It still resolves the motivating example for nesting level from http://dotty.epfl.ch/docs/reference/changed-features/implicit-resolution.html correctly:
def f(implicit i: C) = {
  def g(implicit j: C) = {
    implicitly[C] // picks j since i and j are equally specific.
  }
}
smarter commented 5 years ago

Discussed this with Martin and we came up with two possible mitigations that wouldn't require changing the way we handle nesting: