ClojureScript

The pop function in PersistentVector is buggy

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:

Description

Example:

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

  1. fix-persistentvector-pop.diff
    19/Apr/12 3:48 AM
    1.0 kB
    Nicola Mometto
  2. fix-persistentvector-pop.diff
    18/Apr/12 12:50 PM
    1 kB
    Nicola Mometto
  3. fix-persistentvector-pop-with-tests.diff
    19/Apr/12 10:15 AM
    2 kB
    Nicola Mometto

Activity

Hide
Nicola Mometto added a comment -

sure, here it is

Show
Nicola Mometto added a comment - sure, here it is
Hide
Laszlo Török added a comment -

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))))

Show
Laszlo Török added a comment - 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))))
Hide
Nicola Mometto added a comment -

right, using slice is even faster.
patch updated

Show
Nicola Mometto added a comment - right, using slice is even faster. patch updated
Hide
Laszlo Török added a comment -

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

Show
Laszlo Török added a comment - 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
Hide
Nicola Mometto added a comment -

this patch fixes pop

Show
Nicola Mometto added a comment - this patch fixes pop

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: