Clojure

clojure.walk should support records

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.4
  • Fix Version/s: Release 1.6
  • Component/s: None
  • Labels:
  • 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.

Activity

Stuart Halloway made changes -
Field Original Value New Value
Approval Vetted [ 10003 ]
Fix Version/s Approved Backlog [ 10034 ]
Rich Hickey made changes -
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Release 1.6 [ 10157 ]
Alex Miller made changes -
Description Using clojure.walk functions fails surprisingly for data containing records defined with defrecord:

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)

This seems to be because clojure.walk/walk guards a call to (empty form) with a (coll? form) check. The check succeeds because records implement IPersistentCollection, but (empty form) throws an exception. This looks to me like a bug in clojure.walk (it should check records separately and either treat them as atomic or implement a way of walking through them) but perhaps it is a sign of some unclarity in the contract of collections.
*Problem:* clojure.walk predates the existence of records and will cause exceptions if used with records.

For example:

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)

This seems to be because clojure.walk/walk guards a call to (empty form) with a (coll? form) check. The check succeeds because records implement IPersistentCollection, but (empty form) throws an exception. This looks to me like a bug in clojure.walk (it should check records separately and either treat them as atomic or implement a way of walking through them) but perhaps it is a sign of some unclarity in the contract of collections.
Issue Type Defect [ 1 ] Enhancement [ 4 ]
Summary defrecord classes implement IPersistentCollection but not .empty, clojure.walk assumes collections support empty clojure.walk should support records
Stuart Sierra made changes -
Assignee Stuart Sierra [ stuart.sierra ]
Stuart Sierra made changes -
Patch Code and Test [ 10002 ]
Attachment 0001-CLJ-1105-Support-records-in-clojure.walk.patch [ 12076 ]
Stuart Sierra made changes -
Description *Problem:* clojure.walk predates the existence of records and will cause exceptions if used with records.

For example:

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)

This seems to be because clojure.walk/walk guards a call to (empty form) with a (coll? form) check. The check succeeds because records implement IPersistentCollection, but (empty form) throws an exception. This looks to me like a bug in clojure.walk (it should check records separately and either treat them as atomic or implement a way of walking through them) but perhaps it is a sign of some unclarity in the contract of collections.
*Problem:* clojure.walk throws exceptions if used on records.

*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.

*Demonstration:*

{code}
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)
{code}

This seems to be because clojure.walk/walk guards a call to (empty form) with a (coll? form) check. The check succeeds because records implement IPersistentCollection, but (empty form) throws an exception. This looks to me like a bug in clojure.walk (it should check records separately and either treat them as atomic or implement a way of walking through them) but perhaps it is a sign of some unclarity in the contract of collections.
Alex Miller made changes -
Labels walk
Alex Miller made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Description *Problem:* clojure.walk throws exceptions if used on records.

*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.

*Demonstration:*

{code}
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)
{code}

This seems to be because clojure.walk/walk guards a call to (empty form) with a (coll? form) check. The check succeeds because records implement IPersistentCollection, but (empty form) throws an exception. This looks to me like a bug in clojure.walk (it should check records separately and either treat them as atomic or implement a way of walking through them) but perhaps it is a sign of some unclarity in the contract of collections.
*Problem:* clojure.walk throws exceptions if used on records.

{code}
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)
{code}

*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.
Alex Miller made changes -
Assignee Stuart Sierra [ stuart.sierra ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (2)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved: