-
Notifications
You must be signed in to change notification settings - Fork 9
Compat behaviours: typeof quoted expressions, macro def signatures #44
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
Conversation
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.
Nice, we definitely need this!
I think a few things here should be implemented a bit differently in the longer term (AST interpolation, eval parameters (eventually hooks for eval (??))) though we could go with this more or less as-is and circle back.
There's obviously the longer term question of how expr_compat_mode should be set. My feeling is that it should eventually be a per-module configuration option that eval() reads. And that's set by some system for syntax evolution which probably relates to a package's Project.toml.
| end | ||
|
|
||
| function interpolate_ast(::Type{Expr}, ex, values...) | ||
| Expr(interpolate_ast(SyntaxTree, ex, values...)) |
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.
Oh, this is a rather interesting approach. Preserving SyntaxTree in the IR as the definitive as-parsed representation is cute.
With this approach I'm concerned the logic in _interpolated_value doesn't make sense.
- When interpolating values into
SyntaxTree, we currently assumeSyntaxTreeis AST, and everything else isK"Value"(ie, a leaf) - When interpolating values into
Expr, the existing system assumesExprandLineNumberNodeare pieces of AST, and everything else is a leaf (QuoteNodeis a bit of an oddity ... is it both?)
There seems to be two ways out:
- We could take a union of all these rules and support bidirectional interpolation of Expr-into-SyntaxTree and vice versa.
- We can convert
exto anExprat lowering time then implementinterpolate_astforExprseparately.
I feel like (2) would be a good conservative option for now. But at least we should make a note about these issues.
| Expr(interpolate_ast(SyntaxTree, ex, values...)) | |
| # TODO: Adjust `_interpolated_value` to ensure that incoming `Expr` data structures are treated | |
| # as AST in Expr compat mode, rather than `K"Value"`? | |
| # Or convert `ex` to `Expr` early during lowering and implement `interpolate_ast` for `Expr`? | |
| Expr(interpolate_ast(SyntaxTree, ex, values...)) |
Co-authored-by: Claire Foster <[email protected]>
The last two major pieces needed for compatibility with existing code (that I
know of 🙂):
This is done by adding a return type parameter to
interpolate_ast, which issupplied at macro-expansion time (and in the case of
@SyntaxTree,hard-coded to
SyntaxTree).Unlike with calling old-style macros (#33), we do need a short-term way to turn
compat behaviour off. I've tied it to a a parameter to lowering that lives in
context until the end of desugaring; other suggestions are welcome.
TODO: Run most tests in both modes