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

unadvertised change to case class weight due to copy method #5009

Closed
scabug opened this issue Sep 19, 2011 · 10 comments
Closed

unadvertised change to case class weight due to copy method #5009

scabug opened this issue Sep 19, 2011 · 10 comments

Comments

@scabug
Copy link

scabug commented Sep 19, 2011

In scala 2.7, this class had one field. In scala 2.8 and beyond, it has three.

case class A(x: String)(y: String, z: String) { }

I'm pretty sure that using parameter lists beyond the first has been an advertised way of avoiding field creation for case class parameters. This was undone, since the copy method references all the parameters in all the lists and so fields are created.

You can somewhat distastefully get around this by inhibiting the creation of the copy method.

case class A(x: String)(y: String, z: String) {
  private def copy = "no fields for y and z"
}

This business should at least be documented.

@scabug
Copy link
Author

scabug commented Sep 19, 2011

Imported From: https://issues.scala-lang.org/browse/SI-5009?orig=1
Reporter: @paulp

@scabug
Copy link
Author

scabug commented Oct 4, 2011

@paulp said:
For the record, martin says this is a bug, and we should changes things in some fashion to get back to one field.

@scabug
Copy link
Author

scabug commented Oct 4, 2011

@adriaanm said:
original motivation for multiple argument lists for cases classes: don't need field for those args, not relevant for pattern matching

but, now we have a copy method:
[ - don't copy those fields?]
--> eta-expand the copier for the trailing argument lists

@scabug
Copy link
Author

scabug commented Oct 9, 2011

@paulp said:
To clarify adriaan's comment for the record (I'm thinking he hooked the tubes up to his head to catch the runoff) it was proposed, I think with general agreement, that case classes with a single parameter list be treated as before, and those with multiple have a copy method which returns the eta-expansion. An unusually satisfying solution really.

case class Foo(x: Int)(y: Int, z: Int) {
  <synthetic> def copy(x: Int = this.x) = new Foo(x)(_: Int, _: Int)
}

@scabug
Copy link
Author

scabug commented May 8, 2012

@adriaanm said (edited on May 8, 2012 3:10:33 PM UTC):
Martin says: case class ness is only bestowed on the first argument list
the rest should not be copied. but we still need values for them to call the constructor

@scabug
Copy link
Author

scabug commented May 12, 2012

@lrytz said:
fixed in [https://github.com/scala/scala/commit/40e7cab7a22a8531bdd310bfb57fda51b798c690]

@scabug
Copy link
Author

scabug commented May 29, 2012

@lrytz said:
need to update the spec

@scabug
Copy link
Author

scabug commented Jul 5, 2012

@lrytz said:
refined in scala/scala@6c7f2b6

@scabug
Copy link
Author

scabug commented Jul 12, 2012

@lrytz said:
closing this, pending spec change in #6068

@scabug scabug closed this as completed Jul 12, 2012
@scabug
Copy link
Author

scabug commented Nov 18, 2016

Tushar Kapila (tgkprog) said:
Wish there was an annotation or some signal saying we want this in a copyAll function do we do not need to spin our own. It is useful to have them copied over in many cases.

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

No branches or pull requests

2 participants