Clojure

Declare wipes out existing var metadata

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.2
  • Component/s: None
  • Labels:
    None
  • Approval:
    Ok

Description

If you call declare on a var that has already been defined, it wipes out the metadata on that var. While this is possible to avoid in most cases, it makes like difficult for automatic code generators or scanners. In particular, autodoc loses information in some cases because of this if it scans a file that has already been pulled in because of a dependency.

Here's an example. This code:

(defn hello "I like documentation!" [] (println "works"))
(hello)
(println (:doc (meta #'hello)))

(println "I do declare!")
(declare hello)

(hello)
(println (:doc (meta #'hello)))

Prints:

works
I like documentation!
I do declare!
works
nil

The last line should be "I like documentation!" again.

Activity

Hide
Assembla Importer added a comment -

tomfaulhaber said: [file:bwNFCIwMSr34TZeJe5cbCb]: Patch to preserve metadata in declare

Show
Assembla Importer added a comment - tomfaulhaber said: [file:bwNFCIwMSr34TZeJe5cbCb]: Patch to preserve metadata in declare
Hide
Assembla Importer added a comment -

richhickey said: I'm not sure about this one. Are you evaluating code in autodoc? Otherwise, I don't see the issue for scanners/parsers

Show
Assembla Importer added a comment - richhickey said: I'm not sure about this one. Are you evaluating code in autodoc? Otherwise, I don't see the issue for scanners/parsers
Hide
Assembla Importer added a comment -

tomfaulhaber said: Yeah, my immediate case is autodoc, which loads files without knowing the user desired load order (important for things like contrib or nowadays clojure itself where there is no "root" that would pull everything in).

But the other case I was thinking of was something more like yacc where code is being generated in little snippets. Rather than building all the dependency relationships between the snippets, a program may want to just add declares to the snippet for the "external" vars that it references. Then the individual generated snippets can be emitted independently without concern for order.

At a higher level, why would declare preserve the value of a var but squash the metadata? Seems like it should be all or nothing.

Show
Assembla Importer added a comment - tomfaulhaber said: Yeah, my immediate case is autodoc, which loads files without knowing the user desired load order (important for things like contrib or nowadays clojure itself where there is no "root" that would pull everything in). But the other case I was thinking of was something more like yacc where code is being generated in little snippets. Rather than building all the dependency relationships between the snippets, a program may want to just add declares to the snippet for the "external" vars that it references. Then the individual generated snippets can be emitted independently without concern for order. At a higher level, why would declare preserve the value of a var but squash the metadata? Seems like it should be all or nothing.
Hide
Assembla Importer added a comment -

richhickey said: >At a higher level, why would declare preserve the value of a var but squash the metadata? Seems like it should be all or nothing.

Quite possibly, but then I think it might become declare == unbind-root, which doesn't help you either, does it?

Show
Assembla Importer added a comment - richhickey said: >At a higher level, why would declare preserve the value of a var but squash the metadata? Seems like it should be all or nothing. Quite possibly, but then I think it might become declare == unbind-root, which doesn't help you either, does it?
Hide
Assembla Importer added a comment -

tomfaulhaber said: Two issues with that, one short-term for autodoc and one more abstract:

1) With the current implementation of defmulti (even with my other patch in ticket #309), you wouldn't be able to correctly reload a file that had (declare foo) ... (defmulti foo ...). This is already causing autodoc to mis-document on of the contrib namespaces. (Because it gets loaded twice, once with a use and once with a load).

2) It means that any generation tool that wants to generate "order independent" snippets would be required to do all its own book-keeping in order to determine whether to emit a declare or not.

To me it feels like the more correct semantics would be to have declare be a no-op wrt binding. If we need something for explicitly unbinding, (def var) would be a more obvious candidate. (Right now, I think they are equivalent.)

Show
Assembla Importer added a comment - tomfaulhaber said: Two issues with that, one short-term for autodoc and one more abstract: 1) With the current implementation of defmulti (even with my other patch in ticket #309), you wouldn't be able to correctly reload a file that had (declare foo) ... (defmulti foo ...). This is already causing autodoc to mis-document on of the contrib namespaces. (Because it gets loaded twice, once with a use and once with a load). 2) It means that any generation tool that wants to generate "order independent" snippets would be required to do all its own book-keeping in order to determine whether to emit a declare or not. To me it feels like the more correct semantics would be to have declare be a no-op wrt binding. If we need something for explicitly unbinding, (def var) would be a more obvious candidate. (Right now, I think they are equivalent.)
Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - stu said: [file:dRHbioAFCr34lYeJe5cbLA]
Hide
Assembla Importer added a comment -

stu said: Third patch takes a different approach, working at level of def instead of declare. Def forms without an init-expr set no metadata.

Questions:

(1) Tom, does this solve the problems you are seeing?

(2) Rich, can DefExpr's .meta be typed as MapExpr, instead of Expr?

(3) A further wrinkle would be to def-sans-init set metadata if no metadata is present yet. Too cute?

Show
Assembla Importer added a comment - stu said: Third patch takes a different approach, working at level of def instead of declare. Def forms without an init-expr set no metadata. Questions: (1) Tom, does this solve the problems you are seeing? (2) Rich, can DefExpr's .meta be typed as MapExpr, instead of Expr? (3) A further wrinkle would be to def-sans-init set metadata if no metadata is present yet. Too cute?
Hide
Assembla Importer added a comment -

stu said: Updating tickets (#330, #357, #358, #365)

Show
Assembla Importer added a comment - stu said: Updating tickets (#330, #357, #358, #365)

People

  • Assignee:
    Unassigned
    Reporter:
    Anonymous
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: