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

merge def var meta-data from the expression and the var-name #802

Closed
wants to merge 3 commits into from

Conversation

retrogradeorbit
Copy link
Member

@retrogradeorbit retrogradeorbit commented Sep 22, 2022

Please answer the following questions and leave the below in as part of your PR.

Fixes issue #801

@retrogradeorbit retrogradeorbit changed the title extract def var meta-data from the expression rather than the var-name merge def var meta-data from the expression and the var-name Sep 22, 2022
@borkdude
Copy link
Collaborator

This is still not the proper solution for defs that are defined by macros. Compare this in Clojure and bb:

(defmacro foodef [sym & body]
  `(do (def ~sym ~@body)))

(foodef x 1)

(prn (meta #'x))

To properly test this, I think it would be good to include the macro examples.

@borkdude
Copy link
Collaborator

Completed the PR here:

2b8751f

Couldn't hurt to receive a post-hoc review.

@borkdude borkdude closed this Sep 22, 2022
@retrogradeorbit
Copy link
Member Author

what does the with-top-level-loc do? can you debrief me on that?

@retrogradeorbit
Copy link
Member Author

Compare this in Clojure and bb

I see. missing again...

*top-level-location* is storing the extra metadata of the... outermost level of the macro?

What is top-level?

@borkdude
Copy link
Collaborator

@retrogradeorbit Yes! What SCI does now is that it remembers the top level location of a form that is analyzed. Also, when expanding a macro, the expanded form gets the locations of the unexpanded forms. These two combined:

(defmacro foodef [x v] `(def ~x ~ v))

(foodef x 1) ;; expression with metadata line 3
;;=> '(def x 1) ;; metadata is merged, so line 3 is preserved 

This above is the case you covered.
But when doing something like this:

(defmacro foodef [x v] `[(def ~x ~ v)])

e.g. wrap the expansion in a vector, the def form won't have any metadata, so we remember the top level metadata of the vector and use that in def.

For top-level do expressions, clojure has special semantics, so we do a similar thing there: we remember the top level location of the do expression while analyzing every do child.

@borkdude
Copy link
Collaborator

Another (maybe better) example is this one:

(defmacro foodef [sym & body]
  `(when (odd? 1)
     (def ~sym ~@body)))

(foodef x 1)

The def expression in the macro has no metadata (after expansion), so we piggyback on the metadata the top level expression.

@retrogradeorbit
Copy link
Member Author

Great information. Thankyou!

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.

2 participants