Skip to content

Avoid repeated rewrites of overflowing call arguments#6962

Open
srghma wants to merge 2 commits into
rust-lang:mainfrom
srghma:fix-nested-overflow-rewrite-cache
Open

Avoid repeated rewrites of overflowing call arguments#6962
srghma wants to merge 2 commits into
rust-lang:mainfrom
srghma:fix-nested-overflow-rewrite-cache

Conversation

@srghma

@srghma srghma commented Jul 18, 2026

Copy link
Copy Markdown

Summary

Testing

  • cargo fmt --check
  • cargo check -p rustfmt-nightly
  • cargo test -p rustfmt-nightly --lib overflow
  • cargo test system_tests
  • target/release/rustfmt --edition 2024 --emit stdout tests/source/survive-peano-lesson-queue.rs

@rustbot rustbot added the S-waiting-on-review Status: awaiting review from the assignee but also interested parties. label Jul 18, 2026

@ytmimi ytmimi Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the test case need to be this big? Would be nice if this was smaller so that we could see the code changes in the GitHub UI.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's copied exactly as it is from rust repo. Yes, I could try make it smaller, but my ai subscription ended. Maybe leave as is?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the source file. Having the formatted target file should be enough

Comment thread tests/config/survive-peano-lesson-queue.toml
@ytmimi ytmimi self-assigned this Jul 21, 2026
@srghma
srghma force-pushed the fix-nested-overflow-rewrite-cache branch from 281c10d to 0f27271 Compare July 22, 2026 16:32
@srghma
srghma force-pushed the fix-nested-overflow-rewrite-cache branch from 0f27271 to 7c964f1 Compare July 22, 2026 17:33

@ytmimi ytmimi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at my comments for the changes I think we should make.

View changes since this review

Comment thread src/rewrite.rs
Comment on lines +19 to +32
#[derive(Clone, Eq, Hash, PartialEq)]
pub(crate) struct OverflowRewriteKey {
span_lo: u32,
span_hi: u32,
width: usize,
block_indent: usize,
alignment: usize,
offset: usize,
inside_macro: bool,
use_block: bool,
is_if_else_block: bool,
is_loop_block: bool,
force_one_line_chain: bool,
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add new fields to RewriteContext do we need to update what gets included in OverflowRewriteKey? If so then it probably makes sense to leave a comment explaining that on RewriteContext.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the source file. Having the formatted target file should be enough

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, Let's remove this too. I don't think it's needed.

@rustbot rustbot added S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: awaiting review from the assignee but also interested parties. labels Jul 23, 2026
@rustbot

rustbot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

runs infinitely on https://github.com/rust-lang/rust/blob/main/tests/ui/parser/survive-peano-lesson-queue.rs

3 participants