ClojureScript

Implement specify, allowing instances to implement protocols

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

Javascript objects are fully dynamic. Currently, ClojureScript has no mechanism to exploit that for protocol implementation.

CLJS-398 was a first attempt to implement an operation to extend single instances, but it fell short because it offered no control over which closures to attach onto the object (i.e. allocates every time). Also, specify is a much better name than extend-instance.

extend-type can be implemented in terms of specify (by specifying the .prototype), but extend-type has grown from its humble beginnings
https://github.com/clojure/clojurescript/blob/b75730da8abc3abc6134d8dd9ec426ab792d3662/src/clj/cljs/core.clj#L42
to an monster of complexity
https://github.com/clojure/clojurescript/blob/72e55315c6973caa74af39b66052424f73872033/src/clj/cljs/core.clj#L438
Currently it does

  • print warnings
  • optimize IFn impls
  • special case the Object "protocol"
  • special case js builtins

and all that in a single macro body. So this is also a good opportunity to do some refactoring.

specify should have an interface similar to extend-type. Additionally a lower level operation is needed to attach existing lambdas as protocol methods. It's called specify* in my current implementation. It takes a lambda for every specified protocol-method-arity, with a syntax loosely based on clojure.core/extend.

Activity

Hide
Herwig Hochleitner added a comment -

First patch implements specify, second switches extend-type to use it.

Show
Herwig Hochleitner added a comment - First patch implements specify, second switches extend-type to use it.
Herwig Hochleitner made changes -
Field Original Value New Value
Attachment 0002-CLJS-414-Implement-extend-type-in-terms-of-specify.patch [ 11646 ]
Attachment 0001-CLJS-414-specify-and-specify-macros.patch [ 11645 ]
Hide
David Nolen added a comment -

This is a big patch so it's going to take time to review. First question, if we're going to follow the footsteps of extend what's the purpose of changing the syntax like having to specify a map of arities?

Show
David Nolen added a comment - This is a big patch so it's going to take time to review. First question, if we're going to follow the footsteps of extend what's the purpose of changing the syntax like having to specify a map of arities?
Hide
Herwig Hochleitner added a comment - - edited

The reason is that I wanted specify* to do as little as possible. It's basically just an abstraction over the name mangling for protocol methods, which was completely hidden till now. One of the purported use cases is:

(def methA-1 [this] ...)
(def methA-2 [this x] ...)
(def methB-1 [this] ...)

(defn specify-to-P [o]
  (specify* o
    P {methA {1 methA-1
              2 methA-2}
       methB {1 methB-1}}))

Here the method bodies are not allocated every time specify-to-P is called, as opposed to what the more high-level specify would do. If we didn't know which arities are to be generated, there would be simply no way to infer it:

  • there is no simple way to know at compile time which arities an fn has
  • if there was we wouldn't be able to get the fn at compile time in all cases
  • defprotocol doesn't expose it's methods or their arities

EDIT:

As for using symbols instead of keywords: I could do this, since specify* is a macro, not a function.
Making specify* a function may have its own merits, which I frankly hadn't thought about. Actually, that might be sweet, I'll look into that possibility.

Show
Herwig Hochleitner added a comment - - edited The reason is that I wanted specify* to do as little as possible. It's basically just an abstraction over the name mangling for protocol methods, which was completely hidden till now. One of the purported use cases is:
(def methA-1 [this] ...)
(def methA-2 [this x] ...)
(def methB-1 [this] ...)

(defn specify-to-P [o]
  (specify* o
    P {methA {1 methA-1
              2 methA-2}
       methB {1 methB-1}}))
Here the method bodies are not allocated every time specify-to-P is called, as opposed to what the more high-level specify would do. If we didn't know which arities are to be generated, there would be simply no way to infer it:
  • there is no simple way to know at compile time which arities an fn has
  • if there was we wouldn't be able to get the fn at compile time in all cases
  • defprotocol doesn't expose it's methods or their arities
EDIT: As for using symbols instead of keywords: I could do this, since specify* is a macro, not a function. Making specify* a function may have its own merits, which I frankly hadn't thought about. Actually, that might be sweet, I'll look into that possibility.
Hide
Herwig Hochleitner added a comment - - edited

As for specify* being a function: It uses a map in its syntax and at the same time is used to define PersistentHashMap, so that wouldn't work.
EDIT: the real reason is name mangling, see comment below

What could conceivably be done is to have a core function (specify-method [proto method arity impl]).
EDIT: can't be done without reflection, same reason

That way we could support consumers that want to determine the set of implemented protocols at runtime, similar to extend. This can be a separate ticket where we work out whether it's :method 'method or "method".

As for specify* using symbols: My gut feeling tells me that since it will always be a compiler macro (or builtin) and takes the arities, there is no merit in making it superficially more similar to extend.

OTOH, it occurs to me that given specify-method, we could leave out specify* completely. I think I'll try that when I get home from work. Thoughts on that approach?

Show
Herwig Hochleitner added a comment - - edited As for specify* being a function: It uses a map in its syntax and at the same time is used to define PersistentHashMap, so that wouldn't work. EDIT: the real reason is name mangling, see comment below What could conceivably be done is to have a core function (specify-method [proto method arity impl]). EDIT: can't be done without reflection, same reason That way we could support consumers that want to determine the set of implemented protocols at runtime, similar to extend. This can be a separate ticket where we work out whether it's :method 'method or "method". As for specify* using symbols: My gut feeling tells me that since it will always be a compiler macro (or builtin) and takes the arities, there is no merit in making it superficially more similar to extend. OTOH, it occurs to me that given specify-method, we could leave out specify* completely. I think I'll try that when I get home from work. Thoughts on that approach?
Hide
Herwig Hochleitner added a comment -

Actually, I had a rationale for using symbols in specify*, which I just now rediscovered while trying to implement aforementioned specify-method.
It uses symbols to highlight the fact, that the method names are subject to the same (gclosure) minification as every other name.

This is also the real reason specify* (or specify-method) can't be a function: It would need a reflective layer to go from [proto method arity] to the gclosure minified version of $proto$method$arity$a. We don't have that in clojurescript, since it would considerably add to the output size.

Considering that all, I think specify* in the current form is appropriate after all.

Nevertheless I found an unrelated issue in the patch where 'Object would be resolved (with an unused result), triggering a warning. I added the 0001_1 patch, which supersedes 0001 and is functionally equivalent modulo the warning issue. 0002 still applies.

Show
Herwig Hochleitner added a comment - Actually, I had a rationale for using symbols in specify*, which I just now rediscovered while trying to implement aforementioned specify-method. It uses symbols to highlight the fact, that the method names are subject to the same (gclosure) minification as every other name. This is also the real reason specify* (or specify-method) can't be a function: It would need a reflective layer to go from [proto method arity] to the gclosure minified version of $proto$method$arity$a. We don't have that in clojurescript, since it would considerably add to the output size. Considering that all, I think specify* in the current form is appropriate after all. Nevertheless I found an unrelated issue in the patch where 'Object would be resolved (with an unused result), triggering a warning. I added the 0001_1 patch, which supersedes 0001 and is functionally equivalent modulo the warning issue. 0002 still applies.
Herwig Hochleitner made changes -
Hide
Herwig Hochleitner added a comment -

Attached patch set 01** supersedes patch set 00**

The differences are

  • Make ^:skip-protocol-flag work
  • Added tests
Show
Herwig Hochleitner added a comment - Attached patch set 01** supersedes patch set 00** The differences are
  • Make ^:skip-protocol-flag work
  • Added tests
Hide
Herwig Hochleitner added a comment - - edited
Show
Herwig Hochleitner added a comment - - edited Set up a design page http://dev.clojure.org/display/design/specify+i.e.+reify+for+instances
Hide
Herwig Hochleitner added a comment -

Attached patch 0104, applies on top of other 01* patches.
It adds capability to specify* IFn and allows specify* to take an expression too.

Show
Herwig Hochleitner added a comment - Attached patch 0104, applies on top of other 01* patches. It adds capability to specify* IFn and allows specify* to take an expression too.
Herwig Hochleitner made changes -
Hide
Herwig Hochleitner added a comment -
Show
Herwig Hochleitner added a comment - rebased to current master see also https://groups.google.com/d/topic/clojure-dev/1UegnkLSwhE/discussion
Hide
Gary Trakhman added a comment -

I could see this being immediately useful to me, it would enable some experimentation with dynamic extension of clojure collection functions over JS types. Without it, it's hard to make those impls composable and exportable in a convenient way.

Show
Gary Trakhman added a comment - I could see this being immediately useful to me, it would enable some experimentation with dynamic extension of clojure collection functions over JS types. Without it, it's hard to make those impls composable and exportable in a convenient way.
Hide
David Nolen added a comment -

This ticket needs serious cleanup. All the old patches should be removed and a new patch rebased to master submitted that follows all the conclusions arrived at on the design page.

Show
David Nolen added a comment - This ticket needs serious cleanup. All the old patches should be removed and a new patch rebased to master submitted that follows all the conclusions arrived at on the design page.
Hide
Herwig Hochleitner added a comment -

I have published the branch from which the patches were generated on https://github.com/bendlas/clojurescript/tree/specify
It mostly implements the behavior specified on the design page, but needs rebasing to master and fixing one case relating to IFn.invoke vis a vis .call
I'd like to get back to it and plan to do so, when I can take a few days for working on clojurescript itself.

Show
Herwig Hochleitner added a comment - I have published the branch from which the patches were generated on https://github.com/bendlas/clojurescript/tree/specify It mostly implements the behavior specified on the design page, but needs rebasing to master and fixing one case relating to IFn.invoke vis a vis .call I'd like to get back to it and plan to do so, when I can take a few days for working on clojurescript itself.
Hide
David Nolen added a comment -

fixed, https://github.com/clojure/clojurescript/commit/571e156d2daa223dcef273106827e932283e2f93

further enhancement should be addressed by smaller tickets

Show
David Nolen added a comment - fixed, https://github.com/clojure/clojurescript/commit/571e156d2daa223dcef273106827e932283e2f93 further enhancement should be addressed by smaller tickets
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: