<< Back to previous view

[CLJ-1105] clojure.walk should support records Created: 08/Nov/12  Updated: 14/Feb/14  Resolved: 22/Nov/13

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: Release 1.6

Type: Enhancement Priority: Minor
Reporter: Jouni K. Seppänen Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: walk

Attachments: Text File 0001-CLJ-1105-Support-records-in-clojure.walk.patch    
Patch: Code and Test
Approval: Ok

 Description   

Problem: clojure.walk throws exceptions if used on records.

user=> (defrecord Foo [x])
user.Foo
user=> (def f (Foo. :x))
#'user/f
user=> (use 'clojure.walk)
nil
user=> (postwalk identity {:foo f})
UnsupportedOperationException Can't create empty: user.Foo  user.Foo (NO_SOURCE_FILE:1)

Current Patch: 0001-CLJ-1105-Support-records-in-clojure.walk.patch adds a special case for records.

See also: CLJ-1239 "faster, more flexible dispatch for clojure.walk" which could supersede this ticket.

Screened by: Alex Miller - I think this will likely be superseded by CLJ-1239 in the future, but this is a reasonable short-term step.



 Comments   
Comment by Nicola Mometto [ 08/Nov/12 2:35 PM ]

maybe clojure should follow clojurescript's footsteps and move empty out of IPersistentCollection and create an
interface IEmptyableCollection extends IPersistentCollection { IEmptyableCollection empty(); }

Comment by Stuart Halloway [ 25/Nov/12 6:39 PM ]

Can whoever claims this please consider walk's behavior in the face of all different collection types? I think it also fails with Java collections.

Also, the collection partitioning code in clojure.data may be of use.

Comment by Stuart Sierra [ 26/Jul/13 7:39 AM ]

clojure.walk can be made to support records directly, see https://github.com/stuartsierra/clojure.walk2

Comment by Alex Miller [ 26/Jul/13 10:18 AM ]

Update title and adjust description slightly to focus this ticket on being an enhancement to make clojure.walk work with records.

Comment by Stuart Sierra [ 29/Jul/13 12:15 PM ]

Attachment 0001-CLJ-1105-Support-records-in-clojure.walk.patch is a minimal patch adding support for records without altering the design of clojure.walk.

Comment by Nicola Mometto [ 29/Jul/13 12:45 PM ]

clojure.walk is not required during bootstrapping, so I don't see why we couldn't just substitute clojure.walk with clojure.walk2 given that it only changes the internal implementation to a faster one

Comment by Chouser [ 14/Feb/14 1:50 PM ]

The way this patch behaves can be surprising compared to regular maps:

(clojure.walk/prewalk-replace {[:a 1] nil} {:a 1, :b 2})
;=> {:b 2}

(defrecord Foo [a b])
(clojure.walk/prewalk-replace {[:a 1] nil} (map->Foo {:a 1, :b 2}))
;=> #user.Foo{:a 1, :b 2}

Note how the [:a 1] entry is removed from the map, but not from the record.

I believe CLJ-1239 also demonstrates this behavior.

Comment by Andy Fingerhut [ 14/Feb/14 1:54 PM ]

This comment would probably get more visibility on CLJ-1239, given that this one is closed and CLJ-1239 is still open.

Generated at Thu Jul 31 18:42:50 CDT 2014 using JIRA 4.4#649-r158309.