expr_to_syntaxtree: don't fail when incoming expr is invalid

[mlechu]

Currently, we have a pile of assertions in expr_to_syntaxtree checking the
validity of the given Expr. This means JuliaLowering can't handle old macros
calling old or new macros with unrecognized Expr forms---in the "old calling
old" case, this is breakage, and in the "old calling new" case it's still a
downgrade to the macro system. expr_to_syntaxtree handles macro input and
quoted expressions, so it can't assume anything about the AST structure.

This PR attempts to define expr_to_syntaxtree on all inputs in a sane way.

1. If e is one of the known forms with a different representation in
   SyntaxTree, convert it.
2. Otherwise, if e.head corresponds to a known JuliaSyntax.Kind, insert that
   and recurse on its children.
3. Otherwise, insert an K"expr_syntax" node with e as its .value. We'll
   still get an error as we did before if we try and evaluate this, but a future
   expansion is expected to clean it up.

The assertions have been removed, but their contents are generally kept for
deciding between cases (1) and (2) above.

[c42f]

Fixing the problem of unknown heads seems good and necessary, thanks!

I do find it surprising that this gives us mixtures of SyntaxTree and Expr in the intermediate forms and I'm a little concerned this mixed representation will lead to bugs.

How about we slightly change the representation here such that unknown heads still turn into K"expr_syntax" (or maybe K"unknown_head" or some better name?) but we map the children even when the head is unrecognized. The original head can go into an attribute. Actually this is almost what K"extension" currently is - so there's already a half-formed plan to do this kind of thing.

In addition I think we should put the error message for K"expr_syntax" in desugaring so macro expansion of unknown heads isn't an error.

[mlechu]

I do find it surprising that this gives us mixtures of SyntaxTree and Expr in the intermediate forms and I'm a little concerned this mixed representation will lead to bugs.

Any potential bugs in mind? I'm actually more worried about case 2 (mapping children) than case 3 (give up and don't convert), since case 2 can have incorrect round-tripping behaviour if an invalid Expr is a valid SyntaxTree with different semantics. Unfortunately the only correct solutions are:

• (A) Only convert when we know the semantics, otherwise produce something that will round-trip. This essentially requires validation.jl for Expr (or making compat.jl that complete), and only converting to SyntaxTree if we have a known form; otherwise using K"unknown_expr".
• (B) Only converting to SyntaxTree once macro expansion has finished, so old macros may not expand to new macrocalls.

I decided to inch towards (A), where case 2 is incorrect but not problematic in practice. I left unknown "must round-trip" stuff as Expr since it's probably going to be used by an Expr-macro immediately after, and there isn't any provenance to lose. It would eat some CPU cycles, but wouldn't be incorrect to recursively convert the Expr one-to-one under K"unknown_expr", but I don't know if that's any more useful than the Expr.

[c42f]

Any potential bugs in mind

Not specifically right now. But having a mixed representation seems more confusing to reason about and feels like fertile ground for such bugs.

case 2 can have incorrect round-tripping behaviour if an invalid Expr is a valid SyntaxTree with different semantics

hmm ok. Do you have an example of this? Maybe we already have this problem in the existing eager conversion code?

(B) ... old macros may not expand to new macrocalls

Hmm. I don't love this tradeoff. Though old macros can't even expand to old macrocalls in the existing system and have things generally work so I guess there's precedent in some sense 😅

[mlechu]

Do you have an example of this?

Anywhere SyntaxTree uses a representation that is an invalid Expr (Expr(:var, :x), Expr(:try, Expr(:block, :foo), Expr(:catch, :e, Expr(:block)))), the expression will not round-trip. As I mentioned, we could recursively convert one-to-one if you prefer that (not using insert_convert_expr, a much simpler procedure). This would make forms like (unknown_expr (var x)), but I'm not sure there's anything useful to do with the wrapped form other than converting it back to Expr, and it's just another edge case for "tree-scanning" static analysis tools to skip over

[c42f]

Anywhere SyntaxTree uses a representation that is an invalid Expr(:var, :x) ...

Is the following what you're concerned about:

macro blah()
    v = Expr(:var, :x)
    quote
        @process_var $v
        @process_var x
    end
end

macro process_var(ex)
    if ex isa Expr && ex.head == :var
        "var node"
    else
        "something else"
    end
end

julia> @macroexpand @blah
quote
    #= REPL[11]:4 =#
    "var node"
    #= REPL[11]:5 =#
    "something else"
end

This will indeed produce different results in new macro expansion if we blindly convert between Expr and SyntaxTree without validating both directions of the conversion.

The same thing can happen in reverse if a new macro produces an invalid SyntaxTree which just happens to convert to a valid Expr. This is harder because there's only a limited number of Kinds but I suspect it's possible. It seems less problematic in practice because nobody is writing new-style macros yet.

[mlechu]

Yup, that is the concern