drym-org / qi

An embeddable flow-oriented language.
59 stars 12 forks source link

fix foreign-macro-template-expand hygiene #23

Closed michaelballantyne closed 2 years ago

michaelballantyne commented 2 years ago

Summary of Changes

Fix hygiene in foreign-macro-template-expand. This resolves issue (A) here: https://github.com/countvajhula/qi/pull/16#issuecomment-1041398205

I'm out of time for today but I'm happy to provide an explanation for the fix at a later date.

Public Domain Dedication

(Why: The freely released, copyright-free work in this repository represents an investment in a better way of doing things called attribution-based economics. Attribution-based economics is based on the simple idea that we gain more by giving more, not by holding on to things that, truly, we could only create because we, in our turn, received from others. As it turns out, an economic system based on attribution -- where those who give more are more empowered -- is significantly more efficient than capitalism while also being stable and fair (unlike capitalism, on both counts), giving it transformative power to elevate the human condition and address the problems that face us today along with a host of others that have been intractable since the beginning. You can help make this a reality by releasing your work in the same way -- freely into the public domain in the simple hope of providing value. Learn more about attribution-based economics at drym.org, tell your friends, do your part.)

countvajhula commented 2 years ago

This is amazing! Thank you for fixing the hygiene issue šŸ˜ƒ šŸš€ šŸ†

michaelballantyne commented 2 years ago

Just as a quick explanation of what was going wrong here:

Starting from the example:

((flow (~> (ā–³ +) ā–½)) (list 1 2 3) 10)

ā–³ generates the lambda binding vs:

[(_ ((~or (~datum ā–³) (~datum sep)) onex:clause))
   #'(Ī» (v . vs)
       ((flow (~> ā–³ (>< (apply (flow onex) _ vs)))) v))]

Then >< generates the call to flow with the template:

[(_ ((~or (~datum ><) (~datum amp)) onex:clause))
   #'(curry map-values (flow onex))]

So we end up with:

(curry map-values (flow (apply (flow +) _ vs)))

and we get to the template case. Note that ā–³ generated the lambda binders but another call to flow for >< generated the remaining call to flow.

In foreign-macro-template-expand, syntax->datum walks the given syntax deeply and throws away all the syntax wrappers with hygiene information. This means that the original lexical context of the vs reference is lost! Once the generated code is constructed, datum->syntax attaches the lexical context from stx. Here that's the lexical context of the remaining call to flow in the code above. So the expander ends up trying to resolve the vs reference as though it were inserted by the expansion of flow that handled ><, without the macro-introduction or lambda scopes from the flow expansion for ā–³.

In general syntax->datum is only useful when you want to treat an s-expression purely as data and not as code, so you want to throw away the syntax part. To pull apart syntax without throwing away lexical context, use syntax-parse to pattern match, syntax-e to remove just the top syntax wrapper, or syntax->list to remove just the wrappers in the top-level syntax-list.

You'll also see that my modification constructs references like lambda as syntax quotations so that they are resolved in the lexical context of the generating code rather than in some other lexical context from a later datum->syntax. Unless you're intentionally breaking hygiene, usually you'd only use datum->syntax to copy over the lexical context from an outer pair of parentheses if you've rewritten what's inside them. (The lexical context on the parens matters for resolving #%app). Or for bringing along source locations or syntax properties.

countvajhula commented 2 years ago

That all makes a lot of sense. The resolution of #%app also sounds like a good thing to keep in mind given that it's using fancy-app within Qi, might save me save grief in debugging things down the line. Thanks for taking the time to put it all down while you still had context on it! šŸ„‡