equality bug on records created with nested calls to map->record

Comment made by: mikerod

Thanks for the clarification Andy. As I mentioned my post may be irrelevant.

I just saw this while scanning the 1.7 changes and figured I'd mention in case there was any more circling back to this (if the patch that was applied proved unsuccessful, etc).

Mike, this bug was closed on Aug 29 2014 by applying the patch mentioned after "Patch:" in the description, which is CLJ-1388-record-equality-and-map-record-factory.patch

You can see the commit in the Clojure commit history on github here: https://github.com/clojure/clojure/commit/62acc2aae934ba4c4d0830fe5664d4aa0107d855

Comment made by: mikerod

If the decision is to go forward with CLJ-1388-record-equality-and-map-record-factory.patch on this what I have to say isn't necessarily relevant.
I didn't see anyone mention this before though, so I want to just make a point about the proposal of changing the use of clojure.core/= to clojure.lang.Util/equals in CLJ-1388v2.patch and any other related.

Changing to Util/equals would actually Clojure record types that are nested in the __extmap no longer have type-sensitive equality checking. I think that would be undesirable.

e.g.

This is false prior to any changes from this Jira.
Swapping = to Util/equals in the Clojure record `equiv` implementation would change the result to true.
In general, a switch from clojure.core/= to clojure.lang.Util/equals for the __extmap would "be contagious" to all nested equality comparisons within the __extmap. That doesn't sound desirable.

Again, it sounds like the direction Alex has mentioned this is taking is CLJ-1388-record-equality-and-map-record-factory.patch. I see no issues with this. Perhaps someone already thought of the above case too and just didn't mention it on here. I wanted to make this point here in case there was any further discussion around the = to Util/equals proposal.

On a side note, I do not believe I can find any tests in clojure.test-clojure.protocols with Clojure records having nested Clojure records within their basis fields or __extmap. So I don't think this example would have been something caught there with the = to Util/equals patch (assuming this is the only test namespace with these sorts of tests).

Screened specifically CLJ-1388-record-equality-and-map-record-factory.patch - use map as is if it supports MapEquivalence (and can thus be compared under a map) and otherwise dump into a clojure map.

From brief conversation with Rich, we should not allow arbitrary map types in __extmap so would prefer to force a clean map and rely on standard equality checking. I think CLJ-1388-record-equality-and-map-record-factory.patch is the preferred path based on that, which still seems like it should avoid copying in nearly all common cases.