Skip to content

Arg splat experiment - HIR FnSig impl#157811

Open
teor2345 wants to merge 1 commit into
rust-lang:mainfrom
teor2345:fn-arg-splat-exp-fnsig
Open

Arg splat experiment - HIR FnSig impl#157811
teor2345 wants to merge 1 commit into
rust-lang:mainfrom
teor2345:fn-arg-splat-exp-fnsig

Conversation

@teor2345

Copy link
Copy Markdown
Contributor

View all comments

This PR is part of the argument splatting lang experiment, and FFI overloading / C++ interop project goals:

I've split it from #153697 to make reviewing easier, see that PR for more details.

The PR is the HIR-level FnSig implementation of the feature (next PR will be typecheck):

  • Add splat to HIR FnSig
    • which changes legacy mangling scheme function hashes due to the extra bytes
  • splat mismatch support, including function pretty-printing
  • various FIXME comments

Once this PR merges, I'll rebase #153697.

There aren't any extra tests in this PR, because it doesn't have any externally visible functionality. The visible functionality and tests are in the rest of #153697.

@rustbot

rustbot commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

HIR ty lowering was modified

cc @fmease

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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

cc @rust-lang/rust-analyzer

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

cc @rust-lang/miri

@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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Jun 12, 2026
@rustbot

rustbot commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

r? @dingxiangfei2009

rustbot has assigned @dingxiangfei2009.
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, types
  • compiler, types expanded to 73 candidates
  • Random selection from 19 candidates

#[type_foldable(identity)]
splatted: u16,

_marker: PhantomData<fn() -> I>,

@teor2345 teor2345 Jun 12, 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.

These attributes don't seem to be necessary, but should they be kept anyway?

Suggested change
_marker: PhantomData<fn() -> I>,
#[type_visitable(ignore)]
#[type_foldable(identity)]
_marker: PhantomData<fn() -> I>,

View changes since the review

@rust-bors

This comment has been minimized.

@teor2345 teor2345 force-pushed the fn-arg-splat-exp-fnsig branch from 8a2c568 to d2417dc Compare June 15, 2026 04:44
@rustbot

rustbot commented Jun 15, 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.

@panstromek

Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 16, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 16, 2026
@rust-bors

rust-bors Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: d8eafa9 (d8eafa9dcccd482a75209dc4bc1bd346e954d333, parent: 01dfd79246f1b2d5f146616deff08223a840a9ae)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (d8eafa9): comparison URL.

Overall result: ❌ regressions - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.2% [0.1%, 0.9%] 14
Regressions ❌
(secondary)
0.3% [0.1%, 0.5%] 16
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.1%, 0.9%] 14

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Results (secondary -12.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.6% [4.6%, 4.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-28.8% [-28.8%, -28.8%] 1
All ❌✅ (primary) - - 0

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.2%] 84
Regressions ❌
(secondary)
0.1% [0.0%, 0.7%] 65
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [0.0%, 0.2%] 84

Bootstrap: 519.231s -> 521.837s (0.50%)
Artifact size: 401.41 MiB -> 400.94 MiB (-0.12%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 16, 2026
@teor2345

Copy link
Copy Markdown
Contributor Author

Ok, that's a significant regression we should fix before merging if possible. Waiting on perf on #157954 and #157955 to see if an attribute query (or side-table) or inlining is the best approach here.

@rustbot author

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

rustbot commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

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

@teor2345

teor2345 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Inlining doesn't work, so we'll need to do the equivalent of #157954 for FnSig: 1 bit in the FnSig to mark any splatting, then look up the actual splatted argument in the attributes or a side-table.

https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/macro.find_attr.html is how you query attrs for an item? I would guess find_attr!(tcx, def_id, AttributeKind::Splat => ())

This might require putting the argument attribute in the cross-crate def_id table, like we do for HIR here: https://github.com/rust-lang/rust/pull/157954/changes#diff-ad0c15bbde97a607d4758ec7eaf88248be5d6b8ae084dfc84127f81e3f7a9bb4R1880

This should improve performance by making FnSig smaller, but it won't be as good a #157954, because there are no spare bits in FnSig currently.

Edit: another alternative is just using one byte, and rejecting splatting past the 255th argument.

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

Labels

perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants