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

resolution fails paramters by-name followed by varargs vs parameter by-name #3761

Closed
scabug opened this Issue Aug 14, 2010 · 6 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented Aug 14, 2010

# cat VarArgs.scala
class Foo {
  def info0(m: String) { }
  def info0(m: String, args: Any*) { }

  def info1(m: =>String) { }
  def info1(m: =>String, args: Any*) { }
}
object VarArgs {
  def main(args: Array[String]) {
    val foo = new Foo
    foo.info0("hello")
    foo.info1("world")
  }
}
# scalac -version
Scala compiler version 2.8.0.final -- Copyright 2002-2010, LAMP/EPFL
# scalac VarArgs.scala
VarArgs.scala:16: error: ambiguous reference to overloaded definition,
both method info1 in class Foo of type (m: => String,args: Any*)Unit
and  method info1 in class Foo of type (m: => String)Unit
match argument types (java.lang.String)
    foo.info1("world")
        ^
one error found

The by-value methods can be handled by not the by-name methods.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 14, 2010

Imported From: https://issues.scala-lang.org/browse/SI-3761?orig=1
Reporter: Richard Emberson (rmemberson)
Other Milestones: 2.10.0
See #1386

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 11, 2012

@som-snytt said:
I'm preparing a pull for this issue and #4728.

(This bug had to do with the specificity test trying to apply info1(=>String) to (String); the other bug with expanding the vararg in the same test.)

The fixes addressing the bugs were straightforward, but I got ambitious. Namely, I'd like this to work:

class Tri {
  def foo(m: =>String,n: =>String) = m +" "+ n + "!"
  def foo(m: =>String,n: String) = m +" "+ n + "!!"
  def foo(m: String,n: =>String) = m +" "+ n + "!!!"
  def foo(m: String,n: String) = m +" "+ n + "!!!!"

  // match on foo(String,=>String)
  def bar(m: String,n: =>String) = foo(m,n)
}

object Test {
  def main(args: Array[String]) {
    val tri = new Tri
    println(tri.bar("hello","world"))
  }
}

The motivation is that you can overload m(A) and m(=>A) but then it's difficult to disambiguate in client code because you can't name =>A as in hypothetical application m(null: =>A). You get your by-name API back by using a different interface or a proxy method as above, if the proxy would select the args that match best.

The spec is actually silent on this issue. If =>A is not selected on shape (as though it were ()=>A, it seems to me that m(=>A) is more specific than m(A) [since we can apply m(=>A) to (A) only by a special case and not by a conversion]. Whatever, that's not intuitive.

My patch adds a tie-breaker where the winner has to match the by-namedness of the applied args exactly. See above.

This is an edge case, but has the benefit of being simple and enabling something that's otherwise hard (without putting a can-opener to the complexity worms gathering dust in the tackle box).

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 21, 2012

@som-snytt said:
scala/scala#585

The behavior described in the previous comment is under a private option -Yresolve-by-name.

As described in the commit, the patch is to handle isCompatible test for A and =>A explicitly.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 22, 2012

@retronym said:
Does this change also remedy SI-1386?

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@som-snytt said:
No. Is there something to remedy in SI-1386? (Rhetorical question.)

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@som-snytt said:
lrytz merged commit 94e5d76

The patch doesn't include the variant overloading resolution. Sniff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment