feat(macros): ignore ignored macros completely #15117
Conversation
rustbot
added
the
S-waiting-on-review
|
After spending a few days writing/refactoring an |
That's not the idea behind this setting. Ignoring means we ignore the macros, that's it. Ideally this setting wouldn't exist but r-a is currently not good enough with macros so we need this escape hatch. |
| attr, | ||
| ) { | ||
| Ok(ResolvedAttr::Macro(call_id)) => { | ||
| self.attr_calls.push((ast_id, call_id)); |
There was a problem hiding this comment.
Oh ... this was the reason things broke down for tracing and the like ...
| /// Ignored proc macros should only be expanded | ||
| /// when we are explicitly asked to do so by the user. | ||
| pub ignored: bool, |
There was a problem hiding this comment.
Hmm, let's do things a bit differently. Instead of encoding ignoredness as a flag here, let's just add a second special ProcMacroExpander for ignored ones, samy way we hadnle the dummy expander basically. Then we don't need to fetch the loaded proc-macros everytime to check and can instead immediately check the expander at hand
| // disabled. This is analogous to the handling in | ||
| // `DefCollector::collect_macros`. | ||
| if exp.is_dummy() { | ||
| continue 'attrs; |
There was a problem hiding this comment.
| continue 'attrs; | |
| self.diagnostics.push( | |
| DefDiagnostic::unresolved_proc_macro( | |
| self.module_id.local_id, | |
| loc.kind, | |
| loc.def.krate, | |
| ), | |
| ); | |
| continue 'attrs; |
Veykril
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #15118) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Friendly bump, I presume you just haven't had the time for this but I wanted to ask for a status update nevertheless :) |
|
Sorry, I made no progress on this since. I assume there is not a lot of work to do here just requires a context switch on my part. I'll try to get back to it next week. If anyone wants to take over in the meantime, that's also fine by me. |
tamasfe
force-pushed
the
feat/better-ignored-macros
branch
from
ed2e2a5 to
6699133
Compare
feat: ignored and disabled macro expansion Supersedes #15117, I was having some conflicts after a rebase and since I didn't remember much of it I started clean instead. The end result is pretty much the same as the linked PR, but instead of proc macro lookups, I marked the expanders that explicitly cannot be expanded and we shouldn't even attempt to do so. ## Unresolved questions - [ ] I introduced a `DISABLED_ID` next to `DUMMY_ID` in `hir-expand`'s `ProcMacroExpander`, that is effectively exactly the same thing with slightly different semantics, dummy macros are not (yet) expanded probably due to errors, while not expanding disabled macros is part of the usual flow. I'm not sure if it's the right way to handle this, I also thought of just adding a flag instead of replacing the macro ID, so that the disabled macro can still be expanded for any reason if needed.
4 participants
This PR aims to alter how we handle ignored proc macros so the syntax tree is not altered by them in any way. This way lsp features should keep working inside macros that seemingly do nothing (e.g.
async_trait,tracing::instrument,tokio::main).Note that this approach is still not perfect, as we miss out on any changes by macros, e.g. by ignoring
pin_projectwe get IDE assists working on the struct but RA will not know anything about the methods it added. To handle this properly I'd imagine we could still expand the macro for completions but still ignore expanded code at the invocation site for anything else, doing this is way out of scope for me however.Changes
ignoredwhen we collect proc macros-and_(as not remembering which one a crate used is a source of frustrations for me)TODO
rust-analyzer.procMacro.ignoredconfigWe could alternatively keep macro resolution as it was before but throw away expansions at a later point, which would probably be better but it's deeper in the rabbit hole.