Add `#[rustc_pass_indirectly_in_non_rustic_abis]` #144529

beetrees · 2025-07-27T10:40:17Z

This PR adds an internal #[rustc_pass_indirectly_in_non_rustic_abis] attribute that can be applied to structs. Structs marked with this attribute will always be passed using PassMode::Indirect { on_stack: false, .. } when being passed by value to functions with non-Rustic calling conventions. This is needed by #141980; see that PR for further details.

cc @joshtriplett

rustbot · 2025-07-27T10:40:23Z

r? @SparrowLii

rustbot has assigned @SparrowLii.
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

rustbot · 2025-07-27T10:40:25Z

Some changes occurred in compiler/rustc_attr_data_structures

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

RalfJung · 2025-07-27T16:40:45Z

compiler/rustc_target/src/callconv/amdgpu.rs

    Ty: TyAbiInterface<'a, C> + Copy,
    C: HasDataLayout,
 {
+    if arg.layout.pass_indirectly_in_non_rustic_abis(cx) {


If we rely on every single callconv getting this right, we're toast. It's way too easy to forget this somewhere.

Is there some way we can do this centrally for all ABIs?
For instance, we could apply this logic after the target-specific ABI stuff has been done.
Cc @workingjubilee

The individual calling conventions sometimes still need to be aware of the parameters, to update the number of remaining general-purpose registers, and the current design of the calling convention code makes it hard to abstract this. Separately from this PR, I've been planning to refactor the calling convention handling a bit as even without this change there's a lot of code duplication already (all the compute_abi_info functions are essentially variants of the same function with calls to classify_arg and classify_ret); this refactoring should make it possible to do this in a more centralised way.

For now, this PR previously had a cfg!(debug_assertions)-guarded check at the end of adjust_for_foreign_abi in callconv/mod.rs that asserts that individual calling convention correctly set all the #[rustc_pass_indirectly_in_non_rustic_abis] arguments to be passed indirectly. I've updated the check so it now always run rather than just running when debug_assetions are enabled.

The individual calling conventions sometimes still need to be aware of the parameters, to update the number of remaining general-purpose registers,

Urgh, right, I forgot we need to care about low-level nonsense like that here. :/

Regarding refactoring the ABI code, also see #119183. I think @workingjubilee also has some thoughts in that direction. I'm happy to discuss design options and provide feedback.

Not the most robust idea, but we there could be some kind of ICE-causing bomb that gets defused when checking an arg's pass_indirectly_in_non_rustic_abis and ignored if there are no args. This at least makes sure that new targets don't get very far if they miss this important detail.

Or a codegen test that gets run on all targets?

We have some code doing sanity checks on the ABI after it got computed. We could probably add an assertion there.

rust/compiler/rustc_ty_utils/src/abi.rs

Lines 368 to 372 in d71ed8d

fn fn_abi_sanity_check<'tcx>(

cx: &LayoutCx<'tcx>,

fn_abi: &FnAbi<'tcx, Ty<'tcx>>,

spec_abi: ExternAbi,

) {

The indirect_argument function would not be codegened if it doesn't get called, so that wouldn't trigger the ABI check. It should probably just be a ui test. This won't run in CI for tier 3 targets, but the worst that can happen is that you get an ICE, not a silent miscompilation.

How do we make sure that triggers for every ABI though?

It's an unstable attribute, so worst case we get an ICE when building libcore. That seems fine.

The indirect_argument function would not be codegened if it doesn't get called, so that wouldn't trigger the ABI check.

what if we use a const fn? Maybe using some const _: () = assert!(/* ... */), would that work?

Using const does work, I added that and a fix for transparent wrappers ignoring the attribute at master...folkertdev:rust:pass-indirectly-attr-updates

@beetrees feel free to steal or chery-pick from that. I'd also happily force-push to this branch if you don't have time/interest. (this is on the critical path for c-variadics now that the error messages are in a good state, and I'd hate to waste the reviewer momentum).

I've cherry-picked your patch for this.

RalfJung · 2025-07-27T20:28:44Z

@jdonszelmann could you have a look at the attribute code here? This is my first time actually seeing the new infrastructure so I can't say if the way it is used here is correct.

RalfJung

The compiler part LGTM apart from these comments, but I can't really review these ABI adjustments.

compiler/rustc_target/src/callconv/riscv.rs

compiler/rustc_target/src/callconv/x86_64.rs

bors · 2025-07-31T20:59:45Z

☔ The latest upstream changes (presumably #144740) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2025-08-01T01:43:28Z

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann

SparrowLii · 2025-08-04T01:26:48Z

r? @joshtriplett :)

rustbot · 2025-08-04T01:26:52Z

joshtriplett is not on the review rotation at the moment.
They may take a while to respond.

folkertdev · 2025-09-03T18:48:06Z

Besides Jubilee, who is not available, the only other person I can think of to review ABI code is

r? @bjorn3

bors · 2025-09-09T16:22:05Z

☔ The latest upstream changes (presumably #146360) made this pull request unmergeable. Please resolve the merge conflicts.

…jorn3 Add `#[rustc_pass_indirectly_in_non_rustic_abis]` This PR adds an internal `#[rustc_pass_indirectly_in_non_rustic_abis]` attribute that can be applied to structs. Structs marked with this attribute will always be passed using `PassMode::Indirect { on_stack: false, .. }` when being passed by value to functions with non-Rustic calling conventions. This is needed by rust-lang#141980; see that PR for further details. cc ``@joshtriplett``

Rollup of 9 pull requests Successful merges: - #133149 (Provide more context on `Fn` closure modifying binding) - #144529 (Add `#[rustc_pass_indirectly_in_non_rustic_abis]`) - #145915 (Stabilize `fmt::from_fn`) - #145974 (Stabilize -Zno-jump-tables into -Cjump-tables=bool) - #146057 (feat: add `from_fn_ptr` to `Waker` and `LocalWaker`) - #146301 (library: std: sys: net: uefi: tcp: Implement write_vectored) - #148437 (Regression test for undefined `__chkstk` on `aarch64-unknown-uefi`) - #148448 (Update books) - #148451 (tidy: Fix false positives with absolute repo paths in `pal.rs` `check()`) r? `@ghost` `@rustbot` modify labels: rollup

Zalathar · 2025-11-04T02:44:22Z

Failed in rollup: #148460 (comment)

//@ add-core-stubs has been renamed to //@ add-minicore.

compiletest: rename add-core-stubs/core-stubs-compile-flags to add-minicore/minicore-compile-flags #148305

@bors r-

Also emit an error when `rustc_pass_indirectly_in_non_rustic_abis` is used in combination with `repr(transparent)`.

rustbot · 2025-11-04T09:07:24Z

This PR was rebased onto a different master 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.

bjorn3 · 2025-11-04T10:53:33Z

@bors r+

bors · 2025-11-04T10:53:36Z

📌 Commit 7be6d6f has been approved by bjorn3

It is now in the queue for this repository.

Add `#[rustc_pass_indirectly_in_non_rustic_abis]` This PR adds an internal `#[rustc_pass_indirectly_in_non_rustic_abis]` attribute that can be applied to structs. Structs marked with this attribute will always be passed using `PassMode::Indirect { on_stack: false, .. }` when being passed by value to functions with non-Rustic calling conventions. This is needed by #141980; see that PR for further details. cc `@joshtriplett`

bors · 2025-11-04T12:05:40Z

⌛ Testing commit 7be6d6f with merge a9b8649...

Zalathar · 2025-11-04T12:08:00Z

Yielding to enclosing rollup.

@bors retry

Rollup of 4 pull requests Successful merges: - #144529 (Add `#[rustc_pass_indirectly_in_non_rustic_abis]`) - #147017 (FCW for repr(C) enums whose discriminant values do not fit into a c_int or c_uint) - #148459 (bootstrap: Split out a separate `./x test bootstrap-py` step) - #148468 (add logging to `fudge_inference_if_ok`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #144529 - beetrees:pass-indirectly-attr, r=bjorn3 Add `#[rustc_pass_indirectly_in_non_rustic_abis]` This PR adds an internal `#[rustc_pass_indirectly_in_non_rustic_abis]` attribute that can be applied to structs. Structs marked with this attribute will always be passed using `PassMode::Indirect { on_stack: false, .. }` when being passed by value to functions with non-Rustic calling conventions. This is needed by #141980; see that PR for further details. cc `@joshtriplett`

error on non-rustic ABIs using unsized parameters tracking issue: #48055 This came up in #144529 (comment). The idea is that the layout of an unsized type is unstable (following the rust layout rules), and hence stable ABIs should not use unsized types. On stable, unsized types (or generics with a `?Sized` bound) are not accepted as parameters, so the errors introduced by this PR can only be observed when the unstable `unsized_fn_params` feature is enabled. r? `@bjorn3` cc `@RalfJung`

Zalathar · 2025-11-05T13:30:25Z

Perf results show a few regressions in large-workspace:

error on non-rustic ABIs using unsized parameters tracking issue: #48055 This came up in #144529 (comment). The idea is that the layout of an unsized type is unstable (following the rust layout rules), and hence stable ABIs should not use unsized types. On stable, unsized types (or generics with a `?Sized` bound) are not accepted as parameters, so the errors introduced by this PR can only be observed when the unstable `unsized_fn_params` feature is enabled. r? `@bjorn3` cc `@RalfJung`

folkertdev

How should I evaluate that regression? There is one minor optimization in a comment below, but beyond that I think the cost is additional queries for the attribute flag, and that is impossible to work around I think (plus should be mostly cached).

View changes since this review

folkertdev · 2025-11-05T15:35:44Z

compiler/rustc_middle/src/ty/mod.rs


+        // See `TyAndLayout::pass_indirectly_in_non_rustic_abis` for details.
+        if find_attr!(
+            self.get_all_attrs(did),


This could be re-used from earlier

Zalathar · 2025-11-05T22:43:04Z

I thought the regression looked real, but the same benchmarks bounced back in #148507 (comment), so maybe it’s noise?

workingjubilee · 2025-11-06T18:14:23Z

@folkertdev please feel free to submit the PR and we can run perf to see if it matters.

rustbot assigned SparrowLii Jul 27, 2025

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) 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 27, 2025

beetrees mentioned this pull request Jul 27, 2025

Rework c_variadic #141980

Open

RalfJung reviewed Jul 27, 2025

View reviewed changes

beetrees force-pushed the pass-indirectly-attr branch from c0357c1 to 61196cb Compare July 27, 2025 17:23

RalfJung reviewed Jul 29, 2025

View reviewed changes

compiler/rustc_target/src/callconv/riscv.rs Show resolved Hide resolved

compiler/rustc_target/src/callconv/x86_64.rs Show resolved Hide resolved

beetrees force-pushed the pass-indirectly-attr branch from 61196cb to ed746a3 Compare July 29, 2025 15:27

beetrees force-pushed the pass-indirectly-attr branch from ed746a3 to d91160a Compare August 1, 2025 01:43

This comment has been minimized.

Sign in to view

beetrees force-pushed the pass-indirectly-attr branch from d91160a to 0401bf3 Compare August 1, 2025 01:59

rustbot assigned joshtriplett and unassigned SparrowLii Aug 4, 2025

beetrees force-pushed the pass-indirectly-attr branch from 0401bf3 to b8bd968 Compare September 1, 2025 15:34

This comment has been minimized.

Sign in to view

rustbot assigned bjorn3 and unassigned joshtriplett Sep 3, 2025

folkertdev added the F-c_variadic `#![feature(c_variadic)]` label Sep 9, 2025

beetrees force-pushed the pass-indirectly-attr branch from b8bd968 to 1953e54 Compare September 10, 2025 20:29

This comment has been minimized.

Sign in to view

Zalathar mentioned this pull request Nov 4, 2025

Rollup of 9 pull requests #148460

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 4, 2025

beetrees and others added 3 commits November 4, 2025 09:56

Add #[rustc_pass_indirectly_in_non_rustic_abis]

7354d3d

assert that #[rustc_pass_indirectly_in_non_rustic_abis] is respected

1866b3a

additional tests for pass_indirectly_in_non_rustic_abis

7be6d6f

Also emit an error when `rustc_pass_indirectly_in_non_rustic_abis` is used in combination with `repr(transparent)`.

folkertdev force-pushed the pass-indirectly-attr branch from 27a34ec to 7be6d6f Compare November 4, 2025 09:07

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 4, 2025

Zalathar mentioned this pull request Nov 4, 2025

Rollup of 4 pull requests #148472

Merged

bors merged commit fa9ea6d into rust-lang:master Nov 4, 2025
11 of 12 checks passed

rustbot added this to the 1.93.0 milestone Nov 4, 2025

folkertdev reviewed Nov 5, 2025

View reviewed changes

folkertdev mentioned this pull request Nov 6, 2025

re-use self.get_all_attrs result for pass indirectly attribute #148600

Open

	fn fn_abi_sanity_check<'tcx>(
	cx: &LayoutCx<'tcx>,
	fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
	spec_abi: ExternAbi,
	) {

Add #[rustc_pass_indirectly_in_non_rustic_abis] #144529

Add #[rustc_pass_indirectly_in_non_rustic_abis] #144529

Conversation

beetrees commented Jul 27, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 27, 2025

Uh oh!

rustbot commented Jul 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Jul 27, 2025

Uh oh!

RalfJung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bors commented Jul 31, 2025

Uh oh!

rustbot commented Aug 1, 2025

Uh oh!

This comment has been minimized.

SparrowLii commented Aug 4, 2025

Uh oh!

rustbot commented Aug 4, 2025

Uh oh!

This comment has been minimized.

folkertdev commented Sep 3, 2025

Uh oh!

bors commented Sep 9, 2025

Uh oh!

This comment has been minimized.

Zalathar commented Nov 4, 2025

Uh oh!

rustbot commented Nov 4, 2025

Uh oh!

bjorn3 commented Nov 4, 2025

Uh oh!

bors commented Nov 4, 2025

Uh oh!

bors commented Nov 4, 2025

Uh oh!

Zalathar commented Nov 4, 2025

Uh oh!

Uh oh!

Zalathar commented Nov 5, 2025

Uh oh!

folkertdev left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zalathar commented Nov 5, 2025

Uh oh!

workingjubilee commented Nov 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Add `#[rustc_pass_indirectly_in_non_rustic_abis]` #144529

Add `#[rustc_pass_indirectly_in_non_rustic_abis]` #144529

beetrees commented Jul 27, 2025 •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading