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

Can we subclass clojure.lang.MultiFn? #34

Open
camsaul opened this issue Apr 15, 2020 · 5 comments
Open

Can we subclass clojure.lang.MultiFn? #34

camsaul opened this issue Apr 15, 2020 · 5 comments
Labels
enhancement New feature or request high-priority! more important than the other issues
Milestone

Comments

@camsaul
Copy link
Owner

camsaul commented Apr 15, 2020

So the usual functions like clojure.core/defmethod and the like can be used. This will make Methodical a true drop-in replacement.

Need to figure out how this will work since clojure.lang.MultiFn is a mutable object and .addMethod actually changes its state, while Methodical multimethods are immutable, so you can only add methods and the like with alter-var-root or equivalent.

Not sure it would actually be possible without reworking Methodical multimethods so they are mutable objects

@camsaul camsaul changed the title Can we subclass of clojure.lang.MultiFn? Can we subclass clojure.lang.MultiFn? Apr 23, 2020
@camsaul camsaul added high-priority! more important than the other issues enhancement New feature or request and removed high-priority! more important than the other issues labels Apr 23, 2020
@acobster
Copy link

If I'm understanding correctly, the goal of this is to allow users to use advanced methodical techniques even on multimethods defined upstream in library code out of their control? So for example:

(ns my.project
  (require
    [methodical.core :as m]
    [some.upstream.lib :as lib]))

(m/defmethod lib/vanilla-multimethod :before :default [x]
  (do-stuff x))

Because that would be truly awesome.

Maybe I'm just missing something, but is it possible to accomplish this by delegating to clojure.lang.MultiFn rather than inheriting from it? So in the above example, m/defmethod would detect in some way that lib/vanilla-multimethod is an instance of clojure.lang.MultiFn and call (.addMethod lib/vanilla-multimethod ...) instead of your standard implementation. I believe you could even do this with a protocol that encapsulates only your util fns that map directly on to MultiFn's public methods, such as add-primary-method!:

(defprotocol MethodicalMultiFn
  (add-primary-method! [multifn-var dispatch-val f])
  ...)

(extend-protocol MethodicalMultiFn
  clojure.lang.MultiFn
  (add-primary-method! [multifn-var dispatch-val f]
    (.addMethod (deref multifn-var) dispatch-val f))
  ...

  StandardMultiFn
  (add-primary-method! [multifn-var dispatch-val f]
    ...standard impl...))

I only just started looking at this and don't really understand how auxiliary methods are implemented yet, but in theory as long as your Var -> MethodTable mapping code doesn't care whether the var is a vanilla vs. methodical instance, I think your auxiliary method implementations won't need to change.

@camsaul
Copy link
Owner Author

camsaul commented Apr 29, 2021

@acobster I'm not sure we'd be able to support aux methods on vanilla multimethods. I was thinking more that we'd let you use clojure.core/defmethod and the like on a Methodical multimethod -- this would let you change your own defmultis to Methodical ones without breaking usage of it.

I think this should be possible, because clojure.lang.MultiFn isn't final: https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/MultiFn.java#L18

@camsaul camsaul added the high-priority! more important than the other issues label Jun 4, 2021
@camsaul
Copy link
Owner Author

camsaul commented Jun 4, 2021

For ClojureScript, there's an IMultiFn protocol we could implement https://github.com/clojure/clojurescript/blob/4a73bc8b4c95cfedc614dcabb0fe1795da371d37/src/main/cljs/cljs/core.cljs#L11170-L11179

For JVM Clojure there's no protocol, so we'd have to subclass clojure.lang.MultiFn. I think we'd have to use gen-class, but then it wouldn't work with ClojureScript. So I think we'll need to have separate implementations of StandardMultiFn for Clojure and ClojureScript. Maybe proxy will work, but IIRC that does runtime lookup of methods which would hurt performance

@camsaul
Copy link
Owner Author

camsaul commented Jun 9, 2021

Don't think gen-class is going to work since we have to AOT it, and that requires all of the protocols it implements to be AOT'ed too... generating our own bytecode with something like insn (wraps ASM) might be our best bet.

@camsaul
Copy link
Owner Author

camsaul commented Jun 9, 2021

proxy-plus might actually be perfect for this

@camsaul camsaul added this to the 1.0.0 milestone Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high-priority! more important than the other issues
Projects
None yet
Development

No branches or pull requests

2 participants