rust-analyzer icon indicating copy to clipboard operation
rust-analyzer copied to clipboard

`xflags!` macro fails to expand

Open Veykril opened this issue 1 year ago • 6 comments

https://github.com/rust-lang/rust-analyzer/blob/9b3052104cc997c2d2d28eb0f01ca6684666743c/xtask/src/flags.rs#L7: "compile_error! argument must be a string" since https://github.com/rust-lang/rust-analyzer/pull/16158

Veykril avatar Jan 04 '24 16:01 Veykril

Turns out this desugaring only happens for declarative macros?

Veykril avatar Jan 04 '24 16:01 Veykril

Turns out desugaring only happens for declarative macros?

Procedural macros do not quote it as raw strings, not sure why this difference exists, though. It begins here: https://github.com/rust-lang/rust/blob/8d39ec1825024f3014e1f847942ac5bbfcf055b0/compiler/rustc_expand/src/mbe/macro_rules.rs#L298

Also, xflags doesn't support raw literals: https://github.com/matklad/xflags/blob/master/crates/xflags-macros/src/parse.rs#L391

I didn't do my due diligence when asserting that the function was only used for declarative macros. I'm a bit lost on where to start this fix. Any ideas on how to propagate whether the caller is expanding a declarative or procedural macro?

For reference, my reeeeeeally lazy test: image

saiintbrisson avatar Jan 04 '24 19:01 saiintbrisson

We'll have to add a parameter to syntax_node_to_token_tree (in the syntax_bridge module) and related functions that togglers that behavior, then adjust all the callers appropriately I'd say

Veykril avatar Jan 04 '24 20:01 Veykril

@rustbot claim

saiintbrisson avatar Jan 09 '24 12:01 saiintbrisson

I haven't forgotten about this issue. Just Really Busy Lately :tm:. This is pretty much what was said in https://github.com/rust-lang/rust-analyzer/issues/16259#issuecomment-1877696549, but you'd be surprised at the amount of places this impacts. If someone is up to it, feel free to take it, or I'll eventually come back.

saiintbrisson avatar Feb 21 '24 13:02 saiintbrisson

No worries, you are under no obligation to fix it :) And it doesn't seem too problematic given no issues aside from this one have been raised so far.

Veykril avatar Feb 22 '24 09:02 Veykril

Turns out this desugaring only happens for declarative macros?

It seems so. (surprising to me)

I'm not familiar with the internals of the compiler, but after some digging into the code, my understanding is this:

it's NOT the tt parser, but the macro engine, which desugars the doc comments into doc attributes. the ast actually keeps the original DocComment token all the way, it even made a special case for doc comment attributes:

https://github.com/rust-lang/rust/blob/9ac33d9c33741fc24a2ff4a177e72f31b9dc775f/compiler/rustc_ast/src/ast.rs#L2776-2784

for mbe, the desugar happens here:

https://github.com/rust-lang/rust/blob/8d39ec1825024f3014e1f847942ac5bbfcf055b0/compiler/rustc_expand/src/mbe/macro_rules.rs#L1397

for proc-macro, I believe it's this piece of code:

https://github.com/rust-lang/rust/blob/9ac33d9c33741fc24a2ff4a177e72f31b9dc775f/compiler/rustc_expand/src/proc_macro_server.rs#L238-L260

note it's part of the proc_macro_server, and it create a new string by escaping the original doc comment character by chracter, and re-interns it, and explicitly creates a Str (not StrRaw) out of it.

it seems there are many different types of Tokens, TokenTrees, etc (and maybe TokenStreams, too). for example, there are compiler/rustc_ast::token_stream::TokenTree, compiler/rustc_expand::mbe::TokenTree, library/proc_macro::TokenTree, etc.

I suppose ra would have similar design: some internal types as representation of the parsed code, and some external types used by the macros. should ra also desugar the doc comments lazily, to match the compiler's behavior?

again, I'm not an expert by any means, I just want to share my findings and thoughts, in case it was useful.

nerditation avatar Apr 27 '24 08:04 nerditation