[#CLJS-183] The pop function in PersistentVector is buggy

[CLJS-183] The pop function in PersistentVector is buggy Created: 18/Apr/12 Updated: 19/Apr/12 Resolved: 19/Apr/12
Status:	Resolved
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:

Labels:

bug

Attachments:

fix-persistentvector-pop.diff

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 ]

Generated at Tue May 21 15:18:24 CDT 2013 using JIRA 4.4#649-r158309.