Clojure

Tag gensym sourced symbols with metadata

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Trivial Trivial
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

For static analysis tools derived from TANAL it is frequently useful to determine whether a symbol is user defined or the result of code generation. As tools analyzer depends on the Clojure core for evaluation and symbol generation a user wishing to annotate generated symbols must currently provide a binding replacing clojure.core/gensym with a snippet equivalent to the following patch. Such overloading is not appropriate for TANAL, TE* or user code as it is a redefinition of clojure.core behavior which should be standard rather than subjected to users with crowbars.

Activity

Hide
Gary Trakhman added a comment - - edited

This could eventually help with filtering out def'd symbols like 't131045 coming from reify in CLJS. I've been seeing this behavior with core.async namespaces in an autodoc-cljs proof-of-concept, which could eventually target tools.analyzer.

Show
Gary Trakhman added a comment - - edited This could eventually help with filtering out def'd symbols like 't131045 coming from reify in CLJS. I've been seeing this behavior with core.async namespaces in an autodoc-cljs proof-of-concept, which could eventually target tools.analyzer.
Hide
Alex Miller added a comment -

Re the patch, why not call the Symbol constructor that takes meta instead of with-meta? For performance, it might also be useful to use the same constant map as well.

Show
Alex Miller added a comment - Re the patch, why not call the Symbol constructor that takes meta instead of with-meta? For performance, it might also be useful to use the same constant map as well.
Hide
Reid McKenzie added a comment -

Because the compiler will emit the meta map as a static field the patch as-is will share the same map instance between all annotated symbols. Calling the metadata constructor is reasonable, I'll update the patch.

Show
Reid McKenzie added a comment - Because the compiler will emit the meta map as a static field the patch as-is will share the same map instance between all annotated symbols. Calling the metadata constructor is reasonable, I'll update the patch.
Hide
Reid McKenzie added a comment -

So the metadata constructor of Symbol is private, see https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Symbol.java#L100. Without changing this directly constructing symbols with metadata is not possible from the core. If you're worried about escaping the var indirection cost of adding metadata via with-meta inlining with-meta is an option, however then we're building two symbols for no good reason. Exposing the currently private metadata constructor is probably the right fix, abet its own ticket.

Show
Reid McKenzie added a comment - So the metadata constructor of Symbol is private, see https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Symbol.java#L100. Without changing this directly constructing symbols with metadata is not possible from the core. If you're worried about escaping the var indirection cost of adding metadata via with-meta inlining with-meta is an option, however then we're building two symbols for no good reason. Exposing the currently private metadata constructor is probably the right fix, abet its own ticket.
Hide
Andy Fingerhut added a comment -

From the comments above it appears that this is not planned to be a final version of this patch, but FYI some automated scripts I have found that patch 0001-Annotate-generated-symbols-with-metadata.patch dated Jun 9 2014 applies cleanly to the latest Clojure master as of Jul 1 2014, but Clojure fails to build.

Show
Andy Fingerhut added a comment - From the comments above it appears that this is not planned to be a final version of this patch, but FYI some automated scripts I have found that patch 0001-Annotate-generated-symbols-with-metadata.patch dated Jun 9 2014 applies cleanly to the latest Clojure master as of Jul 1 2014, but Clojure fails to build.
Hide
Reid McKenzie added a comment -

Thanks Andy, I'll rework and test it in the morning

Show
Reid McKenzie added a comment - Thanks Andy, I'll rework and test it in the morning
Hide
Reid McKenzie added a comment -

Because of the work that clojure.lang.Symbol/intern does, exposing and using the metadata constructor directly makes no sense. The updated patch directly invokes clojure.lang.Symbol/withMeta rather than indirecting through clojure.core/with-meta and taking the performance hit of calling through a Var. Builds cleanly on my system.

Show
Reid McKenzie added a comment - Because of the work that clojure.lang.Symbol/intern does, exposing and using the metadata constructor directly makes no sense. The updated patch directly invokes clojure.lang.Symbol/withMeta rather than indirecting through clojure.core/with-meta and taking the performance hit of calling through a Var. Builds cleanly on my system.
Hide
Andy Fingerhut added a comment -

Reid, although JIRA can handle multiple attachments with the same name, it can be a bit confusing for people, and for some scripts I have for determining which patches apply and test cleanly. Would you mind renaming one of your patches?

Show
Andy Fingerhut added a comment - Reid, although JIRA can handle multiple attachments with the same name, it can be a bit confusing for people, and for some scripts I have for determining which patches apply and test cleanly. Would you mind renaming one of your patches?
Hide
Reid McKenzie added a comment -

3rd and final cut at this patch.

Show
Reid McKenzie added a comment - 3rd and final cut at this patch.

People

Vote (2)
Watch (3)

Dates

  • Created:
    Updated: