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 resolution (incorrectly?) picks import #8092

Closed
som-snytt opened this issue Jan 26, 2020 · 8 comments
Closed

Implicit resolution (incorrectly?) picks import #8092

som-snytt opened this issue Jan 26, 2020 · 8 comments

Comments

@som-snytt
Copy link
Contributor

minimized code

This originated as scala/bug#11860

trait T[A]
object T {
  implicit val `default for string`: T[String] = new T[String] {}
}

object Penumbra {
  implicit val ours: T[String] = new T[String] {}
}

object Locally {
  import Penumbra._
  def verify(x: AnyRef) = assert(x eq ours)
  def dotty(x: AnyRef) = assert(x eq Penumbra.ours)
  implicit val ours: T[String] = new T[String] {}
  def test(): Unit = {
    val tested = implicitly[T[String]]   // fails
    //verify(tested)
    //verify(implicitly[T[String]])        // fails
    dotty(tested)
    dotty(implicitly[T[String]])
  }
}

object Memberly {
  import Penumbra._
  def verify(x: AnyRef) = assert(x eq ours)
  def dotty(x: AnyRef) = assert(x eq Penumbra.ours)
  implicit val ours: T[String] = new T[String] {}
  val tested = implicitly[T[String]] // correct
  //verify(tested)                     // OK in scala 2
  //verify(implicitly[T[String]])      // fails
  dotty(tested)
  dotty(implicitly[T[String]])
  def test(): Unit = ()
}

object Test extends App {
  Locally.test()
  Memberly.test()
}
Compilation output

Dotty picks Penumbra.ours in all cases.

Scala 2 incorrectly decides ours is shadowed and picks the default in T, from implicit scope, except for one case, marked OK, which happens to work as expected.

It's not obvious from the docs whether Dotty intends for the imported Penumbra.ours to be implicitly available, or whether shadowing nullifies; or which rule picks it; source file order doesn't matter; changing the identifier to avoid shadowing doesn't matter. Moving the import to outer scope does make it pick Locally.ours, by scope nesting rule, perhaps.

expectation

Locally.ours and Memberly.ours should be preferred.

@odersky
Copy link
Contributor

odersky commented Jan 29, 2020

The issue is that ContextualImplicits are stacked in the order they are encountered. So wehn we first enter a class, we record all its implicit members and then if we find an import all its imports. By the nesting criterion, this means that imports in a class shadow members of that class. For regular members, local definitions shadow imports which in turn shadow inherited members. But for implicit scopes we do not distinguish between locally defined and inherited, so something has to give. I believe the current design is defensible and I see no obvious change that would improve it.

@bjornregnell
Copy link
Contributor

@odersky Thanks for the explanation! But I'm still having difficulties understanding this; if givens are "stacked" then when they are popped from the stack the last one should come first ?? -- otherwise it's seems to be a queue and not a stack... A bit more minimal code than @som-snytt gave with just this phenomena shown, demonstrating a behavior that's counter-intuitive to me (but I have obviously not been able to grok the priority rules in search of givens):

Welcome to Scala 3.1.0 (11.0.11, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> object X:
     |   given Int = 42
     | 
// defined object X

scala> def a = 
     |   import X.given Int
     |   given Int = 43
     |   summon[Int]
     | 
def a: Int

scala> a
val res0: Int = 42   // surprise :o

I think (perhaps naïvely, without understanding the full picture) that this should give 43 or perhaps a compile error stating that givens are ambiguous, but instead the compiler picks a far away imported given and not the closest to the summoning.

@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 22, 2021

Just to update the syntax, which I am liable to forget. I see the original issue was a Scala 2 bug. The purpose of the example was to show that the behavior is the same for template member or local block. (The import is preferred.) (Edit: updated with Bjorn's example and other permutations suggested on the forums.)

package dot8092

import compiletime.testing.typeChecks

trait T[A]
object T:
  given T[String] with { override def toString = "T" }

object Penumbra:
  given T[String] with { override def toString = "Penumbra" }

object Locally:
  def test(): Unit =
    import Penumbra.given
    given T[String] with { override def toString = "Locally" }
    println(summon[T[String]])

object LocallyCompeting:
  def test(): Unit =
    import Penumbra.given
    println(summon[T[String]])
    import T.given
    println(summon[T[String]])

object MemberlyCompeting:
  import Penumbra.given
  println(summon[T[String]])
  import T.given
  println(summon[T[String]])
  def test(): Unit = ()

object Memberly:
  import Penumbra.given
  given T[String] with { override def toString = "Memberly" }
  def test(): Unit = println(summon[T[String]])

object X:
  given x: T[String] with { override def toString = "X" }

object X2:
  given x: T[String] with { override def toString = "X2" }

object Y:
  given y: T[String] with { override def toString = "Y" }

object SameNamesCompeting:
  def test(): Unit =
    import X.given
    println(summon[T[String]])
    println(s"explicit x $x")
    import X2.given
    println(summon[T[String]])
    println(s"can I refer to `x` after import given from X2? ${typeChecks("x")}")

object SameNamesNonCompeting:
  def test(): Unit =
    import X.given
    println(summon[T[String]])
    val x: T[String] = null.asInstanceOf[T[String]]
    println(summon[T[String]])

object DifferentNamesCompeting:
  def test(): Unit =
    import X.given
    println(summon[T[String]])
    import Y.given
    println(s"can I summon[T[String]] after import from Y? ${typeChecks("summon[T[String]])")}")

@main
def test() =
  println("> import competing with imported given")
  Locally.test()
  println("> import competing with given member")
  Memberly.test()
  println("> competing local imports")
  LocallyCompeting.test()
  println("> competing imports in object lexical scope")
  MemberlyCompeting.test()
  println("> competing imports with same name")
  SameNamesCompeting.test()
  println("> competing imports with different name")
  DifferentNamesCompeting.test()
  println("> import noncompeting with nonimplicit with same name")
  SameNamesNonCompeting.test()

For posterity

➜  scala i8092.scala
> import competing with imported given
Penumbra
> import competing with given member
Penumbra
> competing local imports
Penumbra
T
> competing imports in object lexical scope
Penumbra
T
> competing imports with same name
X
explicit x X
X2
can I refer to `x` after import given from X2? false
> competing imports with different name
X
can I summon[T[String]] after import from Y? false
> import noncompeting with nonimplicit with same name
X
X

@bjornregnell
Copy link
Contributor

bjornregnell commented Nov 22, 2021

For later thread-reader's convenience: Actual output in 3.1.0 REPL if pasted without the @main is

scala> test()
Penumbra
Penumbra

But what is expected output? I guess first Locally and then Memberly

@bjornregnell
Copy link
Contributor

For later thread-reader's convenience: I just looked up the by me never heard of word Penumbra and it means HalfShadow. (So spot on naming by @som-snytt )

@som-snytt
Copy link
Contributor Author

Also relevant for the recent historic lunar eclipse, which I forgot to wake up at midnight for.

So I assume shadowing of anonymous given names is not relevant in Scala 3 implicit resolution, but at issue is just how "nesting level" is understood. In Scala 2, an import introduces a level, but I guess not in Scala 3, see the linked ticket for documentation.

@som-snytt
Copy link
Contributor Author

In Scala 3.4, the expectation is now reversed.

@som-snytt
Copy link
Contributor Author

to recap, with today's 3.6.4-RC1-bin-SNAPSHOT, as noted for 3.4, the first two results changed, thankfully:

> import competing with imported given
Locally
> import competing with given member
Memberly
> competing local imports
Penumbra
T
> competing imports in object lexical scope
Penumbra
T
> competing imports with same name
X
explicit x X
X2
can I refer to `x` after import given from X2? false
> competing imports with different name
X
can I summon[T[String]] after import from Y? false
> import noncompeting with nonimplicit with same name
X
X

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

3 participants