-
Notifications
You must be signed in to change notification settings - Fork 889
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
Line indented again each time rustfmt is run until it's 100 columns wide #5044
Comments
Thank you for the report and reproducible example! Idempotency issues are absolutely a bug so this will need to be sorted. This is reproducible on the latest version of rustfmt, but it would be useful if you could update the issue description to also note the version of rustfmt you are currently using for historical tracking. |
All right, version added to the description! |
So I've done some digging and I think I've been able to narrow down where the problem is, but will continue to search for the exact cause. TLDR; I think the issue is somewhere in starting at Lines 1342 to 1353 in a5f8505
Lines 348 to 433 in a5f8505
This is where I started to scratch my head, but @calebcartwright if you could provide some more context for why we need to artificially wrap code in an Take a look at the input snippet that we're trying to format. I've included the leading whitespace in comments. (note: leading whitespace seems to be missing from line 1 unsafe { // <-- 0
core::mem::MaybeUninit::<[core::mem::MaybeUninit<core::primitive::u8>; $capacity]> //<-- 12
::uninit() //<-- 16
} // <-- 8 on line 373 we call fn main() {
unsafe { // <-- 4
core::mem::MaybeUninit::<[core::mem::MaybeUninit<core::primitive::u8>; zcapacity]> //<--16
::uninit() // <-- 20
} // <-- 12
} The issue starts when we call I guess the main thing to note is the relevant indentation between lines is still preserved at this point. However, once we call fn main() {
unsafe { // <-- 4
core::mem::MaybeUninit::<[core::mem::MaybeUninit<core::primitive::u8>; zcapacity]> // <-- 8
::uninit() //<--20
} // <-- 4
} As you can see all lines are formatted except the Moving on. After we format the snippet we work to remove the enclosing Lines 397 to 427 in a5f8505
Now that we've removed the enclosing unsafe { // <-- 0
core::mem::MaybeUninit::<[core::mem::MaybeUninit<core::primitive::u8>; zcapacity]> // <-- 4
::uninit() // <-- 16
} // <-- 0 The important thing to note here is that the relative difference in whitespace between the first line and the second line used to be 4, but now it's 8. Taking a look at the output of running ($capacity:expr) => {
unsafe {
core::mem::MaybeUninit::<[core::mem::MaybeUninit<core::primitive::u8>; $capacity]>
- ::uninit() // <--16
+ ::uninit() // <--24
}
};
} From what I can gather the bug seems to be somewhere in |
Thanks for looking at this one @ytmimi There's a couple of issues at play (a similar variant of the scenario I described in #4629 (review)) but the short version is that we're failing to detect a macro formatting error, and that hits a path of non-idempotency with runaway indentation. rustfmt operates on the compiler's internal AST representation of input programs and needs valid/parseable input in order to get that AST representation. rustfmt knows how to format each type of AST node according to the respective rules codified in the Style Guide. Macros, both definitions and calls, are not (necessarily) valid Rust code, and their contents aren't represented as any specific type of ast node. There's a top level ast node (e.g. rustfmt, to this day, does not really handle macros, at least not well. In the case of macro calls, it uses the parser to see if any of the args "look like" (i.e. can be parsed as) standard/valid Rust code, and if so will attempt to format them. In the case of defs, it's even more complicated, and involves temporarily replacing certain tokens in an attempt to get something that looks more like valid Rust. In order to format invalid/incomplete/unparseable inputs (as is the case with snippets in doc comments, macro content, etc.) there's a process that produces a valid item (the |
I'm having the same issue but with let/else: macro_rules! get_arg {
($args:expr, $index:expr, $ty:ident) => {{
let Some(arg): Option<$ty> = $args
--> .get($index)
--> .and_then(|p| p.$ty()) else {
--> bail!("Expected a `{}` at argument {}", stringify!($ty), $index);
--> };
arg
}};
}
|
@doinkythederp thanks for reaching out! You're experiencing a distinct case of the original issue, which has been reported before in #5503 and others. Your issue should be resolved once support for |
The line containing
::uninit()
in the following code is repeatedly indented if rustfmt is run on the file repeatedly. This also causescargo fmt -- --check
to fail, even whencargo fmt
was just run, unless the line is as long as permissible.rustfmt version:
The text was updated successfully, but these errors were encountered: