use QuoteNode and LineNumberNode consistently #23885
Merged
Conversation
This file contains 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
JeffBezanson
added
breaking
|
Also, this re-breaks #15310, since |
vtjnash
approved these changes
Sep 26, 2017
| @@ -999,7 +999,7 @@ JL_CALLABLE(jl_f_invoke_kwsorter) | |||
| jl_expr_t *jl_exprn(jl_sym_t *head, size_t n) | |||
| { | |||
| jl_ptls_t ptls = jl_get_ptls_states(); | |||
| jl_array_t *ar = n==0 ? (jl_array_t*)jl_an_empty_vec_any : jl_alloc_vec_any(n); | |||
There was a problem hiding this comment.
fix (simplify) jl_copy_ast also now
JeffBezanson
force-pushed
the
jb/fix23661
branch
from
921afad
to
017d1a5
Compare
We no longer use the `:line` Expr head in julia.
JeffBezanson
force-pushed
the
jb/fix23661
branch
from
017d1a5
to
c28ad64
Compare
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.
There is a bit of confusion about when
Exprs are used vs. specialized node types likeQuoteNodeandLineNumberNode. Typically we have used Exprs for ASTs, including all macro inputs and outputs. My original plan for this issue was to plug the "leak" ofQuoteNodes into macros by going back toExprs for all macro arguments. However, it turns out thatQuoteNodeandLineNumberNodehave already leaked into quite a few places, such that it's much simpler to go the other way and just use them more consistently.This removes a bunch of code since we now always represent line numbers with
LineNumberNodeinstead of:lineExprs.This will break code that only looks for
:quoteand:lineexpressions, but I believe a lot of uses already handle QuoteNode and LineNumberNode so it shouldn't be too bad in practice. For example, we have already been returning (perhaps by mistake) these node types fromparse.This "fixes" #23661 by making the new behavior official. The more uniform representation should be worth it.