Skip to content

Fix splat v0 mangling#158890

Open
teor2345 wants to merge 6 commits into
rust-lang:mainfrom
teor2345:splat-mangling-clash
Open

Fix splat v0 mangling#158890
teor2345 wants to merge 6 commits into
rust-lang:mainfrom
teor2345:splat-mangling-clash

Conversation

@teor2345

@teor2345 teor2345 commented Jul 7, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Tracking issue: #153629

Typecheck considers a splatted and non-splatted function as distinct types, so mangling has to include splatting. If it doesn't, we get symbol clashes. See the ticket and the tests in this PR for details.

Demangling PR: rust-lang/rustc-demangle#90
Demangling in rust-lang/rust PR: #159202

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

Fixes #158644

TODO

This PR is based on #159202, so we're waiting for it to merge before we rebase & merge this one.

@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 7, 2026
@rustbot

rustbot commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

r? @hanna-kruppe

rustbot has assigned @hanna-kruppe.
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 75 candidates
  • Random selection from 20 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)]` labels Jul 7, 2026
@teor2345 teor2345 mentioned this pull request Jul 7, 2026
Merged
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@teor2345

This comment was marked as resolved.

Sign in to view
wesleywiser
wesleywiser reviewed Jul 7, 2026
View reviewed changes

@wesleywiser wesleywiser left a comment •
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.

Just so you know, changes to the mangling scheme typically also have a corresponding PR to https://github.com/rust-lang/rustc-demangle 🙂

View changes since this review

Comment thread compiler/rustc_symbol_mangling/src/v0.rs Outdated
@teor2345 teor2345 force-pushed the splat-mangling-clash branch 2 times, most recently from a30c0d8 to afaae5b Compare July 8, 2026 03:16
@teor2345 teor2345 mentioned this pull request Jul 8, 2026
Merged
teor2345
teor2345 commented Jul 8, 2026
View reviewed changes
Comment thread tests/ui/splat/splat-mangling-issue-158644.rs Outdated
Comment on lines +9 to +10
// FIXME(splat): remove this once splat demangling has been added to rustc
//@ exec-env: RUST_BACKTRACE=0

@teor2345 teor2345 Jul 8, 2026 •
edited by rustbot
Loading

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.

Hopefully this should skip demangling in panic backtraces, until the demangling impl in rust-lang/rustc-demangle#90 is added to rustc

@rustbot ready

View changes since the review

@teor2345 teor2345 Jul 8, 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.

Just so you know, changes to the mangling scheme typically also have a corresponding PR to rust-lang/rustc-demangle

@wesleywiser, do we usually wait for the demangling PR to merge, or is there another way to disable demangling in backtraces?

Another alternative is dropping or modifying this test until the demangling merges.

@rustbot author

@wesleywiser wesleywiser Jul 8, 2026 •
edited
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.

I believe in the past, we've usually sequenced this kind of thing as:

  1. Add new mangling to rustc-demangle (done)
  2. Release new rustc-demangle to crates.io (done, v0.1.28)
  3. Update rust-lang/rust to use new rustc-demangle version
  4. Land the equivalent of this PR

Please also add an item to the tracking issue to update the v0 mangling documentation to include this new form when we stabilize it.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@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 8, 2026
@rustbot

rustbot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

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

@hanna-kruppe

hanna-kruppe commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

r? wesleywiser

@rustbot

rustbot commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

wesleywiser is currently at their maximum review capacity.
They may take a while to respond.

@teor2345 teor2345 force-pushed the splat-mangling-clash branch from afaae5b to 1c094c4 Compare July 13, 2026 04:32
@rustbot

rustbot commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

clippy is developed in its own repository. If possible, consider making this change to rust-lang/rust-clippy instead.

cc @rust-lang/clippy

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-clippy Relevant to the Clippy team. labels Jul 13, 2026
@rustbot

rustbot commented Jul 13, 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 13, 2026

Copy link
Copy Markdown
Contributor Author

The rustc-demangle version bump is in #159202, this PR starts with the commits from that PR, then has the working splat mangling and tests on top of them.

This PR should wait to merge until after #159202 merges, just in case we drop some of the version bumps from that PR.

@ada4a ada4a closed this Jul 13, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 13, 2026
@ada4a ada4a reopened this Jul 13, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 13, 2026
@ada4a

ada4a commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Oh god, pressed the wrong button 😅 Apologies

fmease
fmease reviewed Jul 13, 2026
View reviewed changes
Comment thread compiler/rustc_symbol_mangling/src/v0.rs

@fmease fmease Jul 13, 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.

Could you document this in the rustc book? We document even unstable mangling rules AFAICT.

View changes since the review

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

Reviewers

@wesleywiser wesleywiser wesleywiser left review comments
@fmease fmease fmease left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@wesleywiser wesleywiser

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-name-mangling Area: Name mangling / decoration aka symbol mangling C-bug Category: This is a bug. F-splat `#![feature(splat)]` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. 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.

Splat is ignored in symbol mangling, leading to symbol clashes

7 participants

@teor2345 @rustbot @rust-log-analyzer @hanna-kruppe @ada4a @wesleywiser @fmease