Skip to content

Ambiguous overload: false detection #16618

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

Open
satorg opened this issue Jan 5, 2023 · 6 comments
Open

Ambiguous overload: false detection #16618

satorg opened this issue Jan 5, 2023 · 6 comments
Labels
area:infer backlog No work planned on this by the core team for the time being. itype:enhancement

Comments

@satorg
Copy link

satorg commented Jan 5, 2023

Compiler version

Scala 3.2.1

Minimized code

case class Foo[A](a: A) {
  def bar(aa: A): Boolean = a == aa
  def bar(that: Foo[A]) = a == that.a
}

object Test {
  Foo(123).bar(Foo(456))
}

Output

The snippet compiles just fine on 2.12.17 and 2.13.9, but fails to compile on 3.2.1 with the following error:

7 |  Foo(123).bar(Foo(456))
  |  ^^^^^^^^^^^^
  |Ambiguous overload. The overloaded alternatives of method bar in class Foo with types
  | (that: Foo[A]): Boolean
  | (aa: A): Boolean
  |both match arguments (Foo[A])

However, if I change the Test object this way:

object Test {
  val foo123 = Foo(123)
  foo123.bar(Foo(456))
}

then it compiles on 3.2.1 without any error as well.

Expectation

Both cases should compile on 3.2.1.

Notes

May be similar to #14582 and #16067.

@satorg satorg added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 5, 2023
@prolativ prolativ added area:typer and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 5, 2023
@prolativ
Copy link
Contributor

prolativ commented Jan 5, 2023

I looks like the compiler tries to defer the inference of the type parameter of the first Foo and it's yet not inferred when the compiler tries to resolve the overload. The snippet starts to compile if you pass this type explicitly

case class Foo[A](a: A) {
  def bar(aa: A): Boolean = a == aa
  def bar(that: Foo[A]) = a == that.a
}

object Test {
  Foo[Int](123).bar(Foo(456))
  Foo[Any](123).bar(Foo(456))
}

@yzia2000
Copy link
Contributor

yzia2000 commented Jan 8, 2023

Is this a bug or an intended effect?

@prolativ
Copy link
Contributor

prolativ commented Jan 9, 2023

For me the current behaviour of the compiler is rather unexpected and misleading so I would classify this as a bug.
But I cannot say for sure that this delay in type inference was an unintended - maybe there was some corner case when this was helpful but no specific example comes to my mind right now.
However we might try to fix this anyway to make the initial snippet work or at least make the error message clearer

@odersky
Copy link
Contributor

odersky commented Jan 9, 2023

The delay in instantiation is intended. It's one of the reasons Scala 3 generally infers better than Scala 2 (i.e. we are a couple of steps closer to global type inference). But in this case it has negative consequences.

@dwijnand
Copy link
Member

I haven't run the compiler through this, but running my brain through it, I think the only enhancement we could consider is adding a filter on the overloads consider by considering that A <: Int & Foo[...] is uninhabited and thus the overload should be discarded and the remaining one picked. But we would also have to consider null, which could inhabit Int & Foo[...]. And also that would only be true in this case because Int is final.

@odersky
Copy link
Contributor

odersky commented Oct 21, 2023

I haven't run the compiler through this, but running my brain through it, I think the only enhancement we could consider is adding a filter on the overloads consider by considering that A <: Int & Foo[...] is uninhabited and thus the overload should be discarded and the remaining one picked. But we would also have to consider null, which could inhabit Int & Foo[...]. And also that would only be true in this case because Int is final.

I don't think that the added complexity would justify the gains.

@odersky odersky added the backlog No work planned on this by the core team for the time being. label Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:infer backlog No work planned on this by the core team for the time being. itype:enhancement
Projects
None yet
Development

No branches or pull requests

5 participants