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

Overloading or implicit bug in 3.4.1 #20053

Open
soronpo opened this issue Mar 29, 2024 · 9 comments · May be fixed by #20054
Open

Overloading or implicit bug in 3.4.1 #20053

soronpo opened this issue Mar 29, 2024 · 9 comments · May be fixed by #20054
Assignees
Labels
area:overloading itype:bug needs-minor-release This PR cannot be merged until the next minor release

Comments

@soronpo
Copy link
Contributor

soronpo commented Mar 29, 2024

This regression in 3.4.1 first caused me to chase down the wrong rabbit hole of #19955, so it could be related somehow, but I doubt it.
This is a very weird bug, that can disappear if we remove one of the following:

  • Remove the def ^^^ : Unit = ???
  • Remove the (using check: c.OutW =:= c.OutW) from the third def ^^^ declaration
    The third ^^^ definition should not be considered for overloading at all because the first one is the more specific.

The error also disappear if we split x ^^^ x ^^^ x into two separate vals.

Compiler version

Last good release: 3.4.1-RC1-bin-20240126-1716bcd-NIGHTLY
First bad release: 3.4.1-RC1-bin-20240129-b20747d-NIGHTLY
bisect: cce2933 in #19096

Minimized code

trait Summon[R, T <: R]:
  type Out
object Summon:
  given [R, T <: R]: Summon[R, T] with
    type Out = R

sealed class Modifier[+A, +P]
type ModifierAny = Modifier[Any, Any]
sealed trait ISCONST[T <: Boolean]
type CONST = ISCONST[true]

trait DFTypeAny
trait DFBits[W <: Int] extends DFTypeAny
trait DFVal[+T <: DFTypeAny, +M <: ModifierAny]
type DFValAny = DFVal[DFTypeAny, ModifierAny]
type DFValTP[+T <: DFTypeAny, +P] = DFVal[T, Modifier[Any, P]]
type DFConstOf[+T <: DFTypeAny] = DFVal[T, Modifier[Any, CONST]]

trait Candidate[R]:
  type OutW <: Int
  type OutP
object Candidate:
  given [W <: Int, P, R <: DFValTP[DFBits[W], P]]: Candidate[R] with
    type OutW = W
    type OutP = P

extension [L <: DFValAny](lhs: L)(using icL: Candidate[L])
  def ^^^[R](rhs: R)(using
      icR: Candidate[R]
  ): DFValTP[DFBits[icL.OutW], icL.OutP | icR.OutP] = ???
  def ^^^ : Unit = ???
extension [L](lhs: L)
  def ^^^[RW <: Int, RP](
      rhs: DFValTP[DFBits[RW], RP]
  )(using es: Summon[L, lhs.type])(using
      c: Candidate[L]
  )(using check: c.OutW =:= c.OutW): DFValTP[DFBits[c.OutW], c.OutP | RP] = ???

val x: DFConstOf[DFBits[8]] = ???
val zzz = x ^^^ x ^^^ x

Output

   |val zzz = x ^^^ x ^^^ x
   |                       ^
   |No given instance of type Summon[DFValTP[DFBits[?1.OutW], ?2.OutP | ISCONST[(true : Boolean)]],
   |  (?3 : DFValTP[DFBits[?1.OutW], ?2.OutP | ISCONST[(true : Boolean)]])] was found for parameter es of method ^^^.
   |I found:
   |
   |    Summon.given_Summon_R_T[R, T]
   |
   |But given instance given_Summon_R_T in object Summon does not match type Summon[DFValTP[DFBits[?1.OutW], ?2.OutP | ISCONST[(true : Boolean)]],
   |  (?3 : DFValTP[DFBits[?1.OutW], ?2.OutP | ISCONST[(true : Boolean)]])]
   |
   |where:    ?1 is an unknown value of type Candidate.given_Candidate_R[(8 : Int), ISCONST[(true : Boolean)],
   |  DFVal[DFBits[(8 : Int)], Modifier[Any, CONST]]]
   |          ?2 is an unknown value of type Candidate.given_Candidate_R[(8 : Int), ISCONST[(true : Boolean)],
   |  DFVal[DFBits[(8 : Int)], Modifier[Any, CONST]]]
   |          ?3 is an unknown value of type DFValTP[DFBits[?1.OutW], ?2.OutP | ISCONST[(true : Boolean)]]

Expectation

No error.

@soronpo soronpo added itype:bug regression This worked in a previous version but doesn't anymore stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 29, 2024
@EugeneFlesselle
Copy link
Contributor

Minimised a bit more:

trait DFVal[+T <: Int, +P]

trait Summon[R, T <: R]
given [R, T <: R]: Summon[R, T] with {}

trait Candidate[R]:
  type OutW <: Int
  type OutP
given [W <: Int, P, R <: DFVal[W, P]]: Candidate[R] with
  type OutW = W
  type OutP = P

extension [L <: DFVal[Int, Any]](lhs: L)(using icL: Candidate[L])
  def ^^^[R](rhs: R)
            (using icR: Candidate[R])
            : DFVal[icL.OutW, icL.OutP | icR.OutP] = ???
  def ^^^ : Unit = ???

extension [L](lhs: L)
  def ^^^[RW <: Int, RP](rhs: DFVal[RW, RP])
                        (using es: Summon[L, lhs.type])
                        (using c: Candidate[L])
                        (using check: c.OutW =:= c.OutW)
                        : DFVal[c.OutW, c.OutP | RP] = ???

val x: DFVal[8, true] = ???
val z1 = x ^^^ x // Ok
val z2 = z1 ^^^ x // Ok
val zzz = x ^^^ x ^^^ x // Error

When def ^^^ : Unit = ??? is removed, all of z1, z2, zzz succeed with the first def ^^^.
When it is present, they all attempt to use last def ^^^ which fails for zzz only.
If we remove the last def ^^^, then they all succeed with the first one independently of the 2nd one.

@EugeneFlesselle EugeneFlesselle linked a pull request Mar 31, 2024 that will close this issue
@soronpo
Copy link
Contributor Author

soronpo commented Mar 31, 2024

I will say that further look suggests there is a (false?) overloading ambiguity issue that may cause this:

trait DFVal[+T <: Int, +P]
type DFValAny = DFVal[Int, Any]
trait Candidate[R]:
  type OutW <: Int
  type OutP
given [W <: Int, P, R <: DFVal[W, P]]: Candidate[R] with
  type OutW = W
  type OutP = P

val x = new DFVal[5, true]{}

object Works1:
  extension [L <: DFValAny](lhs: L)(using icL: Candidate[L])
    def ^^^(rhs: Any): Unit = ???
  extension [L](lhs: L)
    def ^^^[R](rhs: R): Unit = ???
  val y = x ^^^ x //ok

object Works2:
  extension [L <: DFValAny](lhs: L)(using icL: Candidate[L])
    def ^^^(rhs: Any): Unit = ???
    def ^^^ : Unit = ???
  val y = x ^^^ x //ok

object Fail:
  extension [L <: DFValAny](lhs: L)(using icL: Candidate[L])
    def ^^^(rhs: Any): Unit = ???
    def ^^^ : Unit = ???
  extension [L](lhs: L)
    def ^^^[R](rhs: R): Unit = ???
  val y = x ^^^ x //error

@Gedochao Gedochao added area:implicits related to implicits and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 2, 2024
@soronpo
Copy link
Contributor Author

soronpo commented Apr 15, 2024

OK, so if I understand correctly, the reason my original code works now on nightly is that #19955 was resolved. But there is still an underlying overloading issue, right?

@EugeneFlesselle
Copy link
Contributor

OK, so if I understand correctly, the reason my original code works now on nightly is that #19955 was resolved. But there is still an underlying overloading issue, right?

Yes that's right, it compiles but picks the wrong (3rd) alternative.

@sjrd
Copy link
Member

sjrd commented Jun 28, 2024

Based on my analysis at #20054 (comment), it appears adjusting overload resolution to be intuitive wrt. to this issue would definitely be a breaking change.

@soronpo You mention that

my original code works now on nightly

so the current issue is not a regression per se after all, is it?

@soronpo
Copy link
Contributor Author

soronpo commented Jun 28, 2024

so the current issue is not a regression per se after all, is it?

You are correct. The part that has regressed was fixed. I do think the overloading needs to be fixed, but if we can't tie the knot of the rules well, then we should avoid further regressions first.

@sjrd sjrd added area:overloading and removed area:implicits related to implicits regression This worked in a previous version but doesn't anymore labels Jun 28, 2024
@Gedochao Gedochao changed the title Overloading or implicit regression in 3.4.1 Overloading or implicit bug in 3.4.1 Jul 8, 2024
@Gedochao Gedochao added the needs-minor-release This PR cannot be merged until the next minor release label Jul 8, 2024
@Gedochao
Copy link
Contributor

This has been decided to be worked on in 3.7.0 (TBD what's the timing on that).
For now, we will add a warning in 3.6.0.

@soronpo
Copy link
Contributor Author

soronpo commented Jul 10, 2024

What kind of warning? It's already an error. I would caution from adding redundant warnings. They only increase technical depth with false positives and specific version suppression. Better no action until 3.7.

@EugeneFlesselle
Copy link
Contributor

EugeneFlesselle commented Jul 10, 2024

It's already an error.

It is not in cases such as those discussed in #20054 (comment) and #20054 (comment)

What kind of warning?

We plan of warning (in 3.6.0) about the changes in overload resolution which would apply in 3.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:overloading itype:bug needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants