Skip to content

Ambiguously overloaded methods fail to error when assigned to a val #18294

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

Closed
jchyb opened this issue Jul 26, 2023 · 6 comments
Closed

Ambiguously overloaded methods fail to error when assigned to a val #18294

jchyb opened this issue Jul 26, 2023 · 6 comments

Comments

@jchyb
Copy link
Contributor

jchyb commented Jul 26, 2023

Compiler version

main (97677cc), 3.3.0 and likely any previous version

Minimized code

def f(s: Int): Unit = println("a")
def f: Int => Unit = _ => println("b")

@main def Test =
  val test: Int => Unit = f
  test(0)
  // f(0) // this errors out as expected

Output

Compiles, and when run prints "a".

Expectation

Should error out with an Ambiguous Overload error, like using f directly does.

Additional comment

I found this when working on #18286, and I know that resolveOverloaded1 method here can affect which f is being chosen - so the issue likely lies somewhere there.

@jchyb jchyb added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jul 26, 2023
@jchyb jchyb changed the title Ambigous overloaded methods fail to error when assigned to a val Ambiguous overloaded methods fail to error when assigned to a val Jul 26, 2023
@jchyb jchyb changed the title Ambiguous overloaded methods fail to error when assigned to a val Ambiguously overloaded methods fail to error when assigned to a val Jul 26, 2023
@jchyb jchyb added area:typer area:overloading and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jul 26, 2023
@iusildra
Copy link
Contributor

iusildra commented Aug 2, 2023

I'll try to look at it, but I have a few questions:

Why should it produce an ambiguous overload error ? We want to assign to test an instance of Function1 and even though f(s: Int): Unit is a match, it requires a few transformations beforehand whereas f: Int => Unit can be taken as it is. In other words, it's a bit like the 2nd def is "more specific" than the 1st one. So I would expect it to print "b". Am I wrong ?

Also, if both "direct" & "indirect" calls must fail, shouldn't we instead prevent people from defining 2 methods where the second returns a FunctionN with the same arg types and return type than the first one ?

@som-snytt
Copy link
Contributor

som-snytt commented Aug 2, 2023

It's "b" in Scala 2.

I think this issue was previously discussed for Dotty, but I don't have a link or explanation handy.

(Maybe in Scala 2, overload resolution precedes eta expansion; that is, it's not an application, so first take alternatives that are compatible with the expected type; but the definition of compatible says after eta expansion, so that idea is not right.)

@dwijnand
Copy link
Member

dwijnand commented Aug 3, 2023

Why should it produce an ambiguous overload error ? We want to assign to test an instance of Function1 and even though f(s: Int): Unit is a match, it requires a few transformations beforehand whereas f: Int => Unit can be taken as it is. In other words, it's a bit like the 2nd def is "more specific" than the 1st one. So I would expect it to print "b". Am I wrong ?

It's a little ironic how your logical steps lead you to the other answer than what the compiler does - perhaps good evidence that it should be made to error as ambiguous. Not that we could afford to just flip the meaning from one version to the next anyways...

At a guess, the requirement of Int => Unit on test makes f be desugared to x => f(x), which the first def matches more readily. Btw, not only does the second def require an extra .apply, being a def means that it's an ExprType, i.e. => Int => Unit, which also might make it less readily applicable.

@iusildra
Copy link
Contributor

iusildra commented Aug 3, 2023

It's a little ironic how your logical steps lead you to the other answer than what the compiler does - perhaps good evidence that it should be made to error as ambiguous.

Seems legit 😆

@som-snytt
Copy link
Contributor

Should error out with an Ambiguous Overload error, like using f directly does.

This is a "direct use" of f:

val test: Int => Unit = f

Spec says:

Implicit conversions can be applied to expressions whose type does not match their expected type, to qualifiers in selections, and to unapplied methods.

None of those conditions apply. Odersky is firm in the linked PR that the behavior is "as specified". Maybe these are the spec words which mean "do the obvious thing and take f since it works."

@som-snytt
Copy link
Contributor

Duplicates #4831

Dotty came around and agrees with Scala 2 since [checks watch] 3.3.4.

➜  snips scala-cli run --server=false -S 3.5.2 i18294.scala
b
➜  snips scala-cli run --server=false -S 3.3.4 i18294.scala
b
➜  snips scala-cli run --server=false -S 3.3.3 i18294.scala
a

I see this behavior progressed in the linked PR for 3.4.1 then backported to LTS.

@som-snytt som-snytt closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants