Skip to content

Fix a splat legacy demangling display bug - #160050

Open
teor2345 wants to merge 2 commits into
rust-lang:mainfrom
teor2345:splat-legacy-demangle
Open

Fix a splat legacy demangling display bug#160050
teor2345 wants to merge 2 commits into
rust-lang:mainfrom
teor2345:splat-legacy-demangle

Conversation

@teor2345

@teor2345 teor2345 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Tracking issue: #153629

This PR fixes a splat legacy demangling display bug, where the comma is in the wrong place in the argument list.

It also does a cleanup from @folkertdev's PR #159643 review:

@rustbot label +F-splat +C-bug +A-name-mangling

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 28, 2026
@rustbot

rustbot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

r? @camelid

rustbot has assigned @camelid.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 74 candidates
  • Random selection from 19 candidates

@rustbot rustbot added A-name-mangling Area: Name mangling / decoration aka symbol mangling C-bug Category: This is a bug. F-splat `#![feature(splat)]` https://github.com/rust-lang/rust/issues/153629 labels Jul 28, 2026
camelid
camelid requested changes Jul 28, 2026
View reviewed changes
Comment thread compiler/rustc_middle/src/ty/print/pretty.rs
if let Some(index) = splatted_arg_index {
self.comma_sep((&mut input_iter).take(usize::from(index)))?;
write!(self, ", #[splat]")?;
write!(self, "#[splat] ")?;

@camelid camelid Jul 28, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Won't this break any cases where there are arguments before the splat? Like fn foo(x: i32, #[splat] ...). This is the first time I'm hearing about this feature, but the code still seems wrong here.

View changes since the review

@camelid camelid Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems like a good idea to have some tests for splatting with regular arguments before and/or after the the splatted argument -- assuming that that's supported. If it's not supported, then I'm confused why splatted_arg_index exists and is not just always None or 0.

@teor2345 teor2345 Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Won't this break any cases where there are arguments before the splat? Like fn foo(x: i32, #[splat] ...). This is the first time I'm hearing about this feature, but the code still seems wrong here.

Turns out comma-separated lists are hard 😭
I fixed the logic and added a comment for future maintainers, this is my third try at getting this right.

It seems like a good idea to have some tests for splatting with regular arguments before and/or after the the splatted argument

I also added tests to make sure to cover leading, trailing, middle, and internal splats. (The original tests were adapted from tests for a symbol mangling clash, so I didn't really think about splat positioning at the time.)

@rustbot rustbot added S-waiting-on-author Status: This is 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 28, 2026
@rustbot

rustbot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

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

@teor2345
teor2345 force-pushed the splat-legacy-demangle branch from c82af0a to 980ae5d Compare July 29, 2026 01:15
@rustbot

rustbot commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@teor2345

teor2345 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

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

This comment has been minimized.

Sign in to view
@teor2345
teor2345 force-pushed the splat-legacy-demangle branch from 980ae5d to 1099c87 Compare July 29, 2026 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@camelid camelid camelid requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@camelid camelid

Labels

A-name-mangling Area: Name mangling / decoration aka symbol mangling C-bug Category: This is a bug. F-splat `#![feature(splat)]` https://github.com/rust-lang/rust/issues/153629 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@teor2345 @rustbot @rust-log-analyzer @camelid