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

Comments

Projects
None yet
2 participants
@scabug
Copy link

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

This comment has been minimized.

Copy link
Author

commented Sep 19, 2011

@scabug

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

commented May 12, 2012

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

@scabug

This comment has been minimized.

Copy link
Author

commented May 29, 2012

@lrytz said:
need to update the spec

@scabug

This comment has been minimized.

Copy link
Author

commented Jul 5, 2012

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

@scabug

This comment has been minimized.

Copy link
Author

commented Jul 12, 2012

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

@scabug scabug closed this Jul 12, 2012

@scabug

This comment has been minimized.

Copy link
Author

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
You can’t perform that action at this time.