Bypass SyntaxNode in JuliaLowering; fix kw bug
#60162
Open
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.
I set out to fix a stdlib compilation failure c42f/JuliaLowering.jl#98, but ran
into a limitation in the JuliaSyntax AST. Instead of hacking in a fix, this PR
attempts to address c42f/JuliaLowering.jl#77 too.
kwThe
kwform isn't produced inSyntaxNode/SyntaxTreelike it is in Expr,which simplifies the parser, but causes problems once we start start caring
about the semantics of these trees. Parsing
"f(a=1)"will always produce anExpr(:call, :f, Expr(:kw, :a, 1)), but the equivalent SyntaxNoderepresentation
(call f (= a 1))makesExpr(:call, :f, Expr(:(=), :a, 1))unrepresentable, even though that's valid syntax a macro could produce.
To fix this, we need to convert
=tokwwithin certain forms beforemacro expansion.
One problem
There is currently no good place to put this conversion. Here's the path
from source to lowering:
*(2) and (3) are pretty trivial, and data structure names are used as
shorthand for what actually matters to this issue (the tree represented by the
data structure).
JS._to_SyntaxNode, which just deletes triviaJL._convert_nodes, which is more-or-less a one-to-one conversion, and Ithink is an artifact of JuliaSyntax and JuliaLowering being in separate
repositories. @c42f has talked about wanting to replace SyntaxNode with
SyntaxTree eventually.
easier
JS._node_to_expr, which is used for femtolisp lowering (5a) and forJuliaLowering's compatibility with existing macros (5b)
JL.expr_to_syntaxtree, which is also used for Expr-macro compatibilityExpr gets the chance to swap out
=within(call (parameters ...))in_node_to_expr, but to change SyntaxTree we can only change parsing. (Theinverse is also true: if someone wants to change parsing to
RawGreenNode,JuliaLowering will have to eat it).
This PR implements the following instead, where
_green_to_asthappens at (2)and may fix up the AST before lowering. Ideally this means more room for parser
cleanup and eventually eliminating our "pre-desugaring" mixed in with macro
expansion.
The catch is that if we want SyntaxTree to be different from RawGreenNode, we do
need to convert them to Expr differently. There are a few things we could do:
(pictured, and implemented).
This would add a step to our current femtolisp-lowered parsing (why)
SyntaxTreestructurally different from Expr (I wouldn'tmind this) and try to feed them through the same generic code (not so sure
about that).
For now I've done the lazy thing by using a boolean parameter to implement the
first option.
I also took this opportunity to put the green tree into the syntax graph. This
doesn't really change anything given how little we look at the green tree, but
wasn't difficult, and it's a neat representation.
kwnotesThis change uses
K"kw"in SyntaxTree wherever a textual=behaves more likea
=>, or if=is a parse error, wherever it would be more consistent totreat it like
=>.Note that this is slightly different from Expr. Each new place we produce
kwis either a place where
=hadkwsemantics (onlytuple) or an invalidlocation for
=, though, so everything is representable. This should fix theoverlap problem, and could allow us to permit real assignment in each
kwcellif we really wanted that.
call*x(a=1,;b=1)(kw a 1) (kw b 1)(kw a 1) (kw b 1)dotcallx.(a=1,;b=1)(kw a 1) (kw b 1)(kw a 1) (kw b 1)refx[a=1,;b=1](kw a 1) (= b 1)(kw a 1) (kw b 1)curlyx{a=1,;b=1}(= a 1) (= b 1)(kw a 1) (kw b 1)tuple(a=1,;b=1)(= a 1) (kw b 1)(kw a 1) (kw b 1)vect[a=1,;b=1](= a 1) (= b 1)(= a 1) (kw b 1)braces{a=1,;b=1}(= a 1) (= b 1)(= a 1) (kw b 1)macrocall@x(a=1,;b=1)(= a 1) (kw b 1)(= a 1) (kw b 1)