-
Notifications
You must be signed in to change notification settings - Fork 32
Huge macro refactor: Unify built-in and user-defined macro formats #34
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change allows macros to return arbitrary [estree][1] objects as an alternative to returning arrays and objects as usual. This means you can now write macros that support estree features that eslisp doesn't yet (e.g. ES6 arrow functions), as long as [escodegen][2] supports them. You can even use crazy experimental estree extensions (e.g. Facebook's [JSX][3], or #11; ES7 async/await), as long as your macro can do the appropriate transformation to the core eslisp standard that escodegen understands before returning. [1]: https://github.com/estree/estree [2]: https://github.com/estools/escodegen [3]: https://github.com/facebook/jsx
This is a major rework of the macro API to clean up abstractions. I tried to avoid making a single large commit, but because the macro API touches almost everything, this was hard to chunk. Summary of changes ------------------ These changes eliminate the technical distinction between user-defined and built-in macros. Now all macros take JS objects as arguments and output either those same JS objects or estree objects. S-expressions are now always represented in the form { type : "list", values : [ { type : "atom", value : "f" }, { type : "string", value : "hi" } ] } This unification of macro formats also means dropping the "capturing macros" vs "non-capturing macros" distinction. Macro return values can now contain calls to any macro in their outer environment (also macros defined after them), which fixes #27. User macros being able to return estree objects also enables experimental estree extensions like JSX or ES6/7 features (#11). As long as you can come up with a way of converting them to valid estree that [escodegen][1] understands, anything goes. Now that user macros can access the full compilation environment that built-in macros use, they can do low-level stuff like create new macro environments with `env.derive`, and query the macro table with `env.findMacro`. These can be very helpful when debugging. Also threw away some tests that were just showing off rather than actually testing something, and made some others more specific. Still-awkward points -------------------- These will be addressed in soon forthcoming commits. The macro environment (containing stuff like the `compile` and `evaluate` functions) is currently passed to macros as the first argument, which is super clumsy to use, but was great for integration-testing that these changes are OK otherwise, since that's how all the built-in macros received their env. The environment should be passed as `this`. The "type-value"-object format is quite annoying to write out in full, and is fragile to future changes. It might be cleaner to expose S-expr AST-node constructors on the environment object, so they could be constructed with a simple `env.atom("hi")` call and typechecked with `instanceof`. [1]: https://github.com/estools/escodegen
This mirrors the style previously used for user-defined macros, which is much conciser, but is new for built-ins. It is also less conceptualy confusing, as the function's arguments are exactly the macro's arguments.
Just to keep them doing one thing.
Just a neat shorthand for a common operation.
`make test-readme test-docs` was quite helpful, but since there were lots of conceptual changes too, this needed a read-through.
Awesome! |
Made a typo earlier and there was no unit test to catch it. There is now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are the big macro API changes I foreshadowed in chat. Comments appreciated!
2b47f4b is the central commit. Its message is a good overall summary.
If you're skimming, here's an even shorter bullet-point summary: