scala / scala3

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

False positive warning for new implicit resolution rules in 3.5.0-RC3 #21036

Closed lenguyenthanh closed 2 months ago

lenguyenthanh commented 3 months ago

Compiler version

3.5.0-RC3

Minimized code

//> using scala 3.5.0-RC3
// //> using options -source:3.6-migration

trait SameRuntime[A, B]
trait BSONWriter[T]
trait BSONHandler[T] extends BSONWriter[T]

opaque type Id = String
object Id:
  given SameRuntime[Id, String] = ???

given BSONHandler[String]                    = ???
given [T: BSONHandler]: BSONHandler[List[T]] = ???

given opaqueWriter[T, A](using
    rs: SameRuntime[T, A],
    writer: BSONWriter[A]
): BSONWriter[T] = ???

def write[A](a: A)(using writer: BSONWriter[A]): Unit = ???

val ids: List[Id] = ???

val x = write(ids)

Output

[warn] ./ImplicitResolution2.scala:24:19
[warn] Given search preference for BSONWriter[List[Id]] between alternatives (given_BSONHandler_List :
[warn]   [T](implicit evidence$1: BSONHandler[T]): BSONHandler[List[T]]) and (opaqueWriter :
[warn]   [T, A](using rs: SameRuntime[T, A], writer: BSONWriter[A]): BSONWriter[T]) will change
[warn] Current choice           : the first alternative
[warn] New choice from Scala 3.6: none - it's ambiguous
[warn] val x = write(ids)
[warn]                   ^
Compiled project (Scala 3.5.0-RC3, JVM (21))

Expectation

It shouldn't emit any warnings, as there is no implicit evidence for the second case. Also if we enabled //> using options -source:3.6-migration it works perfectly.

Related discussions: https://github.com/scala/scala3/pull/19300 and https://github.com/scala/scala3/issues/20484

lenguyenthanh commented 3 months ago

trying to minimize a bit further and found out this desn't emit false warning: oops, it actually does emit warning I summoned the wrong type 🤦

//> using scala 3.5.0-RC3

trait SameRuntime[A, B]
trait BSONWriter[T]
trait BSONHandler[T] extends BSONWriter[T]

opaque type Id = String
object Id:
  given SameRuntime[Id, String] = ???

given BSONHandler[String]                    = ???
given [T: BSONHandler]: BSONHandler[List[T]] = ???

given opaqueWriter[T, A](using rs: SameRuntime[T, A], writer: BSONWriter[A]): BSONWriter[T] = ???

val x = summon[BSONHandler[List[Id]]] // this doesn't emit warning
val y = summon[BSONWriter[List[Id]]] // this does emit warning
Gedochao commented 3 months ago

The reproduction from the first post emits the warning on current main (f2829c3fab28cc6ab47a5627abda855884476572), 3.5.0-RC3 and 3.5.0-RC2. 3.5.0-RC1 and earlier versions are unaffected (no warning is emitted).

Gedochao commented 3 months ago

cc @odersky @hamzaremmal @WojciechMazur

WojciechMazur commented 3 months ago

Bisect points to https://github.com/scala/scala3/commit/bc26c5176a42281b6621550237818c91e7acdecd so it's basically since the introduction of #19300 before 3.5.0-RC2 it was hidden, because compiler used -source:3.4 as default one. The issue exists only with -source:3.5 which is default since 3.5.0-RC2. Current main uses -source:3.6

odersky commented 3 months ago

Isn't that a minimization of one of the known failures in reactive mongo. In that case the warning would be correct: implicit resolution will change. I'll verify that this is indeed the case.

lenguyenthanh commented 3 months ago

Isn't that a minimization of one of the known failures in reactive mongo. In that case the warning would be correct: implicit resolution will change. I'll verify that this is indeed the case.

No, it's not the case, If I compile this with //> using options -source:3.6-migration, it works perfectly fine. And by using //> using options -Xprint:typer, I can verify it resolves exactly the same result with and without //> using options -source:3.6-migration.

EugeneFlesselle commented 3 months ago

Minimization:

//> using options -source:3.5

given Int = 1

trait A[T]
trait B[T] extends A[T]

given b[T](using Int): B[List[T]] = ???
given a[T](using String): A[T] = ???

val x = summon[B[List[String]]] // no warning
val y = summon[A[List[String]]] // warning

Note the order of declarations of a and b is relevant. If b is removed, then y yields a no given was found error.

odersky commented 3 months ago

I think I found it. We call compareAlternatives in disambiguate, to drop more pending candidates. In that case, we should issue a warning only if the pending candidate is in the final result, not the candidate that was found so far.

som-snytt commented 3 months ago

also https://github.com/scala/scala3/issues/20572

OndrejSpanel commented 2 months ago

For the record: I have experienced this warning with RC3 in my project using borer library. Same as here, the warning disappeared when using -source:3.6-migration, it also disappeared in RC4.

There were many warnings with RC3, some contained implicit evidence in both alternatives, some not:

[warn]     |Given search preference for io.bullet.borer.Encoder[List[XXXXX]] between alternatives (io.bullet.borer.Encoder.forLinearSeq :
[warn]     |  [T, M[X] <: scala.collection.LinearSeq[X]]
[warn]     |    (implicit evidence$5: io.bullet.borer.Encoder[T]):
[warn]     |      io.bullet.borer.Encoder.DefaultValueAware[M[T]]
[warn]     |) and (io.bullet.borer.Encoder.given_Encoder_T :
[warn]     |  [T](using ev: io.bullet.borer.Codec.All[T]): io.bullet.borer.Encoder[T]) will change
[warn]     |Current choice           : the first alternative
[warn]     |New choice from Scala 3.6: none - it's ambiguous
[warn]    |Given search preference for io.bullet.borer.Decoder[Option[Double]] between alternatives (io.bullet.borer.Decoder.forOption :
[warn]    |  [T]
[warn]    |    (implicit evidence$4: io.bullet.borer.Decoder[T]):
[warn]    |      io.bullet.borer.Decoder.DefaultValueAware[Option[T]]
[warn]    |) and (io.bullet.borer.Decoder.fromFactory :
[warn]    |  [T, M[_$11]]
[warn]    |    (implicit evidence$5: io.bullet.borer.Decoder[T], factory:
[warn]    |      scala.collection.Factory[T, M[T]]): io.bullet.borer.Decoder[M[T]]
[warn]    |) will change
[warn]    |Current choice           : the first alternative
[warn]    |New choice from Scala 3.6: none - it's ambiguous

Unless proven otherwise, I assume all those warnings were false, caused by this RC3 issue.

odersky commented 2 months ago

If the warning disappeared in RC4, it definitely was a false positive.