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

Alter compiler generated case class copy to use apply (instead of new) #8827

Closed
scabug opened this issue Aug 27, 2014 · 4 comments · Fixed by scala/scala#10497
Closed

Alter compiler generated case class copy to use apply (instead of new) #8827

scabug opened this issue Aug 27, 2014 · 4 comments · Fixed by scala/scala#10497

Comments

@scabug
Copy link

scabug commented Aug 27, 2014

The copy method is not generated for this example case class and its explicit companion object:

object A {
  def apply(i: Int, j: Int): A =
    new A(i, j) {}
}
abstract case class A private[A] (i: Int, j: Int)

According to the The Scala Language Specification 2.9 in section 5.2.3:

A method named copy is implicitly added to every case class unless the class already has a member (directly defined or inherited) with that name, or the class has a repeated parameter. The method is defined as follows:
def copy[tps](ps'1)...(ps'n): c[tps] = new c[Ts](xs1)...(xsn) 

Since no explicit copy method is defined for the case class, then the compiler generated copy method should exist, right? The specification definition doesn't clarify what to do for the context where the case class has been declared abstract.

Of course, having the compiler blindly generate/implement the copy method as defined in the spec would result in failed compilation as the new operator doesn't exist due to the case class being declared as abstract.

Here are the possible resolution scenarios I can see (listed in least amount of effort to implement order):

  • Update the specification document to cover NOT generating the copy method if the case class is declared abstract
  • Update the specification document to cover using the empty curly braces when generating/implementing the copy method if the case class is declared abstract. And altering the compiler implementation to match the specification update.
  • Update the specification document to cover using the apply method if defined in the (implicit/explicit) case class companion object. When the apply method is not defined, falling back to new operator if the class is not marked abstract. And if the case class has no apply method and is marked abstract, then falling back to not generating the copy implementation. And altering the compiler implementation to match the specification update.

While it is the most work, I strongly prefer the third solution as it follows the principle of "least surprise" while also maximizing what I personally sense to be the core values behind using the case class model.

Whatever the choice, the specification document needs to be altered to cover this undefined context.

@scabug
Copy link
Author

scabug commented Aug 27, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8827?orig=1
Reporter: Jim O'Flaherty (chaotic3quilibrium)

@scabug
Copy link
Author

scabug commented Aug 28, 2014

Jim O'Flaherty (chaotic3quilibrium) said:
After lots of writing and attempting to explore this area, I finally understand both why the compiler generate code uses new (has to do with the contract around constructor inheritance which does not work for the apply method) and why it cannot use new + {} when the class is marked abstract. So, the current compiler behavior appears to be optimal leaving only option 1 as viable; updating the spec to accurately reflect what the compiler is already generating (or not generating in the case of abstract).

@SethTisue
Copy link
Member

leaving only option 1 as viable; updating the spec to accurately reflect what the compiler is already generating (or not generating in the case of abstract)

agree. I've reclassified this as a doc ticket.

(I verified that Scala 3.3.0's behavior is the same: if abstract, no copy.)

@som-snytt
Copy link

Doc tickets should have a label warning that it may require use of semicolon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants