Auto merge of rust-lang#98393 - michaelwoerister:new-cpp-like-enum-debuginfo, r=wesleywiser

debuginfo: Generalize C++-like encoding for enums.

The updated encoding should be able to handle niche layouts where more than one variant has fields (as introduced in rust-lang#94075).

The new encoding is more uniform as there is no structural difference between direct-tag, niche-tag, and no-tag layouts anymore. The only difference between those cases is that the "dataful" variant in a niche-tag enum will have a `(start, end)` pair denoting the tag range instead of a single value.

The new encoding now also supports 128-bit tags, which occur in at least some standard library types. These tags are represented as `u64` pairs so that debuggers (which don't always have support for 128-bit integers) can reliably deal with them. The downside is that this adds quite a bit of complexity to the encoding and especially to the corresponding NatVis.

The new encoding seems to increase the size of (x86_64-pc-windows-msvc) debuginfo by 10-15%. The size of binaries is not affected (release builds were built with `-Cdebuginfo=2`, numbers are in kilobytes):

EXE | before | after | relative
-- | -- | -- | --
cargo (debug) | 40453 | 40450 | +0%
ripgrep (debug) | 10275 | 10273 | +0%
cargo (release) | 16186 | 16185 | +0%
ripgrep (release) | 4727 | 4726 | +0%

PDB | before | after | relative
-- | -- | -- | --
cargo (debug) | 236524 | 261412 | +11%
ripgrep (debug) | 53140 | 59060 | +11%
cargo (release) | 148516 | 169620 | +14%
ripgrep (release) | 10676 | 11804 | +11%

Given that the new encoding is more general, this is to be expected. Only platforms using C++-like debuginfo are affected -- which currently is only `*-pc-windows-msvc`.

*TODO*
- [x] Properly update documentation
- [x] Add regression tests for new optimized enum layouts as introduced by rust-lang#94075.

r? `@wesleywiser`

Author: bors

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -114,6 +114,7 @@ macro_rules! return_if_di_node_created_in_meantime {
 }
 
 /// Extract size and alignment from a TyAndLayout.
+#[inline]
 fn size_and_align_of<'tcx>(ty_and_layout: TyAndLayout<'tcx>) -> (Size, Align) {
     (ty_and_layout.size, ty_and_layout.align.abi)
 }

compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs

@@ -10,7 +10,6 @@ use rustc_middle::{
     ty::{
         self,
         layout::{IntegerExt, LayoutOf, PrimitiveExt, TyAndLayout},
-        util::Discr,
         AdtDef, GeneratorSubsts, Ty, VariantDef,
     },
 };
@@ -90,8 +89,11 @@ fn build_c_style_enum_di_node<'ll, 'tcx>(
             cx,
             &compute_debuginfo_type_name(cx.tcx, enum_type_and_layout.ty, false),
             tag_base_type(cx, enum_type_and_layout),
-            &mut enum_adt_def.discriminants(cx.tcx).map(|(variant_index, discr)| {
-                (discr, Cow::from(enum_adt_def.variant(variant_index).name.as_str()))
+            enum_adt_def.discriminants(cx.tcx).map(|(variant_index, discr)| {
+                let name = Cow::from(enum_adt_def.variant(variant_index).name.as_str());
+                // Is there anything we can do to support 128-bit C-Style enums?
+                let value = discr.val as u64;
+                (name, value)
             }),
             containing_scope,
         ),
@@ -152,7 +154,7 @@ fn build_enumeration_type_di_node<'ll, 'tcx>(
     cx: &CodegenCx<'ll, 'tcx>,
     type_name: &str,
     base_type: Ty<'tcx>,
-    variants: &mut dyn Iterator<Item = (Discr<'tcx>, Cow<'tcx, str>)>,
+    enumerators: impl Iterator<Item = (Cow<'tcx, str>, u64)>,
     containing_scope: &'ll DIType,
 ) -> &'ll DIType {
     let is_unsigned = match base_type.kind() {
@@ -161,18 +163,15 @@ fn build_enumeration_type_di_node<'ll, 'tcx>(
         _ => bug!("build_enumeration_type_di_node() called with non-integer tag type."),
     };
 
-    let enumerator_di_nodes: SmallVec<Option<&'ll DIType>> = variants
-        .map(|(discr, variant_name)| {
-            unsafe {
-                Some(llvm::LLVMRustDIBuilderCreateEnumerator(
-                    DIB(cx),
-                    variant_name.as_ptr().cast(),
-                    variant_name.len(),
-                    // FIXME: what if enumeration has i128 discriminant?
-                    discr.val as i64,
-                    is_unsigned,
-                ))
-            }
+    let enumerator_di_nodes: SmallVec<Option<&'ll DIType>> = enumerators
+        .map(|(name, value)| unsafe {
+            Some(llvm::LLVMRustDIBuilderCreateEnumerator(
+                DIB(cx),
+                name.as_ptr().cast(),
+                name.len(),
+                value as i64,
+                is_unsigned,
+            ))
         })
         .collect();
 
@@ -247,23 +246,27 @@ fn build_enumeration_type_di_node<'ll, 'tcx>(
 /// and a DW_TAG_member for each field (but not the discriminant).
 fn build_enum_variant_struct_type_di_node<'ll, 'tcx>(
     cx: &CodegenCx<'ll, 'tcx>,
-    enum_type: Ty<'tcx>,
+    enum_type_and_layout: TyAndLayout<'tcx>,
     enum_type_di_node: &'ll DIType,
     variant_index: VariantIdx,
     variant_def: &VariantDef,
     variant_layout: TyAndLayout<'tcx>,
 ) -> &'ll DIType {
-    debug_assert_eq!(variant_layout.ty, enum_type);
+    debug_assert_eq!(variant_layout.ty, enum_type_and_layout.ty);
 
     type_map::build_type_with_children(
         cx,
         type_map::stub(
             cx,
             Stub::Struct,
-            UniqueTypeId::for_enum_variant_struct_type(cx.tcx, enum_type, variant_index),
+            UniqueTypeId::for_enum_variant_struct_type(
+                cx.tcx,
+                enum_type_and_layout.ty,
+                variant_index,
+            ),
             variant_def.name.as_str(),
             // NOTE: We use size and align of enum_type, not from variant_layout:
-            cx.size_and_align_of(enum_type),
+            size_and_align_of(enum_type_and_layout),
             Some(enum_type_di_node),
             DIFlags::FlagZero,
         ),
@@ -290,9 +293,9 @@ fn build_enum_variant_struct_type_di_node<'ll, 'tcx>(
                         type_di_node(cx, field_layout.ty),
                     )
                 })
-                .collect()
+                .collect::<SmallVec<_>>()
         },
-        |cx| build_generic_type_param_di_nodes(cx, enum_type),
+        |cx| build_generic_type_param_di_nodes(cx, enum_type_and_layout.ty),
     )
     .di_node
 }
@@ -398,6 +401,19 @@ pub fn build_generator_variant_struct_type_di_node<'ll, 'tcx>(
     .di_node
 }
 
+#[derive(Copy, Clone)]
+enum DiscrResult {
+    NoDiscriminant,
+    Value(u128),
+    Range(u128, u128),
+}
+
+impl DiscrResult {
+    fn opt_single_val(&self) -> Option<u128> {
+        if let Self::Value(d) = *self { Some(d) } else { None }
+    }
+}
+
 /// Returns the discriminant value corresponding to the variant index.
 ///
 /// Will return `None` if there is less than two variants (because then the enum won't have)
@@ -407,30 +423,38 @@ fn compute_discriminant_value<'ll, 'tcx>(
     cx: &CodegenCx<'ll, 'tcx>,
     enum_type_and_layout: TyAndLayout<'tcx>,
     variant_index: VariantIdx,
-) -> Option<u64> {
+) -> DiscrResult {
     match enum_type_and_layout.layout.variants() {
-        &Variants::Single { .. } => None,
-        &Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => Some(
-            enum_type_and_layout.ty.discriminant_for_variant(cx.tcx, variant_index).unwrap().val
-                as u64,
+        &Variants::Single { .. } => DiscrResult::NoDiscriminant,
+        &Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => DiscrResult::Value(
+            enum_type_and_layout.ty.discriminant_for_variant(cx.tcx, variant_index).unwrap().val,
         ),
         &Variants::Multiple {
             tag_encoding: TagEncoding::Niche { ref niche_variants, niche_start, dataful_variant },
             tag,
             ..
         } => {
             if variant_index == dataful_variant {
-                None
+                let valid_range = enum_type_and_layout
+                    .for_variant(cx, variant_index)
+                    .largest_niche
+                    .as_ref()
+                    .unwrap()
+                    .valid_range;
+
+                let min = valid_range.start.min(valid_range.end);
+                let min = tag.size(cx).truncate(min);
+
+                let max = valid_range.start.max(valid_range.end);
+                let max = tag.size(cx).truncate(max);
+
+                DiscrResult::Range(min, max)
             } else {
                 let value = (variant_index.as_u32() as u128)
                     .wrapping_sub(niche_variants.start().as_u32() as u128)
                     .wrapping_add(niche_start);
                 let value = tag.size(cx).truncate(value);
-                // NOTE(eddyb) do *NOT* remove this assert, until
-                // we pass the full 128-bit value to LLVM, otherwise
-                // truncation will be silent and remain undetected.
-                assert_eq!(value as u64 as u128, value);
-                Some(value as u64)
+                DiscrResult::Value(value)
             }
         }
     }

compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs

@@ -88,7 +88,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
                     variant_name: Cow::from(enum_adt_def.variant(variant_index).name.as_str()),
                     variant_struct_type_di_node: super::build_enum_variant_struct_type_di_node(
                         cx,
-                        enum_type,
+                        enum_type_and_layout,
                         enum_type_di_node,
                         variant_index,
                         enum_adt_def.variant(variant_index),
@@ -413,7 +413,13 @@ fn build_enum_variant_member_di_node<'ll, 'tcx>(
             enum_type_and_layout.size.bits(),
             enum_type_and_layout.align.abi.bits() as u32,
             Size::ZERO.bits(),
-            discr_value.map(|v| cx.const_u64(v)),
+            discr_value.opt_single_val().map(|value| {
+                // NOTE(eddyb) do *NOT* remove this assert, until
+                // we pass the full 128-bit value to LLVM, otherwise
+                // truncation will be silent and remain undetected.
+                assert_eq!(value as u64 as u128, value);
+                cx.const_u64(value as u64)
+            }),
             DIFlags::FlagZero,
             variant_member_info.variant_struct_type_di_node,
         )

compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs

@@ -47,6 +47,8 @@ pub(super) enum UniqueTypeId<'tcx> {
     VariantPart(Ty<'tcx>, private::HiddenZst),
     /// The ID for the artificial struct type describing a single enum variant.
     VariantStructType(Ty<'tcx>, VariantIdx, private::HiddenZst),
+    /// The ID for the additional wrapper struct type describing an enum variant in CPP-like mode.
+    VariantStructTypeCppLikeWrapper(Ty<'tcx>, VariantIdx, private::HiddenZst),
     /// The ID of the artificial type we create for VTables.
     VTableTy(Ty<'tcx>, Option<PolyExistentialTraitRef<'tcx>>, private::HiddenZst),
 }
@@ -71,6 +73,15 @@ impl<'tcx> UniqueTypeId<'tcx> {
         UniqueTypeId::VariantStructType(enum_ty, variant_idx, private::HiddenZst)
     }
 
+    pub fn for_enum_variant_struct_type_wrapper(
+        tcx: TyCtxt<'tcx>,
+        enum_ty: Ty<'tcx>,
+        variant_idx: VariantIdx,
+    ) -> Self {
+        debug_assert_eq!(enum_ty, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), enum_ty));
+        UniqueTypeId::VariantStructTypeCppLikeWrapper(enum_ty, variant_idx, private::HiddenZst)
+    }
+
     pub fn for_vtable_ty(
         tcx: TyCtxt<'tcx>,
         self_type: Ty<'tcx>,

compiler/rustc_codegen_llvm/src/debuginfo/metadata/type_map.rs

@@ -2079,6 +2079,19 @@ extern "C" {
         Ty: &'a DIType,
     ) -> &'a DIType;
 
+    pub fn LLVMRustDIBuilderCreateStaticMemberType<'a>(
+        Builder: &DIBuilder<'a>,
+        Scope: &'a DIDescriptor,
+        Name: *const c_char,
+        NameLen: size_t,
+        File: &'a DIFile,
+        LineNo: c_uint,
+        Ty: &'a DIType,
+        Flags: DIFlags,
+        val: Option<&'a Value>,
+        AlignInBits: u32,
+    ) -> &'a DIDerivedType;
+
     pub fn LLVMRustDIBuilderCreateLexicalBlock<'a>(
         Builder: &DIBuilder<'a>,
         Scope: &'a DIScope,

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -18,11 +18,10 @@ use rustc_hir::definitions::{DefPathData, DefPathDataName, DisambiguatedDefPathD
 use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Mutability};
 use rustc_middle::ty::layout::{IntegerExt, TyAndLayout};
 use rustc_middle::ty::subst::{GenericArgKind, SubstsRef};
-use rustc_middle::ty::{self, ExistentialProjection, GeneratorSubsts, ParamEnv, Ty, TyCtxt};
-use rustc_target::abi::{Integer, TagEncoding, Variants};
+use rustc_middle::ty::{self, ExistentialProjection, ParamEnv, Ty, TyCtxt};
+use rustc_target::abi::Integer;
 use smallvec::SmallVec;
 
-use std::borrow::Cow;
 use std::fmt::Write;
 
 use crate::debuginfo::wants_c_like_enum_debuginfo;
@@ -98,7 +97,6 @@ fn push_debuginfo_type_name<'tcx>(
 
             if let Some(ty_and_layout) = layout_for_cpp_like_fallback {
                 msvc_enum_fallback(
-                    tcx,
                     ty_and_layout,
                     &|output, visited| {
                         push_item_name(tcx, def.did(), true, output);
@@ -391,11 +389,10 @@ fn push_debuginfo_type_name<'tcx>(
             // Name will be "{closure_env#0}<T1, T2, ...>", "{generator_env#0}<T1, T2, ...>", or
             // "{async_fn_env#0}<T1, T2, ...>", etc.
             // In the case of cpp-like debuginfo, the name additionally gets wrapped inside of
-            // an artificial `enum$<>` type, as defined in msvc_enum_fallback().
+            // an artificial `enum2$<>` type, as defined in msvc_enum_fallback().
             if cpp_like_debuginfo && t.is_generator() {
                 let ty_and_layout = tcx.layout_of(ParamEnv::reveal_all().and(t)).unwrap();
                 msvc_enum_fallback(
-                    tcx,
                     ty_and_layout,
                     &|output, visited| {
                         push_closure_or_generator_name(tcx, def_id, substs, true, output, visited);
@@ -428,58 +425,17 @@ fn push_debuginfo_type_name<'tcx>(
 
     /// MSVC names enums differently than other platforms so that the debugging visualization
     // format (natvis) is able to understand enums and render the active variant correctly in the
-    // debugger. For more information, look in `src/etc/natvis/intrinsic.natvis` and
-    // `EnumMemberDescriptionFactor::create_member_descriptions`.
+    // debugger. For more information, look in
+    // rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs.
     fn msvc_enum_fallback<'tcx>(
-        tcx: TyCtxt<'tcx>,
         ty_and_layout: TyAndLayout<'tcx>,
         push_inner: &dyn Fn(/*output*/ &mut String, /*visited*/ &mut FxHashSet<Ty<'tcx>>),
         output: &mut String,
         visited: &mut FxHashSet<Ty<'tcx>>,
     ) {
         debug_assert!(!wants_c_like_enum_debuginfo(ty_and_layout));
-        let ty = ty_and_layout.ty;
-
-        output.push_str("enum$<");
+        output.push_str("enum2$<");
         push_inner(output, visited);
-
-        let variant_name = |variant_index| match ty.kind() {
-            ty::Adt(adt_def, _) => {
-                debug_assert!(adt_def.is_enum());
-                Cow::from(adt_def.variant(variant_index).name.as_str())
-            }
-            ty::Generator(..) => GeneratorSubsts::variant_name(variant_index),
-            _ => unreachable!(),
-        };
-
-        if let Variants::Multiple {
-            tag_encoding: TagEncoding::Niche { dataful_variant, .. },
-            tag,
-            variants,
-            ..
-        } = &ty_and_layout.variants
-        {
-            let dataful_variant_layout = &variants[*dataful_variant];
-
-            // calculate the range of values for the dataful variant
-            let dataful_discriminant_range =
-                dataful_variant_layout.largest_niche().unwrap().valid_range;
-
-            let min = dataful_discriminant_range.start;
-            let min = tag.size(&tcx).truncate(min);
-
-            let max = dataful_discriminant_range.end;
-            let max = tag.size(&tcx).truncate(max);
-
-            let dataful_variant_name = variant_name(*dataful_variant);
-            write!(output, ", {}, {}, {}", min, max, dataful_variant_name).unwrap();
-        } else if let Variants::Single { index: variant_idx } = &ty_and_layout.variants {
-            // Uninhabited enums can't be constructed and should never need to be visualized so
-            // skip this step for them.
-            if !ty_and_layout.abi.is_uninhabited() {
-                write!(output, ", {}", variant_name(*variant_idx)).unwrap();
-            }
-        }
         push_close_angle_bracket(true, output);
     }
 

compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs

@@ -172,7 +172,9 @@ impl<I: Idx, T> IndexVec<I, T> {
     }
 
     #[inline]
-    pub fn indices(&self) -> impl DoubleEndedIterator<Item = I> + ExactSizeIterator + 'static {
+    pub fn indices(
+        &self,
+    ) -> impl DoubleEndedIterator<Item = I> + ExactSizeIterator + Clone + 'static {
         (0..self.len()).map(|n| I::new(n))
     }
 

compiler/rustc_index/src/vec.rs

@@ -924,6 +924,30 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateVariantMemberType(
                                                fromRust(Flags), unwrapDI<DIType>(Ty)));
 }
 
+extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateStaticMemberType(
+    LLVMRustDIBuilderRef Builder,
+    LLVMMetadataRef Scope,
+    const char *Name,
+    size_t NameLen,
+    LLVMMetadataRef File,
+    unsigned LineNo,
+    LLVMMetadataRef Ty,
+    LLVMRustDIFlags Flags,
+    LLVMValueRef val,
+    uint32_t AlignInBits
+) {
+  return wrap(Builder->createStaticMemberType(
+    unwrapDI<DIDescriptor>(Scope),
+    StringRef(Name, NameLen),
+    unwrapDI<DIFile>(File),
+    LineNo,
+    unwrapDI<DIType>(Ty),
+    fromRust(Flags),
+    unwrap<llvm::ConstantInt>(val),
+    AlignInBits
+  ));
+}
+
 extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateLexicalBlock(
     LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope,
     LLVMMetadataRef File, unsigned Line, unsigned Col) {