- 
                Notifications
    You must be signed in to change notification settings 
- Fork 29
SIP-76 - Unpack Case Classes into Parameter Lists and Argument Lists #114
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
base: main
Are you sure you want to change the base?
Conversation
        
          
                content/unpack.md
              
                Outdated
          
        
      |  | ||
| | Date | Version | | ||
| |-------------|--------------------| | ||
| | 23 Aug 2024 | Initial Draft | | 
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.
| | 23 Aug 2024 | Initial Draft | | |
| | 23 Aug 2025 | Initial Draft | | 
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 think the use case addressed by the proposal is important.
The proposal is very long and it seems there are lots of interactions with other features. This makes me a bit hesitant whether an implementation would be feasible. I believe we can reduce complexity and feature interactions dramatically if we pass unpack arguments as a single object instead of passing each field separately.
That's also what Python does and what Kotlin seems to propose. The next to last comment on the Kotlin KEEP seems to indicate this:
It is still planned to be experimental in 2.2 ?
Unfortunately, no. We have a design and a prototype, but to make the feature both expressive and performant, we need scalarization to avoid extra boxing on calling a method. We'd like to achieve an experience where developers can move from a bunch of parameters in their function signatures to a class-centric approach without sacrificing performance.
So we need more time to experiment. However, we’re cautiously confident that the semantics of datargs, combined with Valhalla’s value classes, can help with runtime performance, but it needs some time.
I fields were passed individually there would be no need for value classes, so this seems to indicate that fields are in fact passed in bulk.
So my recommendation would be to rewrite the proposal so that
- An unpack parameter is passed into just like a regular parameter.
- At the call site, we transparently construct an unpack class object from individual parameters.
- Also at the call site, if we have an argument c*wherec'stype is a case class instance we expand to named parameters as described in the proposal.
| 
 @odersky The challenge with this is that we want to support the ability to migrate a non-unpacked  One option we discussed before is that rather than constructing the case class at every call site, we lower the  
 (1) never needs to be externally callable, and we can have all calls to the method go through (2). This will be functionally equivalent to constructing the  This seems like it should give us the best of both worlds, with both the implementation simplicity and the the user-facing compatibility? | 
| 
 In that scenario, I expect the library developer to add an appropriate  If we generate the forwarders automatically, we will ease that specific migration scenario. However, we will make another migration scenario much harder: that of adding one field in the unpack class. I expect the latter to be a much more common scenario than the former. So I don't think the added forwarder is a benefit. | 
| Unpacking should work for generic methods and `case class`es: | ||
|  | ||
| ```scala | ||
| case class Vector[T](x: T, y: T) | 
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.
My guess is Vector[T] needs to be Point[T]
| I'm missing what should happen in: 
 def foo[T <: Product](unpack t: T): Unit = {}
 def foo(using unpack config: RequestConfig): Unit = {} | 
| 
 This case would already be handled by usage of  
 So combined, this would allow us all three migrations without breaking binary compatibility: non- And the user would need to add an  | 
| Ouch. I had not seen that. That solves this particular issue, but poses even bigger problems. It means you can't use an unpacked case class anywhere  With the design Martin and I are advocating, it's unnecessary anyway. You get that compatibility for free, without the features having to interact at all. | 
| 
 That isn't quite accurate; the flat design means you can't use an unpacked case class and evolve the method in a binary compatible manner anywhere  The boxed design provides additional bincompat evolution opportunities over what is allowed today, in that you can add  So while the boxed design would add some opportunity for evolution of already- The flat design would not improve upon the evolution of  | 
| I really believe you're overestimating the proportion of scenarios of a) evolve from pre- Scala does not have a history of designing new replacement features so that they are binary compatible with what we had before (see for example  | 
| It's not just "what we had before" for Scala as a language, but "what they had before" for every individual library and callsite. Even after this feature is released and in the wild for a decade, everything I said still applies. People using large flat argument sets rarely know what the "meaningful" groupings of those parameters are until after they have grown a long list of them which have been out there for a while. That means they will not be able to up-front define the  You've said yourself you never use large lists of default parameters, so maybe this use case never arises for you. But it has arisen for me many times in the libraries I maintain, which is why I opened this SIP. Every example described in the proposal falls under the category above. I maintain 5-6 libraries which match this exactly. If you don't believe me and don't believe the examples in the proposal that show that this use case is common, there isn't really anything else I can say | 
| To take a concrete example from the proposal, consider the potential usage of  In March 2018, uPickle started off with a single  def write[T: Writer](t: T)Later that month, it grew an  def write[T: Writer](t: T, indent: Int = -1): StringLater that month, it grew a  def write[T: Writer](t: T, indent: Int = -1): String
def writeTo[T: Writer](t: T, out: java.io.Writer, indent: Int = -1): UnitShould  7 months later, in October 2018, both methods grew an  def write[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = true): String 
def writeTo[T: Writer](t: T, out: java.io.Writer, indent: Int = -1, escapeUnicode: Boolean = true): UnitNow it seems more clear we should move the shared  In November 2018, the API grew an additional  def write[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = false): String 
def writeTo[T: Writer](t: T, out: java.io.Writer, indent: Int = -1, escapeUnicode: Boolean = false): Unit
def writeToByteArray[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = false): Array[Byte]A year later, in December 2019, it grew a  def write[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = true): String 
def writeTo[T: Writer](t: T, out: java.io.Writer, indent: Int = -1, escapeUnicode: Boolean = true): Unit
def writeToByteArray[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = false): Array[Byte]
def stream[T: Writer](t: T,
                        indent: Int = -1,
                        escapeUnicode: Boolean = false): geny.Writable2 years later in March 2021, the API grew  def write[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = false): String 
def writeTo[T: Writer](t: T, out: java.io.Writer, indent: Int = -1, escapeUnicode: Boolean = false): Unit
def writeToByteArray[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = false): Array[Byte]
def stream[T: Writer](t: T,
                        indent: Int = -1,
                        escapeUnicode: Boolean = false): geny.Writable
def writeToOutputStream[T: Writer](t: T, out: java.io.OutputStream, indent: Int = -1, escapeUnicode: Boolean = false): Unit3 years later, in September 2024, all APIs grew a  def write[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = false, sortKeys: Boolean = false): String 
def writeTo[T: Writer](t: T, out: java.io.Writer, indent: Int = -1, escapeUnicode: Boolean = false, sortKeys: Boolean = false): Unit
def writeToByteArray[T: Writer](t: T, indent: Int = -1, escapeUnicode: Boolean = false, sortKeys: Boolean = false): Array[Byte]
def stream[T: Writer](t: T,
                        indent: Int = -1,
                        escapeUnicode: Boolean = false,
                        sortKeys: Boolean = false): geny.Writable
def writeToOutputStream[T: Writer](t: T, out: java.io.OutputStream, indent: Int = -1, escapeUnicode: Boolean = false, sortKeys: Boolean = false): UnitThe moral of this story is that with the boxed design, there is no point in the evolution of this uPickle library that using  
 If you never use default parameters then obviously this won't apply to you, but for anyone who does use default parameters this is a story that is told again and again. In ~every project in the scala-toolkit or com-lihaoyi platform: uPickle, os-lib, requests-scala, pprint, mill, cask, we can see the same pattern playing out. With the status quo, or even with the boxed  And that is, after all, the motivation for this proposal in the first place | 
| 
 I already mentioned the solution: a single, one-time  | 
| Does  | 
| No, you'll need to keep the full signature exactly as it was before you decided to migrate to  It doesn't solve the duplication that you already had in your to before you migrated, but it won't accumulate more duplication after you migrate. Again, that's quite similar to other migration scenarios, for example when you deprecate methods in favor of new ones that are better designed: you have to keep the deprecated signature exactly as it was before your refactoring, but then you can forget about it. | 
No description provided.