safe transmute: support non-ZST, variantful, uninhabited enums by jswrenn · Pull Request #126493 · rust-lang/rust (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

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

Jun 14, 2024

Comment on lines +244 to +253

Previously, Tree::from_enum's implementation branched into three disjoint cases:

1. enums that uninhabited
2. enums for which all but one variant is uninhabited
3. enums with multiple inhabited variants

This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like:

enum Uninhabited { A(!, u128) }

...which, currently, has the same size as u128. This faulty assumption manifested as the ICE reported in rust-lang#126460.

In this PR, we revise the first case of Tree::from_enum to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with Variants::Single { index }, are special in their layouts otherwise resemble the ! type and cannot be descended into like typical enums. This first case captures uninhabited enums like:

enum Uninhabited { A(!, !), B(!) }

The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with Variants::Single { index } and that do have a variant at index. This second case captures uninhabited enums that are not ZSTs, like:

enum Uninhabited { A(!, u128) }

...which represent their variants with Variants::Single.

Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited, non-ZST enums like:

enum Uninhabited { A(u8, !), B(!, u32) }

...which represent their variants with Variants::Multiple.

This PR also adds a comment requested by RalfJung in his review of rust-lang#126358 to compiler/rustc_const_eval/src/interpret/discriminant.rs.

Fixes rust-lang#126460

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

Jun 18, 2024

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Jun 18, 2024

safe transmute: support non-ZST, variantful, uninhabited enums

Previously, Tree::from_enum's implementation branched into three disjoint cases:

1. enums that uninhabited
2. enums for which all but one variant is uninhabited
3. enums with multiple variants

This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like:

enum Uninhabited { A(!, u128) }

...which, currently, has the same size as u128. This faulty assumption manifested as the ICE reported in rust-lang#126460.

In this PR, we revise the first case of Tree::from_enum to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with Variants::Single { index }, are special in their layouts otherwise resemble the ! type and cannot be descended into like typical enums. This first case captures uninhabited enums like:

enum Uninhabited { A(!, !), B(!) }

The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with Variants::Single { index } and that do have a variant at index. This second case captures uninhabited enums that are not ZSTs, like:

enum Uninhabited { A(!, u128) }

...which represent their variants with Variants::Single.

Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like:

enum Uninhabited { A(u8, !), B(!, u32) }

...which represent their variants with Variants::Multiple.

This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to compiler/rustc_const_eval/src/interpret/discriminant.rs.

Fixes rust-lang#126460

r? @compiler-errors

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Jun 18, 2024

safe transmute: support non-ZST, variantful, uninhabited enums

Previously, Tree::from_enum's implementation branched into three disjoint cases:

1. enums that uninhabited
2. enums for which all but one variant is uninhabited
3. enums with multiple variants

This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like:

enum Uninhabited { A(!, u128) }

...which, currently, has the same size as u128. This faulty assumption manifested as the ICE reported in rust-lang#126460.

In this PR, we revise the first case of Tree::from_enum to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with Variants::Single { index }, are special in their layouts otherwise resemble the ! type and cannot be descended into like typical enums. This first case captures uninhabited enums like:

enum Uninhabited { A(!, !), B(!) }

The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with Variants::Single { index } and that do have a variant at index. This second case captures uninhabited enums that are not ZSTs, like:

enum Uninhabited { A(!, u128) }

...which represent their variants with Variants::Single.

Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like:

enum Uninhabited { A(u8, !), B(!, u32) }

...which represent their variants with Variants::Multiple.

This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to compiler/rustc_const_eval/src/interpret/discriminant.rs.

Fixes rust-lang#126460

r? @compiler-errors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 18, 2024

…llaumeGomez

Rollup of 9 pull requests

Successful merges:

• rust-lang#123782 (Test that opaque types can't have themselves as a hidden type with incompatible lifetimes)
• rust-lang#124580 (Suggest removing unused tuple fields if they are the last fields)
• rust-lang#125852 (improve tip for inaccessible traits)
• rust-lang#126422 (Suggest using a standalone doctest for non-local impl defs)
• rust-lang#126427 (Rewrite intrinsic-unreachable, sepcomp-cci-copies, sepcomp-inlining and sepcomp-separate run-make tests to rmake.rs)
• rust-lang#126493 (safe transmute: support non-ZST, variantful, uninhabited enums)
• rust-lang#126572 (override user defined channel when using precompiled rustc)
• rust-lang#126615 (Add rustc-ice* to .gitignore)
• rust-lang#126632 (Replace move|| with move ||)

Failed merges:

• rust-lang#126558 (hir_typeck: be more conservative in making "note caller chooses ty param" note)

r? @ghost @rustbot modify labels: rollup

jieyouxu added a commit to jieyouxu/rust that referenced this pull request

Jun 19, 2024

safe transmute: support non-ZST, variantful, uninhabited enums

Previously, Tree::from_enum's implementation branched into three disjoint cases:

1. enums that uninhabited
2. enums for which all but one variant is uninhabited
3. enums with multiple variants

This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like:

enum Uninhabited { A(!, u128) }

...which, currently, has the same size as u128. This faulty assumption manifested as the ICE reported in rust-lang#126460.

In this PR, we revise the first case of Tree::from_enum to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with Variants::Single { index }, are special in their layouts otherwise resemble the ! type and cannot be descended into like typical enums. This first case captures uninhabited enums like:

enum Uninhabited { A(!, !), B(!) }

The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with Variants::Single { index } and that do have a variant at index. This second case captures uninhabited enums that are not ZSTs, like:

enum Uninhabited { A(!, u128) }

...which represent their variants with Variants::Single.

Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like:

enum Uninhabited { A(u8, !), B(!, u32) }

...which represent their variants with Variants::Multiple.

This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to compiler/rustc_const_eval/src/interpret/discriminant.rs.

Fixes rust-lang#126460

r? @compiler-errors

This was referenced

Jun 19, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jun 19, 2024

Rollup of 10 pull requests

Successful merges:

• rust-lang#124135 (delegation: Implement glob delegation)
• rust-lang#125078 (fix: break inside async closure has incorrect span for enclosing closure)
• rust-lang#125293 (Place tail expression behind terminating scope)
• rust-lang#126422 (Suggest using a standalone doctest for non-local impl defs)
• rust-lang#126493 (safe transmute: support non-ZST, variantful, uninhabited enums)
• rust-lang#126504 (Sync fuchsia test runner with clang test runner)
• rust-lang#126558 (hir_typeck: be more conservative in making "note caller chooses ty param" note)
• rust-lang#126586 (Add @badboy and @BlackHoleFox as Mac Catalyst maintainers)
• rust-lang#126615 (Add rustc-ice* to .gitignore)
• rust-lang#126632 (Replace move|| with move ||)

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Jun 19, 2024

Rollup merge of rust-lang#126493 - jswrenn:fix-126460, r=compiler-errors

safe transmute: support non-ZST, variantful, uninhabited enums

Previously, Tree::from_enum's implementation branched into three disjoint cases:

1. enums that uninhabited
2. enums for which all but one variant is uninhabited
3. enums with multiple variants

This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like:

enum Uninhabited { A(!, u128) }

...which, currently, has the same size as u128. This faulty assumption manifested as the ICE reported in rust-lang#126460.

In this PR, we revise the first case of Tree::from_enum to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with Variants::Single { index }, are special in their layouts otherwise resemble the ! type and cannot be descended into like typical enums. This first case captures uninhabited enums like:

enum Uninhabited { A(!, !), B(!) }

The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with Variants::Single { index } and that do have a variant at index. This second case captures uninhabited enums that are not ZSTs, like:

enum Uninhabited { A(!, u128) }

...which represent their variants with Variants::Single.

Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like:

enum Uninhabited { A(u8, !), B(!, u32) }

...which represent their variants with Variants::Multiple.

This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to compiler/rustc_const_eval/src/interpret/discriminant.rs.

Fixes rust-lang#126460

r? @compiler-errors