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

Fix exception when instrumenting code with embedded namespace #329

Merged
merged 1 commit into from
Mar 31, 2016

Conversation

grammati
Copy link
Contributor

Instrumentation would throw when trying to macroexpand code containing a
namespace object with metadata.

The logging macros in clojure.tools.logging embed the current namespace
object into the code at macroexpansion time. Instrumenting a function
that contained logging calls would throw an exception if the current
namespace had metadata (eg: a docstring).

This is because Namespace has the unusual property of implementing
IMeta (so you can call (meta *ns*)), but not IObj (so (with-meta *ns* ...) throws).

Instrumentation would throw when trying to macroexpand code containing a
namespace object with metadata.

The logging macros in clojure.tools.logging embed the current namespace
object into the code at macroexpansion time. Instrumenting a function
that contained logging calls would throw an exception if the current
namespace had metadata (eg: a docstring).

This is because Namespace has the unusual property of implementing
IMeta (so you can call `(meta *ns*)`), but not IObj (so `(with-meta *ns*
...)` throws).
@bbatsov bbatsov merged commit ab2ab9c into clojure-emacs:master Mar 31, 2016
@bbatsov
Copy link
Member

bbatsov commented Mar 31, 2016

👍 Good catch! Thanks!

You might also want to mention this in CIDER's changelog.

@Malabarba
Copy link
Member

Thanks indeed!

@grammati grammati deleted the instrument-namespace-object branch April 22, 2017 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants