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

specify case class equals only considers params from first parameter list #5605

Closed
scabug opened this issue Mar 23, 2012 · 10 comments
Closed

specify case class equals only considers params from first parameter list #5605

scabug opened this issue Mar 23, 2012 · 10 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Mar 23, 2012

Welcome to Scala version 2.9.1.final (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_29).
Type in expressions to have them evaluated.
Type :help for more information.

scala> case class Error(x : Int)(y : Int)
defined class Error

scala> Error(1)(0) == Error(1)(2)
res0: Boolean = true

@scabug
Copy link
Author

@scabug scabug commented Mar 23, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5605?orig=1
Reporter: Oscar Boykin (oscar)

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 23, 2012

@VladUreche said:
I think that's the way it's meant to behave. Still, the language spec says otherwise:

Method equals: (Any)Boolean is structural equality, where two instances are equal if they both belong to the case class in question and they have equal (with respect to equals) constructor arguments. (Scala Language Reference, page 68)

It should probably say constructor arguments in the first parameter section.

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 24, 2012

Oscar Boykin (oscar) said:
I can't see why you would want this behavior. Can you give an example?

Also, since this is triggered with implicit parameters, it means a case class with implicit parameters must override hashCode and equals.

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 24, 2012

@VladUreche said:
My usecase was adding additional tagging information to AST nodes. I did not want tagging to influence pattern matching and equality but at the same time I didn't want to have a separate map for the tags. Before I learned about curried case class arguments I used:

case class Node(info: Info) { var tag: SomeTag /* two classses with the same info but with different tags SHOULD be equal */ }

which could be replaced by:

case class Node(info: Info)(var tag: SomeTag)

Indeed, if you care about the implicit parameters in equality and hashCode, then it's a pain. Here's a (long) workaround:

scala> :paste
// Entering paste mode (ctrl-D to finish)

case class A(i: Int, j: String)
object A {
  class AvoidErasure
  implicit val avoidErasure = new AvoidErasure
  // simply defining apply(i: Int)(implicit j: String) will cause the erasure phase to complain
  def apply(i: Int)(implicit j: String, ae: AvoidErasure) = new A(i, j)
}

// Exiting paste mode, now interpreting.

defined class A
defined module A

scala> implicit val theString="hello"
theString: String = hello

scala> A(3).j
res0: String = hello

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 24, 2012

Oscar Boykin (oscar) said:
Thanks for the work-around.

Unfortunately that will be a major pain for our use case because this pattern is very common. I guess it will be easier to avoid case classes and implement equals and hashCode, and more clear about what we are doing.

I do hope this would be labeled as a bug, rather than the language spec changed. It was very confusing to everyone I showed it to and no one expected that the additional parameters would be silently ignored in equals.

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 25, 2012

@Blaisorblade said (edited on Mar 25, 2012 10:51:37 AM UTC):
IMHO, the major problem is that the treatment of "case" classes just does not belong to the core language - it should be part of a library, so that different variants could be easily added, too. I just opened a request for improvement (#5607) about this.

If we had something like

@case class foo(i: Int, j: Int)

it would become easy to support things like:

@case(all_param_lists=true) class foo(i: Int, j: Int)

or whichever other syntax is preferred.

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 25, 2012

@retronym said:
#5607 has been moved to SUGGEST-23

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 25, 2012

@paulp said:

I can't see why you would want this behavior. Can you give an example?

It is by design. An example is anytime you want to be able to construct case classes without every parameter both receiving a field and being considered part of the class's equality contract. Surely examples of this are not hard to come by.

The spec should be made more explicit, but the standard case class behavior isn't likely to change.

Loading

@scabug
Copy link
Author

@scabug scabug commented Oct 15, 2013

@gkossakowski said:
Unassigning and rescheduling to M7 as previous deadline was missed.

Loading

@scabug
Copy link
Author

@scabug scabug commented Mar 13, 2014

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants