Clojure

crafted `vec' call allows created vector to be mutated

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.3, Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Environment:
    Found in 1.3.0; tested in 1.4.0-alpha2 as well.
  • Patch:
    Code
  • Approval:
    Ok

Description

user> (let [a (to-array (repeat 31 0)), v (vec a)]
        [(v 2) (do (aset a 2 'no) (v 2))])
[0 no]
user> (let [a (to-array (repeat 33 0)), v (vec a)]
        [(v 2) (do (aset a 2 'no) (v 2))])
[0 0]

While a relaxation of when different changes to the environment are made may impact the resulting value of the vector, as it is with lazy seqs, it oughtn't be possible to get two different results for the same access, just as with lazy seqs.

Activity

Hide
Stuart Halloway added a comment -

The code path followed here lets vec assume (without copying) a passed-in array. to-array just returns the array, and createOwning doesn't copy either.

Seems like we need a defensive copy somewhere

Show
Stuart Halloway added a comment - The code path followed here lets vec assume (without copying) a passed-in array. to-array just returns the array, and createOwning doesn't copy either. Seems like we need a defensive copy somewhere
Hide
Rich Hickey added a comment -

Just document that, when passed an array, vec aliases it, and it should not be changed. Then only people that can't ensure that must copy it first.

Show
Rich Hickey added a comment - Just document that, when passed an array, vec aliases it, and it should not be changed. Then only people that can't ensure that must copy it first.
Hide
Andy Fingerhut added a comment -

Proposed patch to document the existing behavior, and briefly suggest safe ways to use vec with mutable collections.

Show
Andy Fingerhut added a comment - Proposed patch to document the existing behavior, and briefly suggest safe ways to use vec with mutable collections.
Hide
Stuart Sierra added a comment -

I think the docstring in the 24/Feb/12 patch is too long. Also not entirely correct: calling vec on a mutable java.util.List, for example, will create an immutable vector from the contents of the List.

Instead just say that Java arrays may be aliased by vec and should not be modified.

Show
Stuart Sierra added a comment - I think the docstring in the 24/Feb/12 patch is too long. Also not entirely correct: calling vec on a mutable java.util.List, for example, will create an immutable vector from the contents of the List. Instead just say that Java arrays may be aliased by vec and should not be modified.
Hide
Andy Fingerhut added a comment -

clj-893-doc-vec-aliases-java-array-patch2.txt of Mar 26, 2012 is shorter than the previous attempt, and avoids the error Stuart Sierra found.

Show
Andy Fingerhut added a comment - clj-893-doc-vec-aliases-java-array-patch2.txt of Mar 26, 2012 is shorter than the previous attempt, and avoids the error Stuart Sierra found.
Hide
Rich Hickey added a comment -

I'd prefer it said will alias and "should not be modified" vs "should be copied".

Show
Rich Hickey added a comment - I'd prefer it said will alias and "should not be modified" vs "should be copied".
Hide
Andy Fingerhut added a comment -

Would someone better at writing Clojure style doc strings take a crack at this? I'm too verbose. I have added a (verbose) note about this behavior on clojuredocs.org.

Show
Andy Fingerhut added a comment - Would someone better at writing Clojure style doc strings take a crack at this? I'm too verbose. I have added a (verbose) note about this behavior on clojuredocs.org.
Hide
Brenton Ashworth added a comment -

Added patch

clj-893-doc-vec-aliases-java-array-patch3.txt

With this patch the doc string would be:

Creates a new vector containing the contents of coll. Java arrays
will be aliased and should not be modified.
Show
Brenton Ashworth added a comment - Added patch clj-893-doc-vec-aliases-java-array-patch3.txt With this patch the doc string would be:
Creates a new vector containing the contents of coll. Java arrays
will be aliased and should not be modified.
Hide
Andy Fingerhut added a comment -

Removed non-preferred patch clj-893-doc-vec-aliases-java-array-patch2.txt of Mar 26, 2012.

Show
Andy Fingerhut added a comment - Removed non-preferred patch clj-893-doc-vec-aliases-java-array-patch2.txt of Mar 26, 2012.

People

Vote (1)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: