Repository navigation
"cfg-if"-included modules ignored if formatted after parsing a top-level macro in another file #4656
Description
Describe the bug
Rustfmt generally tries to follow cfg-if-conditional modules, which is great -- this allows parsing modules which are e.g. arch/OS specific. However, this logic operates incorrectly when parsed on the Nth file, for N > 1 where some Mth file (M < N) contains a top-level macro.
This is a bit quirky, and I'm not sure I'm using the best terminology, so I'll use an example:
To Reproduce
format_me_please.rs
// Any invalid formatting will do
pub fn hello ( ) { }
lib.rs:
cfg_if! {
if #[cfg(target_family = "unix")] {
mod format_me_please;
}
}
lib2.rs:
its_a_me_a_macro! {
// Any contents; this comment will suffice
}
With this setup, the bug can be reproduced on command-line using rustfmt, though it is also possible to repro via cargo fmt:
// This works as expected; lib.rs + format_me_please.rs are formatted.
// The "-v" lets us see that the `format_me_please.rs` file is detected + fixed.
$ rustfmt -v lib.rs
// BUG: This case is unexpected. Basically, lib2.rs and lib.rs are formatted,
// but the `format_me_please.rs` file is not even considered.
$ rustfmt -v lib2.rs lib.rsMeta
- rustfmt version: 1.33
- From where did you install rustfmt?: Rustup, but also compiled from source.
Activity
smklein commentedon Jan 19, 2021
I actually have a patch for this bug, but I wanted to get an issue number first so I could reference it in my PR. Will upload shortly!
TL;DR: Interned symbols have different indices when crossing files; the check for
cfg_ifis currently a lazily-initialized global variable, but when it becomes a local, this issue is resolved.fix: Avoid incorrect global 'cfg_if' Symbol interning
smklein commentedon Jan 19, 2021
Also, I think this is related to #3253
smklein commentedon Jan 19, 2021
Looking at rustfmt 2.0, it seems this issue was resolved by 3b7fd26 , but this PR is still relevant; as this bug currently exists in the 1.x branch
Format kern's cfg-if'd modules
Format kern's cfg-if'd modules
Format kern's cfg-if'd modules (#135)
fix: Avoid incorrect global 'cfg_if' Symbol interning
calebcartwright commentedon Jan 28, 2021
Have pulled this into the 1.4.33 release. Would you have any interest in adding your test case against the master branch? If not we can go ahead and close this
Rollup merge of rust-lang#81501 - calebcartwright:update-rustfmt, r=s…
ytmimi commentedon Apr 11, 2022
@calebcartwright as you mentioned you included this in the 1.4.33 release so I'm assuming this can be closed now
calebcartwright commentedon Apr 11, 2022
I still think a test case would be worthwhile, but sure; test case can be added independently of the open/closed status of this issue.
ytmimi commentedon Apr 12, 2022
@calebcartwright I think we've already got issue 4656 tests in the code base. Was there another test you had in mind? I can work to get a PR ready, I'm just not sure which test you had in mind.
calebcartwright commentedon Apr 12, 2022
Nope that works! Just bad memory on my part then 😅