From 650f854ce0325b0090d0e8da10d53d4456f8d213 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 13 Jul 2023 18:53:17 +0100 Subject: [PATCH 1/4] Select local implicits over name-imported over wildcard imported --- .../dotty/tools/dotc/typer/Implicits.scala | 17 ++++++-- tests/run/i18183.scala | 41 +++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 tests/run/i18183.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 5ae40421add2..394246153700 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -299,7 +299,7 @@ object Implicits: class ContextualImplicits( val refs: List[ImplicitRef], val outerImplicits: ContextualImplicits | Null, - isImport: Boolean)(initctx: Context) extends ImplicitRefs(initctx) { + val isImport: Boolean)(initctx: Context) extends ImplicitRefs(initctx) { private val eligibleCache = EqHashMap[Type, List[Candidate]]() /** The level increases if current context has a different owner or scope than @@ -330,8 +330,19 @@ object Implicits: if ownEligible.isEmpty then outerEligible else if outerEligible.isEmpty then ownEligible else - val shadowed = ownEligible.map(_.ref.implicitName).toSet - ownEligible ::: outerEligible.filterConserve(cand => !shadowed.contains(cand.ref.implicitName)) + def filter(xs: List[Candidate], remove: List[Candidate]) = + val shadowed = remove.map(_.ref.implicitName).toSet + xs.filterConserve(cand => !shadowed.contains(cand.ref.implicitName)) + def isWildcardImport(using Context) = ctx.importInfo.nn.isWildcardImport + if (irefCtx.scope eq irefCtx.outer.scope) && ( + isImport && !outerImplicits.nn.isImport + || isWildcardImport && !isWildcardImport(using outerImplicits.nn.irefCtx) + ) then + // special cases: definitions beat imports, and named imports beat + // wildcard imports, provided both are in contexts with same scope + filter(ownEligible, outerEligible) ::: outerEligible + else + ownEligible ::: filter(outerEligible, ownEligible) def uncachedEligible(tp: Type)(using Context): List[Candidate] = Stats.record("uncached eligible") diff --git a/tests/run/i18183.scala b/tests/run/i18183.scala new file mode 100644 index 000000000000..99b83d7f48c7 --- /dev/null +++ b/tests/run/i18183.scala @@ -0,0 +1,41 @@ +case class Foo(n: Int) + +class Bar(n: Int): + implicit val foo: Foo = new Foo(n) + +class InMethod: + def wild(bar: Bar): Unit = + import bar._ + implicit val foo: Foo = new Foo(2) + assert(foo eq implicitly[Foo]) + + def named(bar: Bar): Unit = + import bar.foo + implicit val foo: Foo = new Foo(2) + assert(foo eq implicitly[Foo]) + + def namedWild(bar: Bar): Unit = + val bar2 = new Bar(2) + import bar.foo + import bar2._ + assert(bar.foo eq implicitly[Foo]) + + def wildNamed(bar: Bar): Unit = + val bar2 = new Bar(2) + import bar2._ + import bar.foo + assert(bar.foo eq implicitly[Foo]) + +class InClass(bar: Bar): + import bar._ + implicit val foo: Foo = new Foo(2) + assert(foo eq implicitly[Foo]) + +object Test: + def main(args: Array[String]): Unit = + val bar = new Bar(1) + new InMethod().wild(bar) + new InMethod().named(bar) + new InMethod().namedWild(bar) + new InMethod().wildNamed(bar) + new InClass(bar) From 1892d65b06d3b36309c430a13b66921a8d791235 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 14 Jul 2023 10:33:53 +0100 Subject: [PATCH 2/4] More tests, particular compared to given --- tests/run/i18183.given.scala | 93 +++++++++++++++++++++++ tests/run/i18183.mixed.scala | 141 +++++++++++++++++++++++++++++++++++ tests/run/i18183.scala | 28 ++++--- 3 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 tests/run/i18183.given.scala create mode 100644 tests/run/i18183.mixed.scala diff --git a/tests/run/i18183.given.scala b/tests/run/i18183.given.scala new file mode 100644 index 000000000000..fd8415d383b7 --- /dev/null +++ b/tests/run/i18183.given.scala @@ -0,0 +1,93 @@ +case class Foo(n: Int) + +class Bar(n: Int): + given foo: Foo = new Foo(n) + +class InMethod: + def wild(bar: Bar): Unit = + import bar.* + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def givenWild(bar: Bar): Unit = + import bar.{ given, * } + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def givn(bar: Bar): Unit = + import bar.given + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def givenFoo(bar: Bar): Unit = + import bar.given Foo + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def named(bar: Bar): Unit = + import bar.foo + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def namedGivenWild(bar: Bar, bar2: Bar): Unit = + import bar.foo, bar2.{ given, * } + assert(bar.foo eq summon[Foo]) + + def givenWildNamed(bar: Bar, bar2: Bar): Unit = + import bar2.{ given, * }, bar.foo + assert(bar.foo eq summon[Foo]) + + def namedWild(bar: Bar, bar2: Bar): Unit = + import bar.foo, bar2.* + assert(bar.foo eq summon[Foo]) + + def wildNamed(bar: Bar, bar2: Bar): Unit = + import bar2.*, bar.foo + assert(bar.foo eq summon[Foo]) + +class InClassWild(bar: Bar): + import bar.* + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class InClassGivenWild(bar: Bar): + import bar.{ given, * } + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class InClassGiven(bar: Bar): + import bar.given + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class InClassGivenFoo(bar: Bar): + import bar.given Foo + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class InClassNamed(bar: Bar): + import bar.foo + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +object Test: + def main(args: Array[String]): Unit = + val bar = new Bar(1) + val bar2 = new Bar(2) + + new InMethod().wild(bar) + new InMethod().givenWild(bar) // was: error + new InMethod().givn(bar) // was: error + new InMethod().givenFoo(bar) // was: error + new InMethod().named(bar) // was: error + + new InMethod().namedWild(bar, bar2) + new InMethod().wildNamed(bar, bar2) + new InMethod().namedGivenWild(bar, bar2) // was: error + new InMethod().givenWildNamed(bar, bar2) + + new InClassWild(bar) + new InClassGivenWild(bar) // was: error + new InClassGiven(bar) // was: error + new InClassGivenFoo(bar) // was: error + new InClassNamed(bar) // was: error diff --git a/tests/run/i18183.mixed.scala b/tests/run/i18183.mixed.scala new file mode 100644 index 000000000000..131e73f098de --- /dev/null +++ b/tests/run/i18183.mixed.scala @@ -0,0 +1,141 @@ +case class Foo(n: Int) + +class OldBar(n: Int): + implicit val foo: Foo = new Foo(n) + +class NewBar(n: Int): + given foo: Foo = new Foo(n) + +class OldInMethod: + def wild(bar: OldBar): Unit = + import bar.* + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def named(bar: OldBar): Unit = + import bar.foo + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def namedWild(bar: OldBar, bar2: NewBar): Unit = + import bar.foo, bar2.* + assert(bar.foo eq summon[Foo]) + + def wildNamed(bar: OldBar, bar2: NewBar): Unit = + import bar2.*, bar.foo + assert(bar.foo eq summon[Foo]) + + def namedGivenWild(bar: OldBar, bar2: NewBar): Unit = + import bar.foo + import bar2.{ given, * } + assert(bar.foo eq summon[Foo]) + + def givenWildNamed(bar: OldBar, bar2: NewBar): Unit = + import bar2.{ given, * }, bar.foo + assert(bar.foo eq summon[Foo]) + +class OldInClassWild(bar: OldBar): + import bar.* + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class OldInClassNamed(bar: OldBar): + import bar.foo + given foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + +class NewInMethod: + def givenWild(bar: NewBar): Unit = + import bar.{ given, * } + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def wild(bar: NewBar): Unit = + import bar.* + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def givn(bar: NewBar): Unit = + import bar.given + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def givenFoo(bar: NewBar): Unit = + import bar.given Foo + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def named(bar: NewBar): Unit = + import bar.foo + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + def namedWild(bar: NewBar, bar2: OldBar): Unit = + import bar.foo, bar2.* + assert(bar.foo eq summon[Foo]) + + def wildNamed(bar: NewBar, bar2: OldBar): Unit = + import bar2.*, bar.foo + assert(bar.foo eq summon[Foo]) + +class NewInClassGivenWild(bar: NewBar): + import bar.{ given, * } + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class NewInClassWild(bar: NewBar): + import bar.* + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class NewInClassGiven(bar: NewBar): + import bar.given + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class NewInClassGivenFoo(bar: NewBar): + import bar.given Foo + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + +class NewInClassNamed(bar: NewBar): + import bar.foo + implicit val foo: Foo = new Foo(2) + assert(foo eq summon[Foo]) + + +object Test: + def main(args: Array[String]): Unit = + val oldBar = new OldBar(1) + val newBar = new NewBar(1) + val oldBar2 = new OldBar(2) + val newBar2 = new NewBar(2) + + + new OldInMethod().wild(oldBar) // was: error + new OldInMethod().named(oldBar) // was: error + + new OldInMethod().namedWild(oldBar, newBar2) + new OldInMethod().wildNamed(oldBar, newBar2) + new OldInMethod().namedGivenWild(oldBar, newBar2) // was: error + new OldInMethod().givenWildNamed(oldBar, newBar2) + + new OldInClassWild(oldBar) // was: error + new OldInClassNamed(oldBar) // was: error + + + new NewInMethod().wild(newBar) + new NewInMethod().givenWild(newBar) // was: error + new NewInMethod().givn(newBar) // was: error + new NewInMethod().givenFoo(newBar) // was: error + new NewInMethod().named(newBar) // was: error + + new NewInMethod().namedWild(newBar, oldBar2) // was: error + new NewInMethod().wildNamed(newBar, oldBar2) + + new NewInClassWild(newBar) + new NewInClassGivenWild(newBar) // was: error + new NewInClassGiven(newBar) // was: error + new NewInClassGivenFoo(newBar) // was: error + new NewInClassNamed(newBar) // was: error diff --git a/tests/run/i18183.scala b/tests/run/i18183.scala index 99b83d7f48c7..b5279db35a94 100644 --- a/tests/run/i18183.scala +++ b/tests/run/i18183.scala @@ -14,28 +14,36 @@ class InMethod: implicit val foo: Foo = new Foo(2) assert(foo eq implicitly[Foo]) - def namedWild(bar: Bar): Unit = - val bar2 = new Bar(2) + def namedWild(bar: Bar, bar2: Bar): Unit = import bar.foo import bar2._ assert(bar.foo eq implicitly[Foo]) - def wildNamed(bar: Bar): Unit = - val bar2 = new Bar(2) + def wildNamed(bar: Bar, bar2: Bar): Unit = import bar2._ import bar.foo assert(bar.foo eq implicitly[Foo]) -class InClass(bar: Bar): +class InClassWild(bar: Bar): import bar._ implicit val foo: Foo = new Foo(2) assert(foo eq implicitly[Foo]) +class InClassNamed(bar: Bar): + import bar.foo + implicit val foo: Foo = new Foo(2) + assert(foo eq implicitly[Foo]) + object Test: def main(args: Array[String]): Unit = val bar = new Bar(1) - new InMethod().wild(bar) - new InMethod().named(bar) - new InMethod().namedWild(bar) - new InMethod().wildNamed(bar) - new InClass(bar) + val bar2 = new Bar(2) + + new InMethod().wild(bar) // was: error + new InMethod().named(bar) // was: error + + new InMethod().namedWild(bar, bar2) // was: error + new InMethod().wildNamed(bar, bar2) + + new InClassWild(bar) // was: error + new InClassNamed(bar) // was: error From 463ee8cb1f0636b0d8f3c5ef9c3d88b8551e0828 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sat, 15 Jul 2023 13:07:06 +0100 Subject: [PATCH 3/4] Reproduce and fix error that affected findRef --- .../src/dotty/tools/dotc/typer/Implicits.scala | 10 ++++++---- tests/run/i18183.findRef.scala | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 tests/run/i18183.findRef.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 394246153700..051b83be8bf0 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -333,11 +333,13 @@ object Implicits: def filter(xs: List[Candidate], remove: List[Candidate]) = val shadowed = remove.map(_.ref.implicitName).toSet xs.filterConserve(cand => !shadowed.contains(cand.ref.implicitName)) + + val outer = outerImplicits.uncheckedNN def isWildcardImport(using Context) = ctx.importInfo.nn.isWildcardImport - if (irefCtx.scope eq irefCtx.outer.scope) && ( - isImport && !outerImplicits.nn.isImport - || isWildcardImport && !isWildcardImport(using outerImplicits.nn.irefCtx) - ) then + def preferDefinitions = isImport && !outer.isImport + def preferNamedImport = isWildcardImport && !isWildcardImport(using outer.irefCtx) + + if level == outer.level && (preferDefinitions || preferNamedImport) then // special cases: definitions beat imports, and named imports beat // wildcard imports, provided both are in contexts with same scope filter(ownEligible, outerEligible) ::: outerEligible diff --git a/tests/run/i18183.findRef.scala b/tests/run/i18183.findRef.scala new file mode 100644 index 000000000000..058f793e4ac1 --- /dev/null +++ b/tests/run/i18183.findRef.scala @@ -0,0 +1,17 @@ +// A minimised reproduction of how an initial change to combineEligibles broke Typer#findRef +case class Foo(n: Int) + +class Test: + import this.toString + + val foo1 = Foo(1) + val foo2 = Foo(2) + + def foo(using Foo): Foo = + import this.* + def bar(using Foo): Foo = summon[Foo] + bar(using foo2) + +object Test extends Test: + def main(args: Array[String]): Unit = + assert(foo(using foo1) eq foo2) From ce9e6d6d0705b0435901ff19b2034ff3ac1c9812 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 18 Jul 2023 16:15:52 +0100 Subject: [PATCH 4/4] Reproduce and fix error that affected http4s --- .../dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/pos/i18183.migration.scala | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 tests/pos/i18183.migration.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 051b83be8bf0..78452122df11 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -339,7 +339,7 @@ object Implicits: def preferDefinitions = isImport && !outer.isImport def preferNamedImport = isWildcardImport && !isWildcardImport(using outer.irefCtx) - if level == outer.level && (preferDefinitions || preferNamedImport) then + if !migrateTo3(using irefCtx) && level == outer.level && (preferDefinitions || preferNamedImport) then // special cases: definitions beat imports, and named imports beat // wildcard imports, provided both are in contexts with same scope filter(ownEligible, outerEligible) ::: outerEligible diff --git a/tests/pos/i18183.migration.scala b/tests/pos/i18183.migration.scala new file mode 100644 index 000000000000..8a916ff17794 --- /dev/null +++ b/tests/pos/i18183.migration.scala @@ -0,0 +1,38 @@ +// scalac: -source:3.0-migration + +// A not-fully-minimal reproduction of the CI failure in http4s +// While implementing the fix to name "shadowing" in implicit lookup. + +import scala.util.control.NoStackTrace + +final case class EitherT[F[_], A, B](value: F[Either[A, B]]) { + def semiflatMap[D](f: B => F[D])(implicit F: Monad[F]): EitherT[F, A, D] = ??? +} + +trait Applicative[F[_]] { + def pure[A](x: A): F[A] +} +trait Monad[F[_]] extends Applicative[F] +trait Async[F[_]] extends Monad[F] + +final class Request[+F[_]] + +final case class RequestCookie(name: String, content: String) + +final class CSRF2[F[_], G[_]](implicit F: Async[F]) { self => + import CSRF2._ + + def signToken[M[_]](rawToken: String)(implicit F: Async[M]): M[CSRFToken] = ??? + + def refreshedToken[M[_]](implicit F: Async[M]): EitherT[M, CSRFCheckFailed, CSRFToken] = + EitherT(extractRaw("")).semiflatMap(signToken[M]) + + def extractRaw[M[_]: Async](rawToken: String): M[Either[CSRFCheckFailed, String]] = ??? +} + +object CSRF2 { + type CSRFToken + + case object CSRFCheckFailed extends Exception("CSRF Check failed") with NoStackTrace + type CSRFCheckFailed = CSRFCheckFailed.type +}