From ac6cfdef131b236609ad4e9d82a8474303b26d1b Mon Sep 17 00:00:00 2001 From: Florian Sextl Date: Fri, 27 Jun 2025 13:04:16 +0200 Subject: [PATCH 1/3] improve TagEncoding docs --- compiler/rustc_abi/src/lib.rs | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 6d729b6919a78..b38ee52b796c1 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1592,24 +1592,33 @@ pub enum TagEncoding { /// (so converting the tag to the discriminant can require sign extension). Direct, - /// Niche (values invalid for a type) encoding the discriminant: - /// Discriminant and variant index coincide. + /// Niche (values invalid for a type) encoding the discriminant. + /// Note that for this encoding, the discriminant and variant index of each variant coincide! + /// (This gets checked, for example, in [codegen_ssa](https://github.com/rust-lang/rust/blob/df32e15c56f582eb2bdde07711af6271f2ae660b/compiler/rustc_codegen_ssa/src/mir/operand.rs#L485).) + /// /// The variant `untagged_variant` contains a niche at an arbitrary - /// offset (field `tag_field` of the enum), which for a variant with - /// discriminant `d` is set to - /// `(d - niche_variants.start).wrapping_add(niche_start)` - /// (this is wrapping arithmetic using the type of the niche field). - /// - /// For example, `Option<(usize, &T)>` is represented such that - /// `None` has a null pointer for the second tuple field, and - /// `Some` is the identity function (with a non-null reference). + /// offset (field [`Variants::Multiple::tag_field`] of the enum). + /// For a variant with variant index `i`, such that `i!=untagged_variant`, + /// the tag is set to `(i - niche_variants.start).wrapping_add(niche_start)` + /// (this is wrapping arithmetic using the type of the niche field, cf. the + /// [`tag_for_variant`](../rustc_const_eval/interpret/struct.InterpCx.html#method.tag_for_variant) + /// query implementation). + /// To recover the variant index `i` from a `tag`, the above formula has to be reversed, + /// i.e. `i = (tag.wrapping_sub(niche_start))+niche_variants.start`. If `i` ends up outside + /// `niche_variants`, the tag must have encoded the `untagged_variant`. + /// + /// For example, `Option<(usize, &T)>` is represented such that the tag for + /// `None` is the null pointer in the second tuple field, and + /// `Some` is the identity function (with a non-null reference) + /// and has no additional tag, i.e. the reference being non-null uniquely identifies this variant. /// /// Other variants that are not `untagged_variant` and that are outside the `niche_variants` /// range cannot be represented; they must be uninhabited. + /// Nonetheless, uninhabited variants can also fall into the range of `niche_variants`. Niche { untagged_variant: VariantIdx, - /// This range *may* contain `untagged_variant`; that is then just a "dead value" and - /// not used to encode anything. + /// This range *may* contain `untagged_variant` or uninhabited variants; + /// these are then just "dead values" and not used to encode anything. niche_variants: RangeInclusive, /// This is inbounds of the type of the niche field /// (not sign-extended, i.e., all bits beyond the niche field size are 0). From 3d968973c9563fb19ef041145b8e0ef7bb183b85 Mon Sep 17 00:00:00 2001 From: Florian Sextl Date: Fri, 27 Jun 2025 13:04:39 +0200 Subject: [PATCH 2/3] fix docs of FakeBorrowKind --- compiler/rustc_abi/src/lib.rs | 10 +++++----- compiler/rustc_middle/src/mir/syntax.rs | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index b38ee52b796c1..c34e28ad89a9a 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1595,21 +1595,21 @@ pub enum TagEncoding { /// Niche (values invalid for a type) encoding the discriminant. /// Note that for this encoding, the discriminant and variant index of each variant coincide! /// (This gets checked, for example, in [codegen_ssa](https://github.com/rust-lang/rust/blob/df32e15c56f582eb2bdde07711af6271f2ae660b/compiler/rustc_codegen_ssa/src/mir/operand.rs#L485).) - /// + /// /// The variant `untagged_variant` contains a niche at an arbitrary /// offset (field [`Variants::Multiple::tag_field`] of the enum). /// For a variant with variant index `i`, such that `i!=untagged_variant`, /// the tag is set to `(i - niche_variants.start).wrapping_add(niche_start)` - /// (this is wrapping arithmetic using the type of the niche field, cf. the + /// (this is wrapping arithmetic using the type of the niche field, cf. the /// [`tag_for_variant`](../rustc_const_eval/interpret/struct.InterpCx.html#method.tag_for_variant) /// query implementation). /// To recover the variant index `i` from a `tag`, the above formula has to be reversed, /// i.e. `i = (tag.wrapping_sub(niche_start))+niche_variants.start`. If `i` ends up outside /// `niche_variants`, the tag must have encoded the `untagged_variant`. - /// + /// /// For example, `Option<(usize, &T)>` is represented such that the tag for /// `None` is the null pointer in the second tuple field, and - /// `Some` is the identity function (with a non-null reference) + /// `Some` is the identity function (with a non-null reference) /// and has no additional tag, i.e. the reference being non-null uniquely identifies this variant. /// /// Other variants that are not `untagged_variant` and that are outside the `niche_variants` @@ -1617,7 +1617,7 @@ pub enum TagEncoding { /// Nonetheless, uninhabited variants can also fall into the range of `niche_variants`. Niche { untagged_variant: VariantIdx, - /// This range *may* contain `untagged_variant` or uninhabited variants; + /// This range *may* contain `untagged_variant` or uninhabited variants; /// these are then just "dead values" and not used to encode anything. niche_variants: RangeInclusive, /// This is inbounds of the type of the niche field diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 56f19d7929d43..a651583add5bb 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -284,15 +284,15 @@ pub enum FakeBorrowKind { /// /// This is used when lowering deref patterns, where shallow borrows wouldn't prevent something /// like: - // ```compile_fail - // let mut b = Box::new(false); - // match b { - // deref!(true) => {} // not reached because `*b == false` - // _ if { *b = true; false } => {} // not reached because the guard is `false` - // deref!(false) => {} // not reached because the guard changed it - // // UB because we reached the unreachable. - // } - // ``` + /// ```compile_fail + /// let mut b = Box::new(false); + /// match b { + /// deref!(true) => {} // not reached because `*b == false` + /// _ if { *b = true; false } => {} // not reached because the guard is `false` + /// deref!(false) => {} // not reached because the guard changed it + /// // UB because we reached the unreachable. + /// } + /// ``` Deep, } From 1c25bfba9d5bc6e4dfc295e016aac32ff0546f97 Mon Sep 17 00:00:00 2001 From: Florian Sextl Date: Sat, 28 Jun 2025 13:54:44 +0200 Subject: [PATCH 3/3] move discr=varid check to layout_sanity_check --- compiler/rustc_abi/src/lib.rs | 6 +++--- compiler/rustc_codegen_ssa/src/mir/operand.rs | 11 +---------- compiler/rustc_ty_utils/src/layout/invariant.rs | 6 ++++++ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index c34e28ad89a9a..0df8921c9b721 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1594,17 +1594,17 @@ pub enum TagEncoding { /// Niche (values invalid for a type) encoding the discriminant. /// Note that for this encoding, the discriminant and variant index of each variant coincide! - /// (This gets checked, for example, in [codegen_ssa](https://github.com/rust-lang/rust/blob/df32e15c56f582eb2bdde07711af6271f2ae660b/compiler/rustc_codegen_ssa/src/mir/operand.rs#L485).) + /// This invariant is codified as part of [`layout_sanity_check`](../rustc_ty_utils/layout/invariant/fn.layout_sanity_check.html). /// /// The variant `untagged_variant` contains a niche at an arbitrary /// offset (field [`Variants::Multiple::tag_field`] of the enum). - /// For a variant with variant index `i`, such that `i!=untagged_variant`, + /// For a variant with variant index `i`, such that `i != untagged_variant`, /// the tag is set to `(i - niche_variants.start).wrapping_add(niche_start)` /// (this is wrapping arithmetic using the type of the niche field, cf. the /// [`tag_for_variant`](../rustc_const_eval/interpret/struct.InterpCx.html#method.tag_for_variant) /// query implementation). /// To recover the variant index `i` from a `tag`, the above formula has to be reversed, - /// i.e. `i = (tag.wrapping_sub(niche_start))+niche_variants.start`. If `i` ends up outside + /// i.e. `i = tag.wrapping_sub(niche_start) + niche_variants.start`. If `i` ends up outside /// `niche_variants`, the tag must have encoded the `untagged_variant`. /// /// For example, `Option<(usize, &T)>` is represented such that the tag for diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index 99957c6770845..da615cc9a003d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -479,17 +479,8 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { _ => (tag_imm, bx.cx().immediate_backend_type(tag_op.layout)), }; - // Layout ensures that we only get here for cases where the discriminant + // `layout_sanity_check` ensures that we only get here for cases where the discriminant // value and the variant index match, since that's all `Niche` can encode. - // But for emphasis and debugging, let's double-check one anyway. - debug_assert_eq!( - self.layout - .ty - .discriminant_for_variant(bx.tcx(), untagged_variant) - .unwrap() - .val, - u128::from(untagged_variant.as_u32()), - ); let relative_max = niche_variants.end().as_u32() - niche_variants.start().as_u32(); diff --git a/compiler/rustc_ty_utils/src/layout/invariant.rs b/compiler/rustc_ty_utils/src/layout/invariant.rs index c929de1162416..4b65c05d0e9f9 100644 --- a/compiler/rustc_ty_utils/src/layout/invariant.rs +++ b/compiler/rustc_ty_utils/src/layout/invariant.rs @@ -277,6 +277,12 @@ pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayou if !variant.is_uninhabited() { assert!(idx == *untagged_variant || niche_variants.contains(&idx)); } + + // Ensure that for niche encoded tags the discriminant coincides with the variant index. + assert_eq!( + layout.ty.discriminant_for_variant(tcx, idx).unwrap().val, + u128::from(idx.as_u32()), + ); } } for variant in variants.iter() {