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

rustfmt stopped formatting const items in macros #6035

Open
CAD97 opened this issue Jan 21, 2024 · 6 comments
Open

rustfmt stopped formatting const items in macros #6035

CAD97 opened this issue Jan 21, 2024 · 6 comments
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.

Comments

@CAD97
Copy link
Contributor

CAD97 commented Jan 21, 2024

Test:

> echo "m!(const N: usize = 0;);" | rustup run beta rustfmt
m!(
    const N: usize = 0;
);

> echo "m!(const N: usize = 0;);" | rustup run nightly rustfmt
m!(const N: usize = 0;);

> rustup run beta rustfmt -Vv
rustfmt 1.7.0-beta (f732c37b417 2024-01-12)
> rustup run nightly rustfmt -Vv
rustfmt 1.7.0-nightly (88189a71 2024-01-19)
@ytmimi
Copy link
Contributor

ytmimi commented Jan 21, 2024

We haven't done a subtree sync since 2023-10-22 rust-lang/rust#117066

Running rustfmt built from source :rustfmt 1.7.0-nightly (bf96731 2024-01-20)

cargo run --bin rustfmt <<< "m!(const N: usize = 0;);"
#output
m!(const N: usize = 0;);

Running rustfmt nightly-2023-10-22 (rustfmt 1.6.0-nightly (1c05d50c 2023-10-21))

rustfmt +nightly-2023-10-22 <<< "m!(const N: usize = 0;);"
m!(
    const N: usize = 0;
);

Running rustfmt nightly-2023-10-23 (rustfmt 1.6.0-nightly (54b0434c 2023-10-22))

rustfmt +nightly-2023-10-23 <<< "m!(const N: usize = 0;);"          
m!(
    const N: usize = 0;
);

Running rustfmt nightly-2023-10-24 (rustfmt 1.7.0-nightly (cd674d61 2023-10-24)):

rustfmt +nightly-2023-10-24 <<< "m!(const N: usize = 0;);"
m!(
    const N: usize = 0;
);

Running rustfmt nightly-2023-10-25 (rustfmt 1.7.0-nightly (df871fbf 2023-10-24)):

rustfmt +nightly-2023-10-25 <<< "m!(const N: usize = 0;);" 
m!(
    const N: usize = 0;
);

We've done 2 subtree pushes since the last release: #5980 and #5994. The last of which brought in changes from nightly-2023-12-28

Not really sure what the issue is right now, but given that bors merged the last subtree sync on 2023-10-23 I'd assume that the error would have presented itself in the nightly-2023-10-24 release. Will try to look into this more a little later to figure out which nightly version started demonstrating this different macro formatting behavior.

@ytmimi ytmimi added bug Panic, non-idempotency, invalid code, etc. a-macros labels Jan 21, 2024
@ytmimi
Copy link
Contributor

ytmimi commented Jan 21, 2024

Tracked the issue down to the nightly-2023-12-28 release.

rustfmt +nightly-2023-12-27 <<< "m!(const N: usize = 0;);"
m!(
    const N: usize = 0;
);
rustfmt +nightly-2023-12-28 <<< "m!(const N: usize = 0;);"
m!(const N: usize = 0;);

@ytmimi
Copy link
Contributor

ytmimi commented Jan 21, 2024

bisected the formatting changes back to rust-lang/rust#119099

@compiler-errors
Copy link
Member

compiler-errors commented Jan 22, 2024

This has to do with the weirdness of the way that parse_macro_arg works. Unlike parsing nonterminal args in a macro-by-example, it eagerly tries, for example, to parse a type without checking that the beginning token may begin a type:

|parser: &mut rustc_parse::parser::Parser<'b>| parser.parse_ty(),

Contrast this to the nonterminal parsing code, which first checks that the nonterminal may begin with a given token:

https://github.com/rust-lang/rust/blob/ef71f1047e04438181d7cb925a833e2ada6ab390/compiler/rustc_parse/src/parser/nonterminal.rs#L47

In rust-lang/rust#119099, @fmease implemented a change so that const Tr would be parsed as dyn const Tr (a trait object to a const trait) in edition 2015.

This is okay for the purposes of macros, because he explicitly made sure that const did not get added to the list of tokens that may begin a :ty nonterminal kind: rust-lang/rust#119099 (comment)

However, since rustfmt is not so careful about eagerly parsing macro args before checking that they're legal in macro position, this changed the way that the string of tokens is being parsed into macro args.

I put up a fix against rust-lang/rust (since that should fix the nightly rustfmt): rust-lang/rust#120218.

@ytmimi
Copy link
Contributor

ytmimi commented Jan 22, 2024

@compiler-errors thank you for the detailed explanation and for jumping into action so quickly on this one 🙏🏼

@ytmimi
Copy link
Contributor

ytmimi commented Jan 24, 2024

Just checked and nightly-2024-01-23 restored the formatting.

rustfmt +nightly-2024-01-23 <<< "m!(const N: usize = 0;);"
m!(
    const N: usize = 0;
);

will hold off on closing this until the next subree push so that we incorporate the fix from rust-lang/rust back into rustfmt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

No branches or pull requests

3 participants