-
Notifications
You must be signed in to change notification settings - Fork 79
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 check for extracting var from definition #484
Conversation
forms arising from macroexpansions containing side effects e.g. (do (def foo) do-that) might unintentionally end up wrapped with a ::clerk/var-from-def where the intended return value is discarded
src/nextjournal/clerk/analyzer.clj
Outdated
(and (seq? form) (symbol? (first form)) (str/starts-with? (name (first form)) "def"))) | ||
(and (seq? form) | ||
(symbol? (first form)) | ||
(contains? #{'def 'defonce 'defn} (first form)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intent here was to actually account for user-defined macros that are like def
s, so more than just those three concrete instances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could check the returned value is a var of the same name as the one inferred by the analyzer, to decide when to do the wrapping.
Fixes #499. At present, a user defined macro which starts with
def
and expands to code likewhen called produces a cell result wrapped in a
{::clerk/var-from-def #'stuff}
while the intended return value is actually discarded.