-
Notifications
You must be signed in to change notification settings - Fork 28
SIP-45 - Curried varargs #41
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
Conversation
and also update SIPs to use the template format so that they are shown where they belong
update sip template
Feedback of the previous SIP committee from November 2019 at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some minor comments below. Otherwise, as a SIP, I believe it is well written. It remains to be seen whether the new SIP committee will be more willing to take it on than the above feedback.
On the Pre-SIP discussion thread, it was argued that this should make it into core because it could be used for the collection constructors, such as List(1, 2)
. However, I do not think that this is likely to happen. First because we can't change the stdlib in 3.x. But even after that, I am afraid that it will harm learning by beginners. It would require learners to grasp this complicated desugaring for basic tasks like creating a list; something that we typically show within 1 hour of learning Scala.
I have a mind to attempt writing it as a macro in Scala 3.
.applyEnd | ||
``` | ||
|
||
Unlike traditional repeated parameters, which restrict the sequence argument at the last position, sequence arguments in a curried call are allowed at any position. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be included in this proposal. The desugaring here doesn't make it any more or less difficult to support _*
arguments in the middle than the regular desugaring. Therefore, it should a separate, orthogonal proposal.
|
||
### Implicit parameters | ||
|
||
A more common form of curried function call would be like `f(a)(b)(c)`. We prefer the explicit named method calls to `applyNext` instead of the common form, in order to support implicit parameters in `applyNext`. Therefore, each explicit parameter might come with an implicit parameter list, resolving the infamous [multiple type parameter lists](https://github.com/scala/bug/issues/4719) issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand how the multiple type parameter lists problem comes into play here. Perhaps multiple using
param lists (?), but that is already fixed in Scala 3.
The multiple type param list problem itself would be fixed by Clause interweaving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps multiple using param lists (?), but that is already fixed in Scala 3.
Right. This document was written before Scala 3.
trait Curried extends Any | ||
``` | ||
|
||
When a function call `f(p1, p2, p3, ... pn)` is being type checked, the compiler will firstly look for `apply` method on `f`. If an applicable `apply` method is not found and `f` is a subtype of `Curried`, the compiler will convert the function call to curried form `f.applyBegin.applyNext(p1).applyNext(p2).applyNext(p3) ... .applyNext(pn).applyEnd`, and continue type checking the translated call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear from this spec how it interacts with the scala.Dynamic
rewrites. Should it be tried before or after the scala.Dynamic
rewrite? Should we prevent a class from extending both scala.Dynamic
and Curried
?
$$this.appendChild($m_Lorg_scalajs_dom_package$().document__Lorg_scalajs_dom_raw_HTMLDocument().createTextNode("line1")); | ||
var $$this$1 = $m_Lorg_scalajs_dom_package$().document__Lorg_scalajs_dom_raw_HTMLDocument().createElement("br"); | ||
$$this.appendChild($$this$1); | ||
$$this.appendChild($m_Lorg_scalajs_dom_package$().document__Lorg_scalajs_dom_raw_HTMLDocument().createTextNode("line2")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This level of optimization works because Scala.js has a whole-program optimizer that very capable. Do we achieve the same level of optimization on Scala/JVM? Do we have any evidence that this rewrite is not JS-specific?
For the record, this is an implementation of the each-arg-gets-its-own- import scala.quoted.*
extension (sc: StringContext)
inline def showMe(inline args: Any*): String = ${ showMeExpr('sc, 'args) }
private def showMeExpr(sc: Expr[StringContext], argsExpr: Expr[Seq[Any]])(using Quotes): Expr[String] = {
argsExpr match {
case Varargs(argExprs) =>
val argShowedExprs: Seq[Expr[String]] = argExprs.map {
case '{ $arg: tp } =>
Expr.summon[Show[tp]] match
case Some(showExpr) =>
'{ $showExpr.show($arg) }
case None =>
val typeRepr = quotes.reflect.TypeRepr.of[tp].widenTermRefByName
quotes.reflect.report.error(s"could not find any implicit Show instance for ${typeRepr.show}", arg)
'{ ??? }
}
val newArgsExpr = Expr.ofSeq(argShowedExprs)
'{ $sc.s($newArgsExpr*) }
case _ =>
// `new StringContext(...).showMeExpr(args: _*)` not an explicit `showMeExpr"..."`
quotes.reflect.report.error(s"Args must be explicit, was ${argsExpr.show}", argsExpr)
'{ ??? }
}
}
trait Show[-T] {
def show(x: T): String
}
given Show[Int] = x => s"Int($x)"
given Show[String] = x => s"Str($x)"
@main def TestShowMe(): Unit =
val i = 5
val s = "hello"
val b = true
println(showMe"foo $i - $s") // OK
println(showMe"foo $i - $s - $b") // could not find any implicit Show instance for scala.Boolean And here is my best attempt so far to implement the full import scala.quoted.*
trait Curried
object Curried:
extension [C <: Curried] (self: C)
transparent inline def apply(inline args: Any*): Any = ${ expandedVarArgs('self, 'args) }
def expandedVarArgs[C <: Curried](self: Expr[C], argsExpr: Expr[Seq[Any]])(using Quotes): Expr[Any] =
import quotes.reflect.*
val selfTerm: Term = self.asTerm
val applyBegin = Select.unique(selfTerm, "applyBegin")
val applyNexts: Term = argsExpr match {
case Varargs(argExprs) =>
argExprs.foldLeft[Term](applyBegin) { (prev, argExpr) =>
argExpr match {
case '{ $arg: tp } =>
Select.overloaded(prev, "applyNext", Nil, List(arg.asTerm))
}
}
case _ =>
report.error(s"Arguments must be explicit, was ${argsExpr.show}", argsExpr)
'{ ??? }.asTerm
}
val applyEnd = Select.unique(applyNexts, "applyEnd")
applyEnd.asExpr
// ------------------
class Builder extends Curried:
private val xs = List.newBuilder[String]
def applyBegin: Builder = this
def applyNext(x: String): Builder =
xs += x
this
def applyNext(x: Int): Builder =
xs += s"Int($x)"
this
def applyEnd: List[String] = xs.result()
@main def TestCurried(): Unit =
val i = 5
val s = "hello"
val builder = new Builder()
val x = builder("foo", i, s, 7)
val y: List[String] = x
println(x) As demonstrated, it works with overloaded |
Unfortunately, existing use cases in Scala 2 have type parameters |
In the interest of making the process go forward, I recommend rejecting this proposal at the upcoming SIP meeting. My overriding concern here is that it introduces 2 ways to encode varargs. That is a pretty big source of complexity in the language, which would require overwhelming needs to pull off its weight. At least one previous attempt at having two ways to encode things in the name of performance, namely the |
I also recommend rejecting this proposal. Supporting two encoding for varargs has a high a maintenance cost. For DSL authors, it seems the option through metaprogramming is still a possibility that's not fully explored (potentially with some improvements to the compiler in this area). |
My recommendation goes against as well. The arguments put forward are:
Performance is not quantified in this proposal, and while we agree that there is some overhead in creating a sequence to pass over the var args, this only matters for hot spots and presumably those are a minority of use cases. When performance matters, it is not too hard to avoid varargs and use the builder pattern. I think it's ok to call appendChild from Scala code directly, without a DSL in between (especially when performance matters). On type safety, the propopsal doesn't make a very compelling case either. The DOM example doesn't benefit from it as far as I can tell. The performance argument does have one good point, which is standard collection builders and how List(x) could be misleading and significantly costlier than x :: Nil. However, I think that could be addressed separately with a different approach that does not require language changes. |
This proposal was rejected during today's SIP meeting. The main reasons were as already written above. In particular, having 2 different ways to encode varargs in the language is a lot of complexity, both for learners and maintainers, that we judged not to be offset by the argued benefits of this proposal. Thank you for the proposal, though. |
This pull request has been automatically created to import proposals from scala/docs.scala-lang