Skip to content
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

AST: Rearrange do to sit inside call/macrocall #322

Merged
merged 1 commit into from
Jul 8, 2023
Merged

Conversation

c42f
Copy link
Member

@c42f c42f commented Jul 1, 2023

do syntax is represented in Expr with the do outside the call. This makes some sense syntactically (do appears as "an operator" after the function call).

However semantically this nesting is awkward because the lambda represented by the do block is passed to the call. This same problem occurs for the macro form @f(x) do \n body end where the macro expander needs a special rule to expand nestings of the form Expr(:do, Expr(:macrocall ...), ...), rearranging the expression which are passed to this macro call rather than passing the expressions up the tree.

In this PR, we change the parsing of

@f(x, y) do a, b\n body\n end
f(x, y) do a, b\n body\n end

to tack the do onto the end of the call argument list:

(macrocall @f x y (do (tuple a b) body))
(call f x y (do (tuple a b) body))

This achieves the following desirable properties

  1. Content of do is nested inside the call which improves the match between AST and semantics
  2. Macro can be passed the syntax as-is rather than the macro expander rearranging syntax before passing it to the macro
  3. In the future, a macro can detect when it's being passed do syntax rather than lambda syntax
  4. do head is used uniformly for both call and macrocall
  5. We preserve the source ordering properties we need for the green tree.

Fix #319

@c42f c42f added the AST label Jul 1, 2023
@codecov
Copy link

codecov bot commented Jul 1, 2023

Codecov Report

Merging #322 (12be999) into main (296cd5e) will increase coverage by 0.10%.
The diff coverage is 100.00%.

❗ Current head 12be999 differs from pull request most recent head 1170bb6. Consider uploading reports for the commit 1170bb6 to get more accurate results

@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
+ Coverage   96.47%   96.57%   +0.10%     
==========================================
  Files          14       14              
  Lines        4136     4149      +13     
==========================================
+ Hits         3990     4007      +17     
+ Misses        146      142       -4     
Impacted Files Coverage Δ
src/expr.jl 100.00% <100.00%> (+0.31%) ⬆️
src/parser.jl 98.03% <100.00%> (+<0.01%) ⬆️

... and 2 files with indirect coverage changes

@c42f c42f added the breaking label Jul 1, 2023
@c42f c42f force-pushed the caf/do-syntax-ast branch from 12be999 to f31234f Compare July 1, 2023 21:08
@c42f
Copy link
Member Author

c42f commented Jul 8, 2023

@BenChung you might also appreciate this one (I hope :-) )

`do` syntax is represented in `Expr` with the `do` outside the call.
This makes some sense syntactically (do appears as "an operator" after
the function call).

However semantically this nesting is awkward because the lambda
represented by the do block is passed to the call. This same problem
occurs for the macro form `@f(x) do \n body end` where the macro
expander needs a special rule to expand nestings of the form
`Expr(:do, Expr(:macrocall ...), ...)`, rearranging the expression which
are passed to this macro call rather than passing the expressions up the
tree.

In this PR, we change the parsing of

    @f(x, y) do a, b\n body\n end
    f(x, y) do a, b\n body\n end

to tack the `do` onto the end of the call argument list:

    (macrocall @f x y (do (tuple a b) body))
    (call f x y (do (tuple a b) body))

This achieves the following desirable properties
1. Content of `do` is nested inside the call which improves the match
   between AST and semantics
2. Macro can be passed the syntax as-is rather than the macro expander
   rearranging syntax before passing it to the macro
3. In the future, a macro can detect when it's being passed do syntax
   rather than lambda syntax
4. `do` head is used uniformly for both call and macrocall
5. We preserve the source ordering properties we need for the green tree.
@c42f c42f force-pushed the caf/do-syntax-ast branch from f31234f to 1170bb6 Compare July 8, 2023 21:20
@c42f c42f merged commit 3fe86d3 into main Jul 8, 2023
@c42f c42f deleted the caf/do-syntax-ast branch July 8, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What to do about "do macrocalls"
1 participant