Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) #136985

zachs18 · 2025-02-13T19:32:48Z

Accepted MCP: rust-lang/compiler-team#832

Do not consider the inhabitedness of a type for function call ABI purposes.

Remove the rustc_abi::BackendRepr::Uninhabited variant
- Instead calculate the BackendRepr of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc)
Add an uninhabited: bool field to rustc_abi::LayoutData so inhabitedness of a LayoutData can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it).

This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types.

cc @RalfJung

rustbot · 2025-02-13T19:33:00Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
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-02-13T19:33:02Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

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

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes Stable MIR

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

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

RalfJung · 2025-02-13T19:42:40Z

Cc @workingjubilee

workingjubilee

This requires a codegen test demonstrating the change.

compiler/rustc_codegen_llvm/src/intrinsic.rs

rustbot · 2025-02-14T22:38:24Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

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

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

This PR changes Stable MIR

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

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

zachs18 · 2025-02-14T23:17:07Z

bdc321b fixes codegen (I think) and adds a codegen test. (Previously codegen_call_terminator and make_return_dest assumed that a call with no target block always had a PassMode::Ignore'd return type, so wouldn't prepend the return-place pointer to llargs. Now, make_return_dest doesn't consider the target block, and codegen_call_terminator calls make_return_dest regardless of if there is a target block).

workingjubilee · 2025-02-17T22:09:20Z

cool! sounds like this is finding even more exciting issues in our codegen logic

workingjubilee

One last comment nit, I kind of wish we didn't use another public field here but LayoutData has a lot of construction sites and that would become a much bigger refactor, sooo... whatever.

r=me with nit

compiler/rustc_abi/src/lib.rs

workingjubilee · 2025-02-17T22:30:18Z

@bors delegate+

bors · 2025-02-17T22:30:21Z

✌️ @zachs18, you can now approve this pull request!

If @workingjubilee told you to "r=me" after making some further change, please make that change, then do @bors r=@workingjubilee

zachs18 · 2025-02-18T00:50:58Z

@bors r=@workingjubilee

bors · 2025-02-18T00:51:00Z

📌 Commit 85fe2b1 has been approved by workingjubilee

It is now in the queue for this repository.

…bited, r=workingjubilee Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) Accepted MCP: rust-lang/compiler-team#832 Fixes rust-lang#135802 Do not consider the inhabitedness of a type for function call ABI purposes. * Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc) * Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it). This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types. cc `@RalfJung`

Rollup of 8 pull requests Successful merges: - rust-lang#135767 (Future incompatibility warning `unsupported_fn_ptr_calling_conventions`: Also warn in dependencies) - rust-lang#136457 (Expose algebraic floating point intrinsics) - rust-lang#136985 (Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited)) - rust-lang#137000 (Deeply normalize item bounds in new solver) - rust-lang#137151 (Install more signal stack trace handlers) - rust-lang#137155 (Organize `OsString`/`OsStr` shims) - rust-lang#137161 (Pattern Migration 2024: fix incorrect messages/suggestions when errors arise in macro expansions) - rust-lang#137162 (Move methods from `Map` to `TyCtxt`, part 2.) r? `@ghost` `@rustbot` modify labels: rollup

…bited, r=workingjubilee Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) Accepted MCP: rust-lang/compiler-team#832 Fixes rust-lang#135802 Do not consider the inhabitedness of a type for function call ABI purposes. * Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc) * Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it). This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types. cc ``@RalfJung``

RalfJung · 2025-02-18T10:27:11Z

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -1274,11 +1274,11 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
        // FIXME: We could avoid some redundant checks here. For newtypes wrapping
        // scalars, we do the same check on every "level" (e.g., first we check
        // MyNewtype and then the scalar in there).
+        if val.layout.is_uninhabited() {


Nit: this is now in a section titled "check the ABI", which doesn't make a lot of sense.

I pushed a commit addressing this.

RalfJung · 2025-02-18T10:33:24Z

@bors r=@workingjubilee

bors · 2025-02-18T10:33:27Z

📌 Commit 8382b99 has been approved by workingjubilee

It is now in the queue for this repository.

…bited, r=workingjubilee Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) Accepted MCP: rust-lang/compiler-team#832 Fixes rust-lang#135802 Do not consider the inhabitedness of a type for function call ABI purposes. * Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc) * Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it). This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types. cc `@RalfJung`

…bited, r=workingjubilee Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) Accepted MCP: rust-lang/compiler-team#832 Fixes rust-lang#135802 Do not consider the inhabitedness of a type for function call ABI purposes. * Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc) * Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it). This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types. cc ``@RalfJung``

…ted, r=<try> Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) Accepted MCP: rust-lang/compiler-team#832 Fixes rust-lang#135802 Do not consider the inhabitedness of a type for function call ABI purposes. * Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc) * Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it). This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types. cc `@RalfJung` try-job: x86_64-gnu-nopt

bors · 2025-02-19T23:31:37Z

☀️ Try build successful - checks-actions
Build commit: 87f1eda (87f1eda379072e375e44fb5b9ddad2f03bb42e9e)

workingjubilee · 2025-02-20T01:45:14Z

Cool, we should be good. @bors r+ rollup=maybe

bors · 2025-02-20T01:45:16Z

📌 Commit ed34181 has been approved by workingjubilee

It is now in the queue for this repository.

bors · 2025-02-20T13:18:29Z

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

…l` field in `LayoutData`. Also update comments that refered to BackendRepr::Uninhabited.

…PassMode::Ignore.

…turn ABI as wrapped type. Fix codegen of uninhabited PassMode::Indirect return types. Add codegen test for uninhabited PassMode::Indirect return types. Enable optimizations for uninhabited return type codegen test

Co-authored-by: Jubilee <workingjubilee@gmail.com>

workingjubilee · 2025-02-20T19:52:24Z

Cool!

@bors r+

bors · 2025-02-20T19:52:27Z

📌 Commit e3f5db0 has been approved by workingjubilee

It is now in the queue for this repository.

…bited, r=workingjubilee Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) Accepted MCP: rust-lang/compiler-team#832 Fixes rust-lang#135802 Do not consider the inhabitedness of a type for function call ABI purposes. * Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc) * Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it). This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types. cc `@RalfJung`

…kingjubilee Rollup of 14 pull requests Successful merges: - rust-lang#131651 (Create a generic AVR target: avr-none) - rust-lang#136473 (infer linker flavor by linker name if it's sufficiently specific) - rust-lang#136608 (Pass through of target features to llvm-bitcode-linker and handling them) - rust-lang#136985 (Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited)) - rust-lang#137192 (Remove obsolete Windows ThinLTO+TLS workaround) - rust-lang#137204 (Clarify MIR dialects and phases) - rust-lang#137270 (Fix `*-win7-windows-msvc` target since 26eeac1) - rust-lang#137298 (Check signature WF when lowering MIR body) - rust-lang#137299 (Simplify `Postorder` customization.) - rust-lang#137312 (Update references to cc_detect.rs) - rust-lang#137313 (Some codegen_llvm cleanups) - rust-lang#137318 (Workaround Cranelift not yet properly supporting vectors smaller than 128bit) - rust-lang#137322 (Update docs for default features of wasm targets) - rust-lang#137324 (Make x86 QNX target name consistent with other Rust targets) r? `@ghost` `@rustbot` modify labels: rollup

…kingjubilee Rollup of 12 pull requests Successful merges: - rust-lang#131651 (Create a generic AVR target: avr-none) - rust-lang#134340 (Stabilize `num_midpoint_signed` feature) - rust-lang#136473 (infer linker flavor by linker name if it's sufficiently specific) - rust-lang#136608 (Pass through of target features to llvm-bitcode-linker and handling them) - rust-lang#136985 (Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited)) - rust-lang#137270 (Fix `*-win7-windows-msvc` target since 26eeac1) - rust-lang#137312 (Update references to cc_detect.rs) - rust-lang#137318 (Workaround Cranelift not yet properly supporting vectors smaller than 128bit) - rust-lang#137322 (Update docs for default features of wasm targets) - rust-lang#137324 (Make x86 QNX target name consistent with other Rust targets) - rust-lang#137338 (skip submodule updating logics on tarballs) - rust-lang#137340 (Add a notice about missing GCC sources into source tarballs) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136985 - zachs18:backend-repr-remove-uninhabited, r=workingjubilee Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) Accepted MCP: rust-lang/compiler-team#832 Fixes rust-lang#135802 Do not consider the inhabitedness of a type for function call ABI purposes. * Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc) * Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it). This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types. cc ``@RalfJung``

rustbot assigned wesleywiser Feb 13, 2025

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 Feb 13, 2025

workingjubilee requested changes Feb 13, 2025

View reviewed changes

compiler/rustc_codegen_llvm/src/intrinsic.rs Outdated Show resolved Hide resolved

zachs18 force-pushed the backend-repr-remove-uninhabited branch from 11153ef to bdc321b Compare February 14, 2025 22:38

workingjubilee self-assigned this Feb 17, 2025

workingjubilee approved these changes Feb 17, 2025

View reviewed changes

compiler/rustc_abi/src/lib.rs Outdated Show resolved Hide resolved

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 Feb 18, 2025

Zalathar mentioned this pull request Feb 18, 2025

Rollup of 8 pull requests #137199

Closed

matthiaskrgr mentioned this pull request Feb 18, 2025

Rollup of 8 pull requests #137208

Closed

RalfJung reviewed Feb 18, 2025

View reviewed changes

matthiaskrgr mentioned this pull request Feb 18, 2025

Rollup of 7 pull requests #137236

Closed

workingjubilee mentioned this pull request Feb 19, 2025

compiler: untangle SIMD alignment assumptions #137256

Open

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 Feb 20, 2025

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 Feb 20, 2025

Remove BackendRepr::Uninhabited, replaced with an `uninhabited: boo…

7ba3d7b

…l` field in `LayoutData`. Also update comments that refered to BackendRepr::Uninhabited.

zachs18 force-pushed the backend-repr-remove-uninhabited branch from ed34181 to 3020eef Compare February 20, 2025 19:34

zachs18 and others added 5 commits February 20, 2025 13:40

Update ui tests with LayoutData { uninhabited: ... } etc

c33fb5a

Update check to reflect that non-ZST uninhabited types should not be …

bcfde13

…PassMode::Ignore.

Adjust LayoutData::uninhabited doc comment.

6493cd8

Co-authored-by: Jubilee <workingjubilee@gmail.com>

fine-tune comment

e3f5db0

zachs18 force-pushed the backend-repr-remove-uninhabited branch from 3020eef to e3f5db0 Compare February 20, 2025 19:41

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 Feb 20, 2025

workingjubilee mentioned this pull request Feb 20, 2025

Rollup of 14 pull requests #137339

Closed

workingjubilee mentioned this pull request Feb 20, 2025

Rollup of 12 pull requests #137346

Merged

bors merged commit 8c9e374 into rust-lang:master Feb 21, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Feb 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) #136985

Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) #136985

zachs18 commented Feb 13, 2025 •

edited by workingjubilee

Loading

rustbot commented Feb 13, 2025

rustbot commented Feb 13, 2025

RalfJung commented Feb 13, 2025

workingjubilee left a comment

rustbot commented Feb 14, 2025

zachs18 commented Feb 14, 2025

workingjubilee commented Feb 17, 2025

workingjubilee left a comment

workingjubilee commented Feb 17, 2025

bors commented Feb 17, 2025

zachs18 commented Feb 18, 2025

bors commented Feb 18, 2025

RalfJung Feb 18, 2025

RalfJung Feb 18, 2025

RalfJung commented Feb 18, 2025

bors commented Feb 18, 2025

bors commented Feb 19, 2025

workingjubilee commented Feb 20, 2025

bors commented Feb 20, 2025

bors commented Feb 20, 2025

workingjubilee commented Feb 20, 2025

bors commented Feb 20, 2025

Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) #136985

Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) #136985

Conversation

zachs18 commented Feb 13, 2025 • edited by workingjubilee Loading

rustbot commented Feb 13, 2025

rustbot commented Feb 13, 2025

RalfJung commented Feb 13, 2025

workingjubilee left a comment

Choose a reason for hiding this comment

rustbot commented Feb 14, 2025

zachs18 commented Feb 14, 2025

workingjubilee commented Feb 17, 2025

workingjubilee left a comment

Choose a reason for hiding this comment

workingjubilee commented Feb 17, 2025

bors commented Feb 17, 2025

zachs18 commented Feb 18, 2025

bors commented Feb 18, 2025

RalfJung Feb 18, 2025

Choose a reason for hiding this comment

RalfJung Feb 18, 2025

Choose a reason for hiding this comment

RalfJung commented Feb 18, 2025

bors commented Feb 18, 2025

bors commented Feb 19, 2025

workingjubilee commented Feb 20, 2025

bors commented Feb 20, 2025

bors commented Feb 20, 2025

workingjubilee commented Feb 20, 2025

bors commented Feb 20, 2025

zachs18 commented Feb 13, 2025 •

edited by workingjubilee

Loading