Clojure

Some IReduce/IReduceInit implementors don't respect reduced

Details

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

Description

Several reduce implementations don't properly respect reduced:

  • clojure.core.ArrayChunk's implementation of IChunk/reduce
  • VecSeq's impl of InternalReduce/reduce
  • APersistentVector's reduce with init doesn't unwrap reduced on last value
  • seqs of primitive arrays don't unwrap reduced on last value
  • PersistentList doesn't unwrap reduced on last value

Some examples:

user=> (transduce (take 1) conj (seq (long-array [1 2 3 4])))
#<Reduced@38f774f8: [1]>
user=> (.reduce (list 1 2 3 4 5) (fn [_ a] (if (= a 5) (reduced "foo"))) 1)
#<Reduced@753d01cc: "foo">

Patch: 0001-ensure-IReduce-IReduceInit-implementors-respect-redu.patch
Screened by: Alex Miller
See also: http://dev.clojure.org/jira/browse/CLJ-1537

Activity

Hide
Alex Miller added a comment -

The patch should only be considering the result of calling the reducing function, not checking the init value (this matches what we do elsewhere).

Also, needs at least some simple example tests.

Show
Alex Miller added a comment - The patch should only be considering the result of calling the reducing function, not checking the init value (this matches what we do elsewhere). Also, needs at least some simple example tests.
Hide
Nicola Mometto added a comment -

While reworking my patch to address your comment, I discovered that PersistentList and APersistentList's IReduceInit/reduce implementation aren't handling correctly reduced when the reducing function returns one on the last iteration.

The attached patch fixes those too and contains testcases demonstrating the issues.

Show
Nicola Mometto added a comment - While reworking my patch to address your comment, I discovered that PersistentList and APersistentList's IReduceInit/reduce implementation aren't handling correctly reduced when the reducing function returns one on the last iteration. The attached patch fixes those too and contains testcases demonstrating the issues.
Hide
Nicola Mometto added a comment - - edited

I haven't fixed the IReduce/IReduceInit implementations for range as that's in scope for http://dev.clojure.org/jira/browse/CLJ-1515

Show
Nicola Mometto added a comment - - edited I haven't fixed the IReduce/IReduceInit implementations for range as that's in scope for http://dev.clojure.org/jira/browse/CLJ-1515
Hide
Nicola Mometto added a comment -

As Ghadi Shayban noticed, while reduce doesn't use IReduceInit's reduce impl for PersistentList, transduce does so this might be cause of serious bugs even from clojure code, not only when using `.reduce` calls

Show
Nicola Mometto added a comment - As Ghadi Shayban noticed, while reduce doesn't use IReduceInit's reduce impl for PersistentList, transduce does so this might be cause of serious bugs even from clojure code, not only when using `.reduce` calls
Hide
Alex Miller added a comment -

reduce will use IReduceInit's reduce impl for PersistentList, after CLJ-1572.

Show
Alex Miller added a comment - reduce will use IReduceInit's reduce impl for PersistentList, after CLJ-1572.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: