Skip to content

Varargs and currying in Selectable's applyDynamic #18009

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
prolativ opened this issue Jun 19, 2023 · 4 comments
Open

Varargs and currying in Selectable's applyDynamic #18009

prolativ opened this issue Jun 19, 2023 · 4 comments

Comments

@prolativ
Copy link
Contributor

Compiler version

3.3.2-RC1-bin-20230615-916d4e7-NIGHTLY and before

Minimized code

class Sel1 extends Selectable {
  def applyDynamic(name: String)(args: Int*): Int = args.sum
}
class Sel2 extends Selectable {
  def applyDynamic(name: String)(args: Int*)(arg: Int): Int = args.sum + arg
}

object Main {
  def main(args: Array[String]) = {
    val sel1 = (new Sel1).asInstanceOf[Sel1 {
      def foo(x: Int)(y: Int): Int 
      def bar(xs: Int*)(y: Int): Int 
    }]
    // println(sel1.foo(1)(2))
    // println(sel1.bar(1)(2))

    val sel2 = (new Sel2).asInstanceOf[Sel2 {
      def foo(x: Int)(y: Int): Int 
      def bar(xs: Int*)(y: Int): Int 
    }]
    // println(sel2.foo(1)(2))
    // println(sel2.bar(1)(2))
  }
}

Output

When one tries to compile and run the snippet from above after uncommenting only one commented line at a time, here's what happens in each case:

  • sel1.foo:
    Compilation succeeds, prints 3.
    Relevant -Xprint:typer output:
sel1.applyDynamic("foo")([1,2 : Int]*).$asInstanceOf[Int]
  • sel1.bar:
    Compilation error
[error] Sel.scala:15:22
[error] Sequence argument type annotation `*` cannot be used here:
[error] it is not the only argument to be passed to the corresponding repeated parameter Int*
[error]     println(sel1.bar(1)(2))
[error]                      ^

Relevant -Xprint:typer output:

sel1.applyDynamic("bar")([[1 : Int]*,2 : Int]*).$asInstanceOf[Int]
  • sel2.foo:
    Runtime exception
Exception in thread "main" java.lang.ClassCastException: Main$$$Lambda$3/1929600551 cannot be cast to java.lang.Integer
        at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99)
        at Main$.main(Sel.scala:21)
        at Main.main(Sel.scala)

Relevant -Xprint:typer output:

{
  val args$1: Int* = ([1,2 : Int]*)
  {
    def $anonfun(arg: Int): Int =
      sel2.applyDynamic("foo")(args$1)(arg)
    closure($anonfun)
  }
}.$asInstanceOf[Int]
  • sel2.bar:
    Compilation error
[error] Sel.scala:22:22
[error] Sequence argument type annotation `*` cannot be used here:
[error] it is not the only argument to be passed to the corresponding repeated parameter Int*
[error]     println(sel2.bar(1)(2))
[error]                      ^

Relevant -Xprint:typer output:

sel2.applyDynamic("bar")([[1 : Int]*,2 : Int]*).$asInstanceOf[Int]

Expectation

The specification says:

Given a value v of type C { Rs }, where C is a class reference and Rs are structural refinement declarations, and given v.a of type U [...]

  • If U is a method type (T11, ..., T1n)...(TN1, ..., TNn): R and it is not a dependent method type, we map v.a(a11, ..., a1n)...(aN1, ..., aNn) to:
v.applyDynamic("a")(a11, ..., a1n, ..., aN1, ..., aNn)
  .asInstanceOf[R]

Thus, according to the spec, the 4 cases should get desugared as follows:

  • sel1.foo(1)(2) -> sel1.applyDynamic("foo")(1, 2).asInstanceOf[Int]
  • sel1.bar(1)(2) -> sel1.applyDynamic("bar")(1, 2).asInstanceOf[Int]
  • sel2.foo(1)(2) -> sel2.applyDynamic("foo")(1, 2).asInstanceOf[Int]
  • sel2.bar(1)(2) -> sel2.applyDynamic("bar")(1, 2).asInstanceOf[Int]
    The first two cases should compile successfully and return 3 then, while the latter two should either perform eta-expansion and fail at runtime with an exception on casting a function to Int (we should try to avoid this situation) or complain about a missing parameter.

On the other hand, as a user I would expect code using Selectable to behave analogously to code using Dynamic instead.
Thus, given the snippet below

import scala.language.dynamics

class Dyn1 extends Dynamic {
  def applyDynamic(name: String)(args: Int*): Int = args.sum
}
class Dyn2 extends Dynamic {
  def applyDynamic(name: String)(args: Int*)(arg: Int): Int = args.sum + arg
}

object Main {
  def main(args: Array[String]) = {
    val dyn1 = new Dyn1
    // println(dyn1.foo(1)(2))
    // println(dyn1.bar(1)(2))

    val dyn2 = new Dyn2
    // println(dyn2.foo(1)(2))
    // println(dyn2.bar(1)(2))
  }
}

and uncommenting a single line at a time, I get:

  • dyn1.foo:
[error] Dyn.scala:13:13
[error] method applyDynamic in class Dyn1 does not take more parameters
[error]     println(dyn1.foo(1)(2))
[error]             ^^^^^^^^^^
  • dyn1.bar:
[error] Dyn.scala:14:13
[error] method applyDynamic in class Dyn1 does not take more parameters
[error]     println(dyn1.bar(1)(2))
[error]             ^^^^^^^^^^
  • dyn2.foo:
    Compiles, prints 3

  • dyn2.bar:
    Compiles, prints 3

For Dynamics the behaviour in scala 2 is basically the same, though the compilation errors for dyn1.foo and dyn1.bar are slightly different:

[error] Dyn.scala:13:13
[error] Int does not take parameters
[error]     println(dyn1.foo(1)(2))
[error]             ^^^^^^^^^^^^^^
[error] Dyn.scala:14:13
[error] Int does not take parameters
[error]     println(dyn1.bar(1)(2))
[error]             ^^^^^^^^^^^^^^

Summing this up, the behaviour for Dynamics in this case is the opposite to the specified behaviour for Selectable, which in turn is different to the current actual (definitely buggy in some way) behaviour for Selectable.

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

odersky commented Jun 24, 2023

@prolativ Since you were already looking at the issue would you be up to try to diagnose and fix this?

@prolativ
Copy link
Contributor Author

@odersky how would we like to solve the problem then? Make the actual behaviour compliant with the specification of Selectable? Or is there any chance to update the specification to make things more consistent?

@odersky
Copy link
Contributor

odersky commented Jun 26, 2023

I don't know, and won't have the cycles to get deep into it. We need a simple fix. Just explore whatever can be made to work.

@prolativ
Copy link
Contributor Author

I did some analysis and it seems like an attempt to bring the behaviours of Dynamic and Selectable closer to each other wouldn't be possible without introducing major breaking changes. The desugaring of applyDynamic for subtypes of Selectable takes into account the declared method signatures from refinements to determine how many argument lists should be consumed by the syntactic transformation (where the consumed lists are concatenated), while for Dynamics the number and structure of argument lists are preserved, e.g.

  • sel.foo(x1)(y1, y2)(...) will become sel.applyDynamic("foo")(x1, y1, y2, ...)
  • dyn.foo(x1)(y1, y2)(...) will become dyn.applyDynamic("foo")(x1)(y1, y2)(...)

In both cases the shape of the signatures of applyDynamic don't really have to exactly match the application shape resulting from the desugaring, as long as some further syntactic adjustments can make the entire code compile. This makes

def applyDynamic(name: String)(args: Int*)(arg: Int): Int = args.sum + arg

(in case of Selectables) effectively equivalent to

def applyDynamic(name: String)(args: Int*): Int => Int = (arg: Int) => args.sum + arg

so it will work for a refinement method like

def foo(x: Int)(y: Int): Int => Int

which is sel2.foo case from the initial snippet with a modified result type. I'm still wondering whether we could emit an error (or at least a warning) instead of throwing a ClassCastException for sel2.foo and similar cases. My idea for this moment would be:

  1. Try to compute the normalized result type of applyDynamic by turning trailing curried parameter lists into function parameters, e.g. for
def applyDynamic(name: String)(args: Int*)(x: Boolean)(y: String, z: Char): Int

the normalized type would be Boolean => (String, Char) => Int by transforming to

def applyDynamic(name: String)(args: Int*): Boolean => (String, Char) => Int
  1. Check if a cast from the normalized type from applyDynamic to the type declared in a structural refinement is ever possible, so for
class Sel2 extends Selectable {
  def applyDynamic(name: String)(args: Int*)(arg: Int): Int = args.sum + arg
}
val sel2 = (new Sel2).asInstanceOf[Sel2 {
  def foo(x: Int)(y: Int): Int 
}]

that would cause a compilation warning/error because Int => Int can never happen to be an Int.
However implementing this check might be quite tricky (e.g. what about using parameters placed in different positions in the signature of applyDynamic?)

The problem of dangling parameter lists in applyDynamic, however, seems to be independent of the problem of combining varargs with concatenated parameter lists.
E.g. when given

class Sel1 extends Selectable {
  def applyDynamic(name: String)(args: Int*): Int = args.sum
}
val sel1 = (new Sel1).asInstanceOf[Sel1 {
  def baz(xs: Int*)(ys: Int*): Int 
}]

we could try to make

sel1.baz(1)(2)

compile by desugaring it to

sel1.applyDynamic("baz")(1, 2)

but what about something like

val ints1 = Seq(1)
sel1.baz(ints1*)(2)

?

sel1.applyDynamic("baz")(ints1*, 2)

is not something legal in general. On the other hand

val ints2 = Seq(2)
sel1.baz(1)(ints2*)

desugared as

sel1.applyDynamic("baz")(1, ints2*)

would make sense, although this seems quite inconsistent.

@odersky any opinions on how we should handle these 2 problems?

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

No branches or pull requests

2 participants