Clojure

{:as x} destructuring with an empty list raises exception

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.5
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

user=> (clojure-version)
"1.4.0"
user=> (let [{:as x} '()] x)
{}

...

user=> (clojure-version)
"1.5.0-RC1"
user=> (let [{:as x} '()] x)
IllegalArgumentException No value supplied for key: null  clojure.lang.PersistentHashMap.create (PersistentHashMap.java:77)

The bug was introduced by a change[1] to support duplicate keys in map
destructuring. Using PersistentHashMap/create here introduces the above
bug, since it does not properly handle empty lists.

[1]: https://github.com/clojure/clojure/commit/93c795fe10ee5c92a36b6ec6373b3c80a31135c4

Activity

Toby Crawley made changes -
Field Original Value New Value
Patch Code and Test [ 10002 ]
Attachment empty-list-destructuring-CLJ-1140-12.30.12.diff [ 11782 ]
Hide
Toby Crawley added a comment -

There's been some discussion on clojure-dev around this issue: https://groups.google.com/d/topic/clojure-dev/qdDRNfEVfQ8/discussion

Show
Toby Crawley added a comment - There's been some discussion on clojure-dev around this issue: https://groups.google.com/d/topic/clojure-dev/qdDRNfEVfQ8/discussion
Rich Hickey made changes -
Approval Ok [ 10007 ]
Fix Version/s Release 1.5 [ 10150 ]
Hide
Toby Crawley added a comment -

An issue I brought up in the email thread is consistency: vector binding works with anything nthable, map binding works with anything associative. With my current patch (empty-list-destructuring-CLJ-1140-12.30.12.diff), only ISeqs are supported for kwarg map binding.

I'd prefer it work with anything seqable, and can provide a patch that does that. I would go ahead and do so, but now that this ticket is now Approval: OK, I didn't want to alter what had been OK'ed. Let me know if you want a patch that adds support for anything seqable.

Show
Toby Crawley added a comment - An issue I brought up in the email thread is consistency: vector binding works with anything nthable, map binding works with anything associative. With my current patch (empty-list-destructuring-CLJ-1140-12.30.12.diff), only ISeqs are supported for kwarg map binding. I'd prefer it work with anything seqable, and can provide a patch that does that. I would go ahead and do so, but now that this ticket is now Approval: OK, I didn't want to alter what had been OK'ed. Let me know if you want a patch that adds support for anything seqable.
Rich Hickey made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
Stuart Halloway made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (3)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: