Skip to content
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

SIP-NN - Curried varargs #1487

Merged
merged 3 commits into from Aug 26, 2019
Merged

SIP-NN - Curried varargs #1487

merged 3 commits into from Aug 26, 2019

Conversation

@Atry
Copy link
Contributor

Atry commented Aug 11, 2019

@lihaoyi

This comment has been minimized.

Copy link

lihaoyi commented Aug 25, 2019

I like this a lot:

  • It is a natural solution to the “should varargs be mutable or inmutable or readonly Seq” question: with a builder, you can make your vararg take exactly the type you need, even unusual ones like Map or Set or scalatags.text.Builder, and have it be constructed directly without overhead

  • It provides more information to the compiler and optimizer: where the number of args at a callsite is fixed, simple inlining produces a fixed set of instructions to run on a builder, which makes further optimization possible. The current Seq-based model obfuscates everything by forcing the params into a variable length homogenous sequence of boxes, which any optimizer would have to deobfuscate to figure out is fixed length and homogenous

  • The fact that the builder’s method calls can be overloaded can remove yet another layer of boxing when taking heterogenous varargs: currently we have to take varargs of a supertype/trait and have the possible parameter types boxed into compliance with the vararg type via implicits

  • It has plenty of precedence: it’s just the Visitor Pattern or Finally Tagless. Like how uPickle uses visitors to abstract over multiple ASTs and JSON libraries with a performance boost instead of a penalty, or SAX parsers allow efficient streaming XML transforms unlike DOM based approaches. This would let varargs abstract over multiple possible collection types, and even non collection types, also with a performance boost

  • The fact that it could allow a heterogenous set of types to be inferred from a variadic argument list opens up a lot of possibilities, e.g. it makes translation of Tuple.apply(x, y, z) calls into HList-esque tuples trivial to implement in userland, a nice Rec(“foo” -> bar) syntax for record-types also entirely in userland, and probably other things that I can’t come up with off the top of my head

  • I think this would allow the current awkward builder-based applicative zipMap syntax (foo |@| bar |@| baz)(f) to be written more ergonomically as zipMap(foo, bar, baz)(f) while still being variadic

These benefits are all on top of those that Yang Bo lists, e.g. better perf for XML literal construction, string interpolators, and collection companion factories.

It’s not common that a new technique allows both better abstraction, better flexibility, and better performance. This definitely seems like one of those cases. Details will need to be worked out, but in principle this is great!

@valenterry

This comment has been minimized.

Copy link
Contributor

valenterry commented Aug 25, 2019

  • The fact that it could allow a heterogenous set of types to be inferred from a variadic argument list opens up a lot of possibilities, e.g. it makes translation of Tuple.apply(x, y, z) calls into HList-esque tuples trivial to implement in userland, a nice Rec(“foo” -> bar) syntax for record-types also entirely in userland, and probably other things that I can’t come up with off the top of my head

I agree! This would be huge for making dependently typed apis much more user friendy.

@sjrd
sjrd approved these changes Aug 26, 2019
Copy link
Member

sjrd left a comment

Thank you for the proposal, @Atry. We'll discuss it at the next SIP meeting. There is not scheduled date yet.

@sjrd sjrd merged commit 320fb0e into scala:master Aug 26, 2019
1 check passed
1 check passed
continuous-integration/drone/pr the build was successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.