Clojure

Docstring of defrecord (and =) does not correctly describe equality behavior for records.

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.2, Release 1.3, Release 1.4
  • Fix Version/s: Release 1.3, Release 1.4
  • Component/s: None
  • Labels:
    None
  • Approval:
    Accepted

Description

http://groups.google.com/group/clojure/browse_frm/thread/eba691b38c45196b#

The docstring of defrecord says it "will define type-and-
value-based equality and hashCode". In reality, record types are included in = but not .equals (or hashCode), and so records of different types can collide as map keys.

Along the same lines, the docstring of = says "same as Java x.equals, except it ... compares numbers and collections in a type-independent manner". To me, this seems to imply the opposite of what actually happens with records.

FWIW I think it would be more clear if the behavior for = and .equals were the same in this respect, but in any case the implemented behavior should be properly documented.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Stuart Sierra added a comment -

Also discussed at https://groups.google.com/d/topic/clojure/e6UhXVny8Xc/discussion

Per Rich, "The policy is: = includes the type and .equals doesn't. In this way records can still be proper j.u.Maps and = remains most useful for records (e.g. including type)."

Show
Stuart Sierra added a comment - Also discussed at https://groups.google.com/d/topic/clojure/e6UhXVny8Xc/discussion Per Rich, "The policy is: = includes the type and .equals doesn't. In this way records can still be proper j.u.Maps and = remains most useful for records (e.g. including type)."
Hide
Permalink
Jason Wolfe added a comment -

On a related note:

http://dev.clojure.org/display/doc/Enhanced+Primitive+Support?focusedCommentId=1573146#comment-1573146

Regarding "hash maps and sets now use = for keys ... will use stricter .equals when calling through java.util interfaces":

Note that "=" is actually stricter than ".equals" for record types currently, because it includes type information.

This has important consequences for how (and whether) hash maps and sets actually obey the java.util interfaces.

For instance, if

(defrecord P []), ...

what should (.get {(P.) 1 (Q.) 2} (Q.)) return? How about if we .get an element of a third type (R.)?

In a similar vein, this behavior seems confusing at best:

user=> (java.util.HashMap. {(P.) 1 (Q.) 2})
#<HashMap {user.P@0=2}>

Show
Jason Wolfe added a comment - On a related note: http://dev.clojure.org/display/doc/Enhanced+Primitive+Support?focusedCommentId=1573146#comment-1573146 Regarding "hash maps and sets now use = for keys ... will use stricter .equals when calling through java.util interfaces": Note that "=" is actually stricter than ".equals" for record types currently, because it includes type information. This has important consequences for how (and whether) hash maps and sets actually obey the java.util interfaces. For instance, if (defrecord P []), ... what should (.get {(P.) 1 (Q.) 2} (Q.)) return? How about if we .get an element of a third type (R.)? In a similar vein, this behavior seems confusing at best: user=> (java.util.HashMap. {(P.) 1 (Q.) 2}) #<HashMap {user.P@0=2}>
Hide
Permalink
Aaron Bedra added a comment -

Open question:

  • the phrase "type-independent" is confusing. It is intended to talk about concrete numeric and collection types, but I can see why it gets confusing with records. Need an idea for two different terms here (and define them someplace). If somebody has one, please add it to this ticket.

Non-issues:

  • = and .equals are not and will not be the same. The whole point is to avoid Java's broken semantics (=) while providing an interop formula for those who need it (.equals)
  • Any types can collide as hash keys.
Show
Aaron Bedra added a comment - Open question:
  • the phrase "type-independent" is confusing. It is intended to talk about concrete numeric and collection types, but I can see why it gets confusing with records. Need an idea for two different terms here (and define them someplace). If somebody has one, please add it to this ticket.
Non-issues:
  • = and .equals are not and will not be the same. The whole point is to avoid Java's broken semantics (=) while providing an interop formula for those who need it (.equals)
  • Any types can collide as hash keys.
Aaron Bedra made changes -
Field Original Value New Value
Issue Type Defect [ 1 ] Enhancement [ 4 ]
Assignee Aaron Bedra [ aaron ]
Fix Version/s Release.Next [ 10038 ]
Priority Major [ 3 ] Minor [ 4 ]
Hide
Permalink
Aaron Bedra added a comment -

Any thoughts on the open questions part Rich?

Show
Aaron Bedra added a comment - Any thoughts on the open questions part Rich?
Aaron Bedra made changes -
Waiting On richhickey
Aaron Bedra made changes -
Assignee Aaron Bedra [ aaron ] Stuart Halloway [ stu ]
Waiting On richhickey stu
Hide
Permalink
Stuart Halloway added a comment -

I think the following is better:

monospaced
In addition, defrecord will define type-and-value-based =,
and will defined Java .hashCode and .equals consistent with the
contract for java.util.Map.
monospaced

But I don't want to hold release for discussion, so bumping this to approved backlog for further bikeshedding.

Show
Stuart Halloway added a comment - I think the following is better: monospaced In addition, defrecord will define type-and-value-based =, and will defined Java .hashCode and .equals consistent with the contract for java.util.Map. monospaced But I don't want to hold release for discussion, so bumping this to approved backlog for further bikeshedding.
Stuart Halloway made changes -
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Release.Next [ 10038 ]
Hide
Permalink
Stuart Halloway added a comment -

Better yet, let's just agreed on my prose and call this done.

Show
Stuart Halloway added a comment - Better yet, let's just agreed on my prose and call this done.
Stuart Halloway made changes -
Status Open [ 1 ] Resolved [ 5 ]
Approval Accepted
Fix Version/s Release.Next [ 10038 ]
Fix Version/s Approved Backlog [ 10034 ]
Resolution Completed [ 1 ]
Fogus made changes -
Affects Version/s Release 1.4 [ 10040 ]
Fogus made changes -
Fix Version/s Release 1.4 [ 10040 ]
Stuart Halloway made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.