Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xflags! macro fails to expand #16259

Closed
Veykril opened this issue Jan 4, 2024 · 7 comments · Fixed by #17153
Closed

xflags! macro fails to expand #16259

Veykril opened this issue Jan 4, 2024 · 7 comments · Fixed by #17153
Assignees
Labels
A-macro macro expansion A-proc-macro proc macro C-bug Category: bug E-medium

Comments

@Veykril
Copy link
Member

Veykril commented Jan 4, 2024

xflags::xflags! {
: "compile_error! argument must be a string" since #16158

@Veykril Veykril added C-bug Category: bug A-proc-macro proc macro labels Jan 4, 2024
@Veykril
Copy link
Member Author

Veykril commented Jan 4, 2024

Turns out this desugaring only happens for declarative macros?

@Veykril Veykril added A-macro macro expansion E-medium labels Jan 4, 2024
@saiintbrisson
Copy link
Contributor

saiintbrisson commented Jan 4, 2024

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

@Veykril
Copy link
Member Author

Veykril commented Jan 4, 2024

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

@saiintbrisson
Copy link
Contributor

@rustbot claim

@saiintbrisson
Copy link
Contributor

I haven't forgotten about this issue. Just Really Busy Lately ™️. This is pretty much what was said in #16259 (comment), 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.

@Veykril
Copy link
Member Author

Veykril commented Feb 22, 2024

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.

@nerditation
Copy link

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.

@Veykril Veykril assigned Veykril and unassigned saiintbrisson Apr 27, 2024
@bors bors closed this as completed in f216be4 Apr 27, 2024
lnicola pushed a commit to lnicola/rust that referenced this issue May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion A-proc-macro proc macro C-bug Category: bug E-medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants