Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit nesting rules breaks usage of some existing libraries #7069

Open
smarter opened this issue Aug 20, 2019 · 2 comments
Open

Implicit nesting rules breaks usage of some existing libraries #7069

smarter opened this issue Aug 20, 2019 · 2 comments

Comments

@smarter
Copy link
Member

smarter commented Aug 20, 2019

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
Copy link
Member Author

smarter commented Aug 20, 2019

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
Copy link
Member Author

smarter commented Aug 21, 2019

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

  • Take the arity of the member we're selecting into account when filtering matching candidates (might break existing code that uses auto-tupling)
  • Consider top-level imports to have the same nesting level as Predef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant