Unbox value classes in arguments on dynamic function call

[G1ng3r]

Fixes #16995 and #16984
Perform value class unboxing when it appears in arguments list on dynamic function call;
Dynamic function call with varargs have to be passed as one argument;

[G1ng3r]

@prolativ Hi! Could you take a look, please.

[sjrd]

For varargs, it's debatable, for at least for value classes we should not do this in typer. The behavior of requiring unboxing is very specific to the ReflectiveSelectable (or I don't remember its exact name) mechanism. It probably would not make sense for other implementations of Selectable.

For varargs, I guess it makes sense to make it a Seq. But then I suggest to make it clear in docs/_docs/reference/changed-features/structural-types-spec.md at the same time.

[G1ng3r]

For varargs, it's debatable, for at least for value classes we should not do this in typer. The behavior of requiring unboxing is very specific to the ReflectiveSelectable (or I don't remember its exact name) mechanism. It probably would not make sense for other implementations of Selectable.

@sjrd Thanks! I made some tests and it does not work with subclasses of scala.Selectable. Is it ok if I make varargs transformation only for subclasess of reflect.Selectable, which relies on java reflection? Cause it works then

[sjrd]

If it is limited to reflect.Selectable, it should be fine.

You should also test what happens when using a seq expansion at call site for those varargs.

[G1ng3r]

@sjrd did you mean add test like this one?

[prolativ]

I wanted to suggest adding a test case verifying the behaviour of mixed curried parameters and varargs. However this seems broken in general at the moment #18009

[prolativ]

But it should be possible to add a test case for a curried method without varargs, actually, like

def curried(argument: Argument)(foo: Foo)(someInt: Int) = foo.i + someInt + argument.x.length

[G1ng3r]

@prolativ could you take a look please?

[prolativ]

I think we had slightly wrong assumptions about the tests here. As I pointed out here #18009 (comment) Selectable has a different desugaring than Dynamic according to the language specification and we should always expect the multiple parameters lists of a dynamic method invocation to be flattened for all subtypes of Selectable (not only reflect.Selectable). Because of that applyDynamic in subtypes of Selectable is currently not going to work well in most cases if it has more than one parameter list for actual explicit arguments, so we should not focus on cases like

  def applyDynamic(name: String)(ints: Int*)(x: Int): Int = ints.sum + x

  def applyDynamic(name: String)(foo: Foo)(argument: Argument)(someInt: Int): Int = foo.i + 

in our tests.
I prepared an alternative test (slightly more structured) to show what I think we should actually expect to work to consider #16995 and #16984 as fixed:

import scala.language.dynamics
import scala.reflect.Selectable.reflectiveSelectable

object Test {
  class Num(val i: Int) extends AnyVal

  def show(x: Int | Num | Seq[Int | Num]): String = x match
    case i: Int => i.toString
    case num: Num => num.i.toString
    case seq: Seq[Int | Num] => seq.map(show).mkString(" ")

  trait Nonreflective extends Selectable:
    def selectDynamic(name: String): String = name
    def applyDynamic(name: String)(args: (Int | Num | Seq[Int | Num])*): String =
      val argsString = args.map(show).mkString(" ", " ", "")
      s"${name}${argsString}"

  trait Dynamic0 extends Dynamic:
    def selectDynamic(name: String): String = name

  trait Dynamic1 extends Dynamic:
    def applyDynamic(name: String)(args1: (Int | Num | Seq[Int | Num])*): String =
      val argsString = args1.map(show).mkString(" ", " ", "")
      s"${name}${argsString}"

  trait Dynamic2 extends Dynamic:
    def applyDynamic(name: String)(args1: (Int | Num)*)(args2: (Int | Num)*): String =
      val argsString = (args1 ++ args2).map(show).mkString(" ", " ", "")
      s"${name}${argsString}"

  trait Dynamic3 extends Dynamic:
    def applyDynamic(name: String)(args1: (Int | Num)*)(args2: (Int | Num)*)(args3: (Int | Num)*): String =
      val argsString = (args1 ++ args2 ++ args3).map(show).mkString(" ", " ", "")
      s"${name}${argsString}"

  type Api = {
    def foo: String
    def foo0(): String
    def fooI(i: Int): String
    def fooN(n: Num): String
    def fooII(i1: Int, i2: Int): String
    def fooNN(n1: Num, n2: Num): String
    def fooIvarargs(is: Int*): String
    def fooNvarargs(ns: Num*): String
    def fooIseq(is: Seq[Int]): String
    def fooNseq(ns: Seq[Num]): String
    def fooIIvarargs(i1: Int, is: Int*): String
    def fooNNvarargs(n1: Num, ns: Num*): String
    def fooI_I(i1: Int)(i2: Int): String
    def fooN_N(n1: Num)(n2: Num): String
    def foo0_II()(i1: Int, i2: Int): String
    def foo0_NN()(n1: Num, n2: Num): String
    def foo0_Ivarargs()(is: Int*): String
    def foo0_Nvarargs()(ns: Num*): String
    def foo0_I_I()(i1: Int)(i2: Int): String
    def foo0_N_N()(n1: Num)(n2: Num): String
  }

  class ClassImpl {
    def foo: String = "foo"
    def foo0(): String = "foo0"
    def fooI(i: Int): String = s"fooI ${i}"
    def fooN(n: Num): String = s"fooN ${n.i}"
    def fooII(i1: Int, i2: Int): String = s"fooII ${i1} ${i2}"
    def fooNN(n1: Num, n2: Num): String = s"fooNN ${n1.i} ${n2.i}"
    def fooIvarargs(is: Int*): String = s"fooIvarargs${is.mkString(" ", " ", "")}"
    def fooNvarargs(ns: Num*): String = s"fooNvarargs${ns.map(_.i).mkString(" ", " ", "")}"
    def fooIseq(is: Seq[Int]): String = s"fooIseq${is.mkString(" ", " ", "")}"
    def fooNseq(ns: Seq[Num]): String = s"fooNseq${ns.map(_.i).mkString(" ", " ", "")}"
    def fooIIvarargs(i1: Int, is: Int*): String = s"fooIIvarargs ${i1}${is.mkString(" ", " ", "")}"
    def fooNNvarargs(n1: Num, ns: Num*): String = s"fooNNvarargs ${n1.i}${ns.map(_.i).mkString(" ", " ", "")}"
    def fooI_I(i1: Int)(i2: Int): String = s"fooI_I ${i1} ${i2}"
    def fooN_N(n1: Num)(n2: Num): String = s"fooN_N ${n1.i} ${n2.i}"
    def foo0_II()(i1: Int, i2: Int): String = s"foo0_II ${i1} ${i2}"
    def foo0_NN()(n1: Num, n2: Num): String = s"foo0_NN ${n1.i} ${n2.i}"
    def foo0_Ivarargs()(is: Int*): String = s"foo0_Ivarargs${is.mkString(" ", " ", "")}"
    def foo0_Nvarargs()(ns: Num*): String = s"foo0_Nvarargs${ns.map(_.i).mkString(" ", " ", "")}"
    def foo0_I_I()(i1: Int)(i2: Int): String = s"foo0_I_I ${i1} ${i2}"
    def foo0_N_N()(n1: Num)(n2: Num): String = s"foo0_N_N ${n1.i} ${n2.i}"
  }


  def main(args: Array[String]): Unit = {
    val reflective: Api = new ClassImpl()
    val nonreflective: Nonreflective & Api = (new Nonreflective {}).asInstanceOf[Nonreflective & Api]
    val dynamic0 = new Dynamic0 {}
    val dynamic1 = new Dynamic1 {}
    val dynamic2 = new Dynamic2 {}
    val dynamic3 = new Dynamic3 {}

    println(reflective.foo)
    println(reflective.foo0())
    println(reflective.fooI(1))
    println(reflective.fooN(new Num(1)))
    println(reflective.fooII(1, 2))
    println(reflective.fooNN(new Num(1), new Num(2)))
    println(reflective.fooIvarargs(1, 2))
    println(reflective.fooNvarargs(new Num(1), new Num(2)))
    println(reflective.fooIseq(Seq(1, 2)))
    println(reflective.fooNseq(Seq(new Num(1), new Num(2))))
    println(reflective.fooIIvarargs(1, 2))
    println(reflective.fooNNvarargs(new Num(1), new Num(2)))
    println(reflective.fooI_I(1)(2))
    println(reflective.fooN_N(new Num(1))(new Num(2)))
    println(reflective.foo0_II()(1, 2))
    println(reflective.foo0_NN()(new Num(1), new Num(2)))
    println(reflective.foo0_Ivarargs()(1, 2))
    println(reflective.foo0_Nvarargs()(new Num(1), new Num(2)))
    println(reflective.foo0_I_I()(1)(2))
    println(reflective.foo0_N_N()(new Num(1))(new Num(2)))
    println()
    println(nonreflective.foo)
    println(nonreflective.foo0())
    println(nonreflective.fooI(1))
    println(nonreflective.fooN(new Num(1)))
    println(nonreflective.fooII(1, 2))
    println(nonreflective.fooNN(new Num(1), new Num(2)))
    println(nonreflective.fooIvarargs(1, 2))
    println(nonreflective.fooNvarargs(new Num(1), new Num(2)))
    println(nonreflective.fooIseq(Seq(1, 2)))
    println(nonreflective.fooNseq(Seq(new Num(1), new Num(2))))
    println(nonreflective.fooIIvarargs(1, 2))
    println(nonreflective.fooNNvarargs(new Num(1), new Num(2)))
    println(nonreflective.fooI_I(1)(2))
    println(nonreflective.fooN_N(new Num(1))(new Num(2)))
    println(nonreflective.foo0_II()(1, 2))
    println(nonreflective.foo0_NN()(new Num(1), new Num(2)))
    println(nonreflective.foo0_Ivarargs()(1, 2))
    println(nonreflective.foo0_Nvarargs()(new Num(1), new Num(2)))
    println(nonreflective.foo0_I_I()(1)(2))
    println(nonreflective.foo0_N_N()(new Num(1))(new Num(2)))
    println()
    println(dynamic0.foo)
    println(dynamic1.foo0())
    println(dynamic1.fooI(1))
    println(dynamic1.fooN(new Num(1)))
    println(dynamic1.fooII(1, 2))
    println(dynamic1.fooNN(new Num(1), new Num(2)))
    println(dynamic1.fooIvarargs(1, 2))
    println(dynamic1.fooNvarargs(new Num(1), new Num(2)))
    println(dynamic1.fooIseq(Seq(1, 2)))
    println(dynamic1.fooNseq(Seq(new Num(1), new Num(2))))
    println(dynamic1.fooIIvarargs(1, 2))
    println(dynamic1.fooNNvarargs(new Num(1), new Num(2)))
    println(dynamic2.fooI_I(1)(2))
    println(dynamic2.fooN_N(new Num(1))(new Num(2)))
    println(dynamic2.foo0_II()(1, 2))
    println(dynamic2.foo0_NN()(new Num(1), new Num(2)))
    println(dynamic2.foo0_Ivarargs()(1, 2))
    println(dynamic2.foo0_Nvarargs()(new Num(1), new Num(2)))
    println(dynamic3.foo0_I_I()(1)(2))
    println(dynamic3.foo0_N_N()(new Num(1))(new Num(2)))
  }
}

with expected runtime output

foo
foo0
fooI 1
fooN 1
fooII 1 2
fooNN 1 2
fooIvarargs 1 2
fooNvarargs 1 2
fooIseq 1 2
fooNseq 1 2
fooIIvarargs 1 2
fooNNvarargs 1 2
fooI_I 1 2
fooN_N 1 2
foo0_II 1 2
foo0_NN 1 2
foo0_Ivarargs 1 2
foo0_Nvarargs 1 2
foo0_I_I 1 2
foo0_N_N 1 2

foo
foo0
fooI 1
fooN 1
fooII 1 2
fooNN 1 2
fooIvarargs 1 2
fooNvarargs 1 2
fooIseq 1 2
fooNseq 1 2
fooIIvarargs 1 2
fooNNvarargs 1 2
fooI_I 1 2
fooN_N 1 2
foo0_II 1 2
foo0_NN 1 2
foo0_Ivarargs 1 2
foo0_Nvarargs 1 2
foo0_I_I 1 2
foo0_N_N 1 2

foo
foo0
fooI 1
fooN 1
fooII 1 2
fooNN 1 2
fooIvarargs 1 2
fooNvarargs 1 2
fooIseq 1 2
fooNseq 1 2
fooIIvarargs 1 2
fooNNvarargs 1 2
fooI_I 1 2
fooN_N 1 2
foo0_II 1 2
foo0_NN 1 2
foo0_Ivarargs 1 2
foo0_Nvarargs 1 2
foo0_I_I 1 2
foo0_N_N 1 2

[prolativ]

Suggested change

	if isReflectSelectable
	then untpd.Apply(base, handled.flatten)
	else handled.foldLeft(base)((base, args) => untpd.Apply(base, args))
	untpd.Apply(base, handled.flatten)

We should always flatten the argument lists for Selectable

[G1ng3r]

thanks! I will fix that

[G1ng3r]

@prolativ Hi! Sorry it took me so long, could you please take a look? There was changes in other tests, is it ok? Now scala.Selectable instances can only have applyDynamic(name: String)(args: ?*) method, is it ok too?