Skip to content

Fix formatting of commented-out empty generic bounds#6838

Open
Marsman1996 wants to merge 1 commit intorust-lang:mainfrom
Marsman1996:fix-6837
Open

Fix formatting of commented-out empty generic bounds#6838
Marsman1996 wants to merge 1 commit intorust-lang:mainfrom
Marsman1996:fix-6837

Conversation

@Marsman1996
Copy link
Copy Markdown

Fix #6837

Now for code

trait SpinGuardian {}

struct RwLockUpgradeableGuard<'a, T, G: SpinGuardian>(&'a T, G);

impl<T: /*?Sized*/, G: SpinGuardian> RwLockUpgradeableGuard<'_, T, G> {}

The format result is

trait SpinGuardian {}

struct RwLockUpgradeableGuard<'a, T, G: SpinGuardian>(&'a T, G);

impl<T /*?Sized*/, G: SpinGuardian> RwLockUpgradeableGuard<'_, T, G> {}

@rustbot rustbot added the S-waiting-on-review Status: awaiting review from the assignee but also interested parties. label Mar 23, 2026
Comment on lines -129 to +130
self.ident.span.hi()
self.colon_span
.map_or(self.ident.span.hi(), |colon_span| colon_span.hi())
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.

Can you help me understand how updating the span fixes the issue?

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.

Before the fix

The T: /*?Sized*/, is parsed into T and : /*?Sized*/,.
For : /*?Sized*/,, extract_post_comment removes the : but keeps the , here:

rustfmt/src/lists.rs

Lines 635 to 636 in bdab101

let post_snippet_trimmed = if post_snippet.starts_with(|c| c == ',' || c == ':') {
post_snippet[1..].trim_matches(white_space)

Since the remaining is start with the comment, so write_list adds an extra , here:

result.push_str(formatting.separator);

After the fix

The T: /*?Sized*/, is parsed into T: and /*?Sized*/,.
So extract_post_comment removes the original , here:

rustfmt/src/lists.rs

Lines 644 to 647 in bdab101

else if post_snippet.ends_with(separator)
&& (!post_snippet.trim().starts_with("//") || post_snippet.trim().contains('\n'))
{
post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space)

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.

Thanks for the explanation. It makes sense now. My initial thought is that it seems like a simple fix, but it also feels a little weird to extend the T -> T:. I want to think a bit more on the proposed solution to this one, but I'll get back to you.

Copy link
Copy Markdown
Author

@Marsman1996 Marsman1996 Apr 1, 2026

Choose a reason for hiding this comment

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

I do not think it is too weird, because for an ordinary bound like G: SpinGuardian, the GenericParam span G: SpinGuardian already includes the :.

And I also tried to fix by adding the , removal logic in extract_post_comment, but it will remove the /*?Sized*/ comment:

impl<T: /*Sized*/, G: SpinGuardian> RwLockUpgradeableGuard<'_, T, G> {}

is formatted to

impl<T, G: SpinGuardian> RwLockUpgradeableGuard<'_, T, G> {}

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.

Is that comment removal expected?
When I format

impl<T: /*Size*/ SpinGuardian, G: SpinGuardian> RwLockUpgradeableGuard<'_, T, G> {}

rustfmt also removes the inline comment and produces

impl<T: SpinGuardian, G: SpinGuardian> RwLockUpgradeableGuard<'_, T, G> {}

@ytmimi ytmimi self-assigned this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rustfmt rewrites valid impl<T: /*comment*/> into invalid syntax with ,,

3 participants