-
Notifications
You must be signed in to change notification settings - Fork 898
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 removes crucial code from macro calls (regression) #6424
Comments
Thanks for the report. I tracked this change in behavior back to rust-lang/rust#129514 |
Note that it also removes values which are not contiguous (let's say if |
@nazar-pc sorry, but I'm having a hard time understanding what you're getting at. Could you provide a code snippet? |
Sure: frame_support::construct_runtime!(
pub struct Test {
System: frame_system = 60,
SelfDomainId: pallet_domain_id = 1,
}
); In this case both will be removed too. I just assumed that in some cases it might remove things due to considering them being "the default", but no. |
Thanks for the update. No, I don't think that's the case. An explanation for what's going on here can be found at rust-lang/rust#134668 |
…ue-fmt, r=ytmimi Make sure we don't lose default struct value when formatting struct The reason why rust-lang/rustfmt#6424 broke when rust-lang#129514 landed is because the parser now *successfully* parses default struct values. That means that where we used to fail in `rewrite_macro_inner`: https://github.com/rust-lang/rust/blob/e108481f74ff123ad98a63bd107a18d13035b275/src/tools/rustfmt/src/macros.rs#L263-L267 ... we now succeed, and we now proceed to format the inner struct as a macro arg. Since formatting was never implemented for default struct values, this means that we’ll always rewrite the struct to exclude the default value. This PR makes it so that we simply don’t rewrite struct fields if they have a default value. r? `@ytmimi`
Probably related to #6418, but not 100% sure.
rustfmt recently started formatting code like this incorrectly:
Specifically, it removes
= NUMBER
for some reason, resulting in this:This is incorrect and not an equivalent change.
Affected version:
Version that I used previously and that worked correctly:
The text was updated successfully, but these errors were encountered: