Skip to content

Conversation

zachs18
Copy link
Contributor

@zachs18 zachs18 commented Sep 12, 2025

... since it is unreachable and would ICE anyway.

These branches are unreachable with how store_fn_arg is currently used (where it is called, unsized arguments are either: 1. not (yet) supported, or 2. handled differently)1, and even if they were reachable, they would ICE anyway, since they call OperandValue::store, which calls OperandValue::store_with_flags which panics on any unsized layout.

Also updates the bug! message in store_arg to not suggest store_fn_arg for unsized args.

Footnotes

  1. store_fn_arg is only nontrivially2 called in compiler/rustc_codegen_ssa/src/mir/mod.rs for: Line 428 extern "rust-call" tuple (un)splitting, which does not support unsized arguments, Line 496 which is only for sized PassMode::Indirect (meta_attrs: None) arguments, and Line 521 which is only for non-PassMode::Indirect arguments which can never be unsized.

  2. <Bx as ArgAbiBuilderMethods>::store_fn_arg is what is actually called, but codegen_llvm and codegen_gcc's builders both delegate to their own codegen_crate::ArgAbiExt::store_fn_arg, which contain the actual implementations that are changed in this PR.

@rustbot
Copy link
Collaborator

rustbot commented Sep 12, 2025

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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 Sep 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 12, 2025

r? @davidtwco

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

@davidtwco
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 17, 2025

📌 Commit baed55c has been approved by davidtwco

It is now in the queue for this repository.

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2025
bors added a commit that referenced this pull request Sep 17, 2025
Rollup of 7 pull requests

Successful merges:

 - #146458 (Add parallel-frontend-threads to bootstrap.toml and enable multi-threaded parallel compilation)
 - #146485 (Remove unsized arg handling in `ArgAbiBuilderMethods::store_fn_arg` implementations)
 - #146536 (clean up several trait related UI tests)
 - #146598 (Make llvm_enzyme a regular cargo feature)
 - #146647 (Move `#[rustc_coherence_is_core]` to the `crate_level` file)
 - #146654 (Do not use `git -C dir`)
 - #146681 (Add space after brace in `Box<[T]>::new_uninit_slice` example)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 802343f into rust-lang:master Sep 18, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 18, 2025
rust-timer added a commit that referenced this pull request Sep 18, 2025
Rollup merge of #146485 - zachs18:store_fn_arg-no-unsized, r=davidtwco

Remove unsized arg handling in `ArgAbiBuilderMethods::store_fn_arg` implementations

... since it is unreachable and would ICE anyway.

These branches are unreachable with how `store_fn_arg` is currently used (where it is called, unsized arguments are either: 1. not (yet) supported, or 2. handled differently)[^1], and even if they were reachable, they would ICE anyway, since they call [`OperandValue::store`](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_codegen_ssa/mir/operand.rs.html#855-861), which calls [`OperandValue::store_with_flags`](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_codegen_ssa/mir/operand.rs.html#887-926) which [panics on any unsized layout](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_codegen_ssa/mir/operand.rs.html#900-903).

Also updates the `bug!` message in `store_arg` to not suggest `store_fn_arg` for unsized args.

[^1]: `store_fn_arg` is only nontrivially[^2] called in `compiler/rustc_codegen_ssa/src/mir/mod.rs` for: Line 428 `extern "rust-call"` tuple (un)splitting, which does not support unsized arguments, Line 496 which is only for sized `PassMode::Indirect` (`meta_attrs: None`) arguments, and Line 521 which is only for non-`PassMode::Indirect` arguments which can never be unsized.

[^2]: `<Bx as ArgAbiBuilderMethods>::store_fn_arg` is what is actually called, but codegen_llvm and codegen_gcc's builders both delegate to their own `codegen_crate::ArgAbiExt::store_fn_arg`, which contain the actual implementations that are changed in this PR.
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Oct 9, 2025
…zelmann

Rollup of 7 pull requests

Successful merges:

 - rust-lang#146458 (Add parallel-frontend-threads to bootstrap.toml and enable multi-threaded parallel compilation)
 - rust-lang#146485 (Remove unsized arg handling in `ArgAbiBuilderMethods::store_fn_arg` implementations)
 - rust-lang#146536 (clean up several trait related UI tests)
 - rust-lang#146598 (Make llvm_enzyme a regular cargo feature)
 - rust-lang#146647 (Move `#[rustc_coherence_is_core]` to the `crate_level` file)
 - rust-lang#146654 (Do not use `git -C dir`)
 - rust-lang#146681 (Add space after brace in `Box<[T]>::new_uninit_slice` example)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants