You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
whose job is to massage the syntax tree into the right form before expansion. Concretely, this means stripping attributes of the macro itself.
I don't think there's anything problematic with this code per-se, but I don't like the fact that it blurs the boundary between macro expansion world (which manipulates token trees) and normal world (where syntax trees, onec parsed, are immutable).
Specifically, I don't like that we modify the syntax tree there -- syntax tree modification is the API specific to, and tailored to implementing refactors -- I can imagine a world where we only add .clone_for_update and such at the ide_db layer.
I think a better approach would be to massage the tree at the moment we transform SyntaxNode to TokenTree. I'd imagine it could work like this:
we remove process_macro_input
we restore macro_arg_text to be just an identity function (ie, we don't do atribute stripping at this stage)
The semantics is that we just skip censored nodes when converting SyntaxTree to tt.
To clarify, I honestly don't have a super great motivation other than "my gut tells me that we are on the wrong side of abstraction boundary here". That being said, I wonder if this approach would allow us to remove the "replace attr with an equal amount of whitespace" hack?
The text was updated successfully, but these errors were encountered:
We currently have this function:
https://github.com/rust-analyzer/rust-analyzer/blob/49c02b93b39945bedede363b5f15259570f76304/crates/hir_expand/src/input.rs#L13-L33
whose job is to massage the syntax tree into the right form before expansion. Concretely, this means stripping attributes of the macro itself.
I don't think there's anything problematic with this code per-se, but I don't like the fact that it blurs the boundary between macro expansion world (which manipulates token trees) and normal world (where syntax trees, onec parsed, are immutable).
Specifically, I don't like that we modify the syntax tree there -- syntax tree modification is the API specific to, and tailored to implementing refactors -- I can imagine a world where we only add
.clone_for_update
and such at theide_db
layer.I think a better approach would be to massage the tree at the moment we transform SyntaxNode to TokenTree. I'd imagine it could work like this:
process_macro_input
macro_arg_text
to be just an identity function (ie, we don't do atribute stripping at this stage)https://github.com/rust-analyzer/rust-analyzer/blob/49c02b93b39945bedede363b5f15259570f76304/crates/hir_expand/src/db.rs#L258
we look at the kind of macro, and, if that's attr or derive, we collect a list of nodes to "censor"
syntax_node_to_token_tree
to be like this:To clarify, I honestly don't have a super great motivation other than "my gut tells me that we are on the wrong side of abstraction boundary here". That being said, I wonder if this approach would allow us to remove the "replace attr with an equal amount of whitespace" hack?
The text was updated successfully, but these errors were encountered: