<< Back to previous view

[CLJS-183] The pop function in PersistentVector is buggy Created: 18/Apr/12  Updated: 27/Jul/13  Resolved: 19/Apr/12

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Erik Ouchterlony Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug

Attachments: File fix-persistentvector-pop.diff     File fix-persistentvector-pop.diff     File fix-persistentvector-pop-with-tests.diff    

 Description   

Example:

(conj (pop [1 2]) 3) => [1 2]



 Comments   
Comment by Nicola Mometto [ 18/Apr/12 10:22 AM ]

this patch fixes pop

Comment by Laszlo Török [ 19/Apr/12 2:38 AM ]

Hi,

good catch, but why not simply:

(PersistentVector. meta (dec cnt) shift root (.slice tail 0 -1))
;; you don't need to aclone and splice, .slice returns a new array

Comment by Nicola Mometto [ 19/Apr/12 3:48 AM ]

right, using slice is even faster.
patch updated

Comment by Laszlo Török [ 19/Apr/12 6:01 AM ]

Nice, thank you!

Could you please add a test that covers the issue?

e.g.

(assert (= (vec (range 33))
(-> (vec (range 33))
(pop)
(pop)
(conj 31)
(conj 32))))

Comment by Nicola Mometto [ 19/Apr/12 10:15 AM ]

sure, here it is

Comment by David Nolen [ 19/Apr/12 10:31 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/053b7fd9cbb0a24617592f490893d6a746e54ec7

Generated at Tue Oct 21 10:34:59 CDT 2014 using JIRA 4.4#649-r158309.