scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.85k stars 1.06k forks source link

Varargs and currying in `Selectable`'s `applyDynamic` #18009

Open prolativ opened 1 year ago

prolativ commented 1 year ago

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:

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 [...]

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

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:

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.

odersky commented 1 year ago

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

prolativ commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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.

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
  2. 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?