Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default dispatcher is caching things incorrectly for operator method combinations that return/reduce results #98

Open
camsaul opened this issue Aug 18, 2022 · 0 comments
Labels
bug Something isn't working high-priority! more important than the other issues
Milestone

Comments

@camsaul
Copy link
Owner

camsaul commented Aug 18, 2022

The caching mechanism assumes that only the most-specific primary method will be called. If a :toucan is a :bird and a :toucan is a :can, and we have methods for :bird and :can, and :bird is considered more specific, the cache will incorrectly assume that calling the method for :bird is the same as calling the method for :can. Whichever one is invoked first will result in the other getting wrong values. In the normal one-primary-method-only mode the assumption would hold true, but in that assumption is not true for operator method combinations like seq: calling the method for :bird should only give you the result for :bird, while :toucan should give you the result for both :bird and :can.

I'm not sure about the best way to fix this -- we might need some sort of different type of cache for operator method combinations.

As a workaround for the time being we'll have to use uncached multimethods.

Failing test:

(t/deftest operator-method-combination-caching-tets
  (doseq [ks (clojure.math.combinatorics/permutations [:bird :can :toucan])]
    (t/testing (vec ks)
      (let [mf (-> (m/multifn
                    (m/standard-multifn-impl
                     (m/seq-method-combination)
                     (m/standard-dispatcher
                      keyword
                      :hierarchy (atom (-> (make-hierarchy)
                                           (derive :toucan :can)
                                           (derive :toucan :bird))))
                     (m/standard-method-table)))
                   (m/add-primary-method :bird (constantly {:bird? true}))
                   (m/add-primary-method :can (constantly {:can? true}))
                   (m/prefer-method :bird :can))]
        (doseq [k ks]
          (t/testing k
            (t/is (= (case k
                       :bird   {:bird? true}
                       :can    {:can? true}
                       :toucan {:bird? true, :toucan? true})
                     (reduce merge {} (mf k))))))))))
@camsaul camsaul added bug Something isn't working high-priority! more important than the other issues labels Aug 18, 2022
@camsaul camsaul modified the milestones: 0.14.0, 0.15.0, 0.16.0 Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority! more important than the other issues
Projects
None yet
Development

No branches or pull requests

1 participant