Allow global_asm! in statement positions#156582
Conversation
This comment has been minimized.
This comment has been minimized.
fc5f2bf to
76f8edb
Compare
There was a problem hiding this comment.
The main issue is that GlobalAsm is an Item. In expand_invoc(), it will try to extract the node depending on AstFragmentKind. Until now we only expected it on the item level. But now it can be either an item or a statement. If its a statement, it will find an item and return an error saying that non-statement macros are not allowed in statement positions. So somehow we have to make the macro an item and statement at the same time or change whats expected.
I spent quite some time trying to figure out how this could be done. AFAIU:
- The macro is considered a
AstFragmentKind::Stmtsbecause its considered aast::Stmt. Special casing this would require changingInvocationCollectorNode. - I could try to move away from
SyntaxExtensionKind::BangLegacy:
rust/compiler/rustc_expand/src/expand.rs
Lines 729 to 769 in 7c3c88f
But that does already implement a lot of special casing for(*_)asm!. - Changing it to something else then
ast::Stmtis presumably a terrible idea. - Adapting
MacEagerwould be possible, but not through theMacResultinterface. The only information it has is what kind ofAstFragmentKindwe are dealing with. So some information has to be stored onMacEager. That turned out to be much more code. - Ideally
expand_global_asm()could simply store the rightMacEagertype, but it doesn't know whatAstFragmentKindwill be asked of it.
So just introducing a new MacResult implementation felt like the best solution.
Maybe somebody more knowledgeable about the parser could pinpoint exactly where we could special case this more easily.
There was a problem hiding this comment.
After some more thinking I believe the best alternative is adapting MacEager. E.g. adding a upgradable_to_stmt: bool, in which case make_stmts would upgrade potentially existing items to ast::Stmts.
Let me know if that is the preferred change.
76f8edb to
d0629e2
Compare
This PR makes it possible to put
global_asm!in statement positions. This is particularly useful for proc-macros, where you otherwise have to wrap them inmod foo { global_asm!(...); }.I'm happy to open an ACP first (or a design meeting?).
I would also assume this needs sign-off from the lang-team?
Previously discussed on Zulip.
r? @Amanieu