Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects versions: Release 1.9
    • Fix versions: None
    • Labels:
    • Approval:
      Triaged
    • Patch:
      Code and Test

      Description

      reduce-kv works as expected on vectors with the element index passed as the "key" argument. However, it fails with a subvec because clojure.lang.APersistentVector$SubVector does not implement IKVReduce.

      (reduce-kv + 0 [1 2 3])
      9

      (reduce-kv + 0 (subvec [1 2 3] 1))
      IllegalArgumentException No implementation of method: :kv-reduce of protocol: #'clojure.core.protocols/IKVReduce found for class: clojure.lang.APersistentVector$SubVector clojure.core/-cache-protocol-fn (core_deftype.clj:583)

      One work around is to copy the subvec into a vector:

      (reduce-kv + 0 (into [] (subvec [1 2 3] 1)))
      6

      Note however, the `vec` would not work here. Since Clojure 1.7, vec will return a subvec rather than copying.

      (reduce-kv + 0 (vec (subvec [1 2 3] 1)))
      IllegalArgumentException No implementation of method: :kv-reduce of protocol: #'clojure.core.protocols/IKVReduce found for class: clojure.lang.APersistentVector$SubVector clojure.core/-cache-protocol-fn (core_deftype.clj:583)

      The Clojure user expects that a subvector supports all of the normal vector operations and this exception is confusing. The cause of the problem is that subvec returns a clojure.lang.APersistentVector$SubVector which does not implement clojure.lang.IKVReduce. PersistentVector inherits from APersistentVector and implements IKVReduce but SubVector doesn't get any IKVReduce support. This was probably just an oversight.

      There are two patches attached. The first fixed the problem by extending the IKVReduce protocol in core.clj. That's convenient for Clojure users to drop into their code as a work-around. The second patch, as suggested by Alex Miller, adds the Java implementation of the IKVReduce interface directly to APersistentVector$SubVector. The second patch is the one that should be reviewed. The same test is included in both patches.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              steveminer@gmail.com Steve Miner
            • Votes:
              4 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: