Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

Some IReduce/IReduceInit implementors don't respect reduced

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:

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

Environment

None

Attachments

2

Activity

Alex Miller November 17, 2014 at 8:47 PM

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

Nicola Mometto November 17, 2014 at 7:59 PM

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

Nicola Mometto November 17, 2014 at 7:39 PM

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

Nicola Mometto November 17, 2014 at 7:36 PM

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.

Alex Miller November 17, 2014 at 6:35 PM

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.

Completed

Details

Assignee

Reporter

Approval

Patch

Priority

Affects versions

Fix versions

Created November 15, 2014 at 12:10 AM
Updated January 9, 2015 at 2:02 PM
Resolved January 9, 2015 at 2:02 PM