Clojure

should reify merge rather than replace on repeated specs?

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

reify, deftype, and the like fail silently if you specify a class twice:

(macroexpand '(reify Map (size [this] 0),
Counted (count [this] 0),
Map (keySet [this] nil)))
;=> (reify* [Counted Map] (count [this] 0) (keySet [this] nil))

The later Map section entirely supersedes the former, which I discovered when I wrote a macro that injects some automated method bodies into a reify for you.

I've attached a fix to make the above expand to the expected [for me, anyway] output.

Activity

Hide
Stuart Halloway added a comment -

In Clojure, it is generally the case that redefining something replaces the original, as opposed to augmenting it by merging the old and the new. This makes it easy to reason locally about how code works. One could argue that the following snippet has the same kind of issue you describe:

(defn foo [a] 1)
(defn foo [a b] 1)
(foo 1) ; is it a bug that the first arity is gone?

I also wonder whether the current behavior might be a convenience for some macros. (Clearly it wasn't for yours!) I am changing the type and title of the ticket to better reflect the nature of the request and see what the BDFL says.

Show
Stuart Halloway added a comment - In Clojure, it is generally the case that redefining something replaces the original, as opposed to augmenting it by merging the old and the new. This makes it easy to reason locally about how code works. One could argue that the following snippet has the same kind of issue you describe:
(defn foo [a] 1)
(defn foo [a b] 1)
(foo 1) ; is it a bug that the first arity is gone?
I also wonder whether the current behavior might be a convenience for some macros. (Clearly it wasn't for yours!) I am changing the type and title of the ticket to better reflect the nature of the request and see what the BDFL says.
Hide
Rich Hickey added a comment -

Everything in this ticket needs to be said with more precision. I don't know what exactly the problem is nor what the proposed solution is.

One thing to note is that it is necessary to accept definitions under base interfaces, so the class areas are not strict, nor expected to be complete.

Show
Rich Hickey added a comment - Everything in this ticket needs to be said with more precision. I don't know what exactly the problem is nor what the proposed solution is. One thing to note is that it is necessary to accept definitions under base interfaces, so the class areas are not strict, nor expected to be complete.
Hide
Alan Malloy added a comment - - edited

Stuart: The two foo forms you give are entirely separate, and to unify the two behaviors you would have to group them together. It's not at all unreasonable to suppose the user wants to define foo once, fiddle with it, and then redefine it - clojure.core does similar stuff with let, reduce, etc.

As written, deftype/reify have a somewhat similar "look" - because there is nothing physically grouping the declaration of Map with its functions, it's not clear what should happen when a heading like Map is given twice, and it's not specified in the docs.

I think the difference is that in the reify case, the two are in the same top-level form, so the compiler can detect that you're trying to do something "weird", so a silent redefinition (reasonable for your defn example) is surprising. There are a number of solutions that would reduce this surprise:

1) Permit or require reify to group things, as in (reify (Comparable (compare [this other] 1))). Then the explicit grouping of Comparable with its methods serves two purposes: it implies that other definitions for Comparable should be included in that grouping; and it makes it easier to do that, because you can just iterate over forms until you find Comparable, and then insert another definition.

2) Throw an exception if an interface is specified twice. This is not ideal because it can be a lot of work for the user to group things together themselves, while it's easy for deftype to do given the grouping it's already doing. However, it would avoid the confusion and surprise, by saying "that's not allowed" rather than leaving the user guessing what's gone wrong.

3) Interpret my original example code as an attempt to open the Map interface, add implementations, and then later add some more implementations.

I would have liked reify to implement (1) to begin with, but at this point I don't think the syntax is backwards-compatible, so it doesn't seem like a good idea. I suppose either (2) or (3) is fine, and they both seem like an improvement over the current confusing behavior. Of course, I prefer (3), but I can understand a desire to make reify reject syntax that is not immediately obvious in intent rather than interpreting it as what I think is the most useful intent.

Show
Alan Malloy added a comment - - edited Stuart: The two foo forms you give are entirely separate, and to unify the two behaviors you would have to group them together. It's not at all unreasonable to suppose the user wants to define foo once, fiddle with it, and then redefine it - clojure.core does similar stuff with let, reduce, etc. As written, deftype/reify have a somewhat similar "look" - because there is nothing physically grouping the declaration of Map with its functions, it's not clear what should happen when a heading like Map is given twice, and it's not specified in the docs. I think the difference is that in the reify case, the two are in the same top-level form, so the compiler can detect that you're trying to do something "weird", so a silent redefinition (reasonable for your defn example) is surprising. There are a number of solutions that would reduce this surprise: 1) Permit or require reify to group things, as in (reify (Comparable (compare [this other] 1))). Then the explicit grouping of Comparable with its methods serves two purposes: it implies that other definitions for Comparable should be included in that grouping; and it makes it easier to do that, because you can just iterate over forms until you find Comparable, and then insert another definition. 2) Throw an exception if an interface is specified twice. This is not ideal because it can be a lot of work for the user to group things together themselves, while it's easy for deftype to do given the grouping it's already doing. However, it would avoid the confusion and surprise, by saying "that's not allowed" rather than leaving the user guessing what's gone wrong. 3) Interpret my original example code as an attempt to open the Map interface, add implementations, and then later add some more implementations. I would have liked reify to implement (1) to begin with, but at this point I don't think the syntax is backwards-compatible, so it doesn't seem like a good idea. I suppose either (2) or (3) is fine, and they both seem like an improvement over the current confusing behavior. Of course, I prefer (3), but I can understand a desire to make reify reject syntax that is not immediately obvious in intent rather than interpreting it as what I think is the most useful intent.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated: