into does not work with IReduceInit
Description
Environment
Attachments
Activity
Nicola Mometto December 23, 2014 at 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 at 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 at 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 at 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 at 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.
Details
Details
Assignee
Reporter
Alex MillerLabels
Approval
Patch
Priority
Major
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