into does not work with IReduceInit

Description

This should work:

Cause: CollReduce only supports IReduce, not IReduceInit so when reduce calls into it, it falls back to trying to obtain a seq representation which fails.

Proposed: Extend CollReduce to IReduceInit and in the non-init arity, cast to IReduce. Also, now that CollReduce supports both IReduceInit and Iterable, a coll that implements both makes the path through CollReduce nondeterministic. transduce does an explicit check that prefers IReduceInit - the patch copies that approach to reduce as well.

Another consequence of this change is that since PersistentVector implements IReduce but throws on the non-init path, there are some test breakages. To address this, (which implements the non-init reduce) must be applied first.

Patch: clj-1572-4.patch
Depends on: being applied first

Environment

None

Activity

Show:
Nicola Mometto
December 23, 2014, 10:36 AM

I feel like all those issues introduced by the non-deterministic dispatch of protocol functions in case of multiple available implementations, could (and should?) be solved by a prefer-method-like capability for protocols.
This way we could say have a bunch of hints like (prefer-dispatch CollReduce IReduce Iterable) and be done with it.

Ghadi Shayban
December 23, 2014, 4:58 AM

No I may just be confused.

Rubber-ducking aloud so that I can be corrected:

A call to init reduce with an 'init' supplied is unambiguous.

It's the responsibility of an IReduce to do what is appropriate with 'f', whereas with "improved" reduce and transduce (f) becomes the init. (c.c.reducers/reduce being the improved reduce)

IReduce implementations must preserve compatibility with core/reduce's docstring in the 0 and 1 arity cases.

If coll contains no
items, f must accept no arguments as well, and reduce returns the
result of calling f with no arguments. If coll has only 1 item, it
is returned and f is not called. If val is supplied, returns the
result of applying f to val and the first item in coll, then
applying f to that result and the 2nd item, etc. If coll contains no
items, returns val and f is not called.

A protocol's dispatch is non-deterministic when invoked upon things with multiple paths. One way to resolve the ambiguities in CollReduce is to extend CollReduce directly to any IReduce/IReduceInit impl and not rely upon friends like Iterable.

For example needs the same CollReduce extension as CLJ-1603's Iterable/Repeat/Cycle got.

Nicola Mometto
December 22, 2014, 9:11 PM

Ghadi, what would the rationale be for PV not supporting the no-init arity? I'm not aware of any technical issues caused by my patch for but maybe you know about one?

Ghadi Shayban
December 22, 2014, 9:04 PM

Nicola, that is my impression, that Rich intended PersistentVector to be IReduceInit but not IReduce. But he changed it before that interface was split. Would still need some sort of way to handle the existing no-init case, which he mentioned was unfortunate at the conj.

Nicola Mometto
December 17, 2014, 11:20 PM

An alternative fix would be to just make PersistentVectors IReduceInit rather than IReduce but I don't see the point in doing that since the implementation is trivial.

Completed
Your pinned fields
Click on the next to a field label to start pinning.

Assignee

Unassigned

Reporter

Alex Miller

Labels

Approval

Ok

Patch

Code and Test

Priority

Major

Affects versions

Fix versions