Generate better debuginfo for niche-layout enums

Previously, we would generate a single struct with the layout of the
dataful variant plus an extra field whose name contained the value of
the niche (this would only really work for things like `Option<&_>`
where we can determine that the `None` case maps to `0` but for enums
that have multiple tag only variants, this doesn't work).

Now, we generate a union of two structs, one which is the layout of the
dataful variant and one which just has a way of reading the
discriminant. We also generate an enum which maps the discriminant value
to the tag only variants.

We also encode information about the range of values which correspond to
the dataful variant in the type name and then use natvis to determine
which union field we should display to the user.

As a result of this change, all niche-layout enums render correctly in
WinDbg and Visual Studio!

Author: wesleywiser

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -1594,77 +1594,144 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                 ref variants,
                 tag_field,
             } => {
+                let calculate_niche_value = |i: VariantIdx| {
+                    if i == dataful_variant {
+                        None
+                    } else {
+                        let value = (i.as_u32() as u128)
+                            .wrapping_sub(niche_variants.start().as_u32() as u128)
+                            .wrapping_add(niche_start);
+                        let value = tag.value.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)
+                    }
+                };
+
+                // For MSVC, we will generate a union of two structs, one for the dataful variant and one that just points to
+                // the discriminant field. We also create an enum that contains tag values for the non-dataful variants and
+                // make the discriminant field that type. We then use natvis to render the enum type correctly in Windbg/VS.
                 if fallback {
-                    let variant = self.layout.for_variant(cx, dataful_variant);
-                    // Create a description of the non-null variant.
-                    let (variant_type_metadata, member_description_factory) = describe_enum_variant(
+                    let unique_type_id = debug_context(cx)
+                        .type_map
+                        .borrow_mut()
+                        .get_unique_type_id_of_enum_variant(cx, self.enum_type, "discriminant$");
+
+                    let variant_metadata = create_struct_stub(
+                        cx,
+                        self.layout.ty,
+                        &"discriminant$",
+                        unique_type_id,
+                        Some(self_metadata),
+                        DIFlags::FlagArtificial,
+                    );
+
+                    let dataful_variant_layout = self.layout.for_variant(cx, dataful_variant);
+
+                    let mut discr_enum_ty = tag.value.to_ty(cx.tcx);
+                    // If the niche is the NULL value of a reference, then `discr_enum_ty` will be a RawPtr.
+                    // CodeView doesn't know what to do with enums whose base type is a pointer so we fix this up
+                    // to just be `usize`.
+                    if let ty::RawPtr(_) = discr_enum_ty.kind() {
+                        discr_enum_ty = cx.tcx.types.usize;
+                    }
+
+                    let tags: Vec<_> = variants
+                        .iter_enumerated()
+                        .filter_map(|(variant_idx, _)| {
+                            calculate_niche_value(variant_idx).map(|tag| {
+                                let variant = variant_info_for(variant_idx);
+                                let name = variant.variant_name();
+
+                                Some(unsafe {
+                                    llvm::LLVMRustDIBuilderCreateEnumerator(
+                                        DIB(cx),
+                                        name.as_ptr().cast(),
+                                        name.len(),
+                                        tag as i64,
+                                        !discr_enum_ty.is_signed(),
+                                    )
+                                })
+                            })
+                        })
+                        .collect();
+
+                    let discr_enum = unsafe {
+                        llvm::LLVMRustDIBuilderCreateEnumerationType(
+                            DIB(cx),
+                            self_metadata,
+                            "tag$".as_ptr().cast(),
+                            "tag$".len(),
+                            unknown_file_metadata(cx),
+                            UNKNOWN_LINE_NUMBER,
+                            tag.value.size(cx).bits(),
+                            tag.value.align(cx).abi.bits() as u32,
+                            create_DIArray(DIB(cx), &tags),
+                            type_metadata(cx, discr_enum_ty, self.span),
+                            true,
+                        )
+                    };
+
+                    let (size, align) =
+                        cx.size_and_align_of(dataful_variant_layout.field(cx, tag_field).ty);
+                    let members = vec![MemberDescription {
+                        name: "discriminant".to_string(),
+                        type_metadata: discr_enum,
+                        offset: dataful_variant_layout.fields.offset(tag_field),
+                        size,
+                        align,
+                        flags: DIFlags::FlagArtificial,
+                        discriminant: None,
+                        source_info: None,
+                    }];
+
+                    set_members_of_composite_type(cx, self.enum_type, variant_metadata, members, None);
+
+                    let variant_info = variant_info_for(dataful_variant);
+                    let (variant_type_metadata, member_desc_factory) = describe_enum_variant(
                         cx,
-                        variant,
-                        variant_info_for(dataful_variant),
+                        dataful_variant_layout,
+                        variant_info,
                         Some(NicheTag),
                         self_metadata,
                         self.span,
                     );
 
-                    let variant_member_descriptions =
-                        member_description_factory.create_member_descriptions(cx);
+                    let member_descriptions = member_desc_factory.create_member_descriptions(cx);
 
                     set_members_of_composite_type(
                         cx,
                         self.enum_type,
                         variant_type_metadata,
-                        variant_member_descriptions,
+                        member_descriptions,
                         Some(&self.common_members),
                     );
 
-                    // Encode the information about the null variant in the union
-                    // member's name.
-                    let mut name = String::from("RUST$ENCODED$ENUM$");
-                    // Right now it's not even going to work for `niche_start > 0`,
-                    // and for multiple niche variants it only supports the first.
-                    fn compute_field_path<'a, 'tcx>(
-                        cx: &CodegenCx<'a, 'tcx>,
-                        name: &mut String,
-                        layout: TyAndLayout<'tcx>,
-                        offset: Size,
-                        size: Size,
-                    ) {
-                        for i in 0..layout.fields.count() {
-                            let field_offset = layout.fields.offset(i);
-                            if field_offset > offset {
-                                continue;
-                            }
-                            let inner_offset = offset - field_offset;
-                            let field = layout.field(cx, i);
-                            if inner_offset + size <= field.size {
-                                write!(name, "{}$", i).unwrap();
-                                compute_field_path(cx, name, field, inner_offset, size);
-                            }
-                        }
-                    }
-                    compute_field_path(
-                        cx,
-                        &mut name,
-                        self.layout,
-                        self.layout.fields.offset(tag_field),
-                        self.layout.field(cx, tag_field).size,
-                    );
-                    let variant_info = variant_info_for(*niche_variants.start());
-                    variant_info.map_struct_name(|variant_name| {
-                        name.push_str(variant_name);
-                    });
-
-                    // Create the (singleton) list of descriptions of union members.
-                    vec![MemberDescription {
-                        name,
-                        type_metadata: variant_type_metadata,
-                        offset: Size::ZERO,
-                        size: variant.size,
-                        align: variant.align.abi,
-                        flags: DIFlags::FlagZero,
-                        discriminant: None,
-                        source_info: variant_info.source_info(cx),
-                    }]
+                    vec![
+                        MemberDescription {
+                            // Name the dataful variant so that we can identify it for natvis
+                            name: "dataful_variant".to_string(),
+                            type_metadata: variant_type_metadata,
+                            offset: Size::ZERO,
+                            size: self.layout.size,
+                            align: self.layout.align.abi,
+                            flags: DIFlags::FlagZero,
+                            discriminant: None,
+                            source_info: variant_info.source_info(cx),
+                        },
+                        MemberDescription {
+                            name: "discriminant$".into(),
+                            type_metadata: variant_metadata,
+                            offset: Size::ZERO,
+                            size: self.layout.size,
+                            align: self.layout.align.abi,
+                            flags: DIFlags::FlagZero,
+                            discriminant: None,
+                            source_info: None,
+                        },
+                    ]
                 } else {
                     variants
                         .iter_enumerated()
@@ -1692,19 +1759,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                                 Some(&self.common_members),
                             );
 
-                            let niche_value = if i == dataful_variant {
-                                None
-                            } else {
-                                let value = (i.as_u32() as u128)
-                                    .wrapping_sub(niche_variants.start().as_u32() as u128)
-                                    .wrapping_add(niche_start);
-                                let value = tag.value.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)
-                            };
+                            let niche_value = calculate_niche_value(i);
 
                             MemberDescription {
                                 name: variant_info.variant_name(),
@@ -2040,9 +2095,9 @@ fn prepare_enum_metadata(
 
     if use_enum_fallback(cx) {
         let discriminant_type_metadata = match layout.variants {
-            Variants::Single { .. }
-            | Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, .. } => None,
-            Variants::Multiple { tag_encoding: TagEncoding::Direct, ref tag, .. } => {
+            Variants::Single { .. } => None,
+            Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, ref tag, .. }
+            | Variants::Multiple { tag_encoding: TagEncoding::Direct, ref tag, .. } => {
                 Some(discriminant_type_metadata(tag.value))
             }
         };

compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs

@@ -3,7 +3,8 @@
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
-use rustc_middle::ty::{self, subst::SubstsRef, Ty, TyCtxt};
+use rustc_middle::ty::{self, subst::SubstsRef, AdtDef, Ty, TyCtxt};
+use rustc_target::abi::{TagEncoding, Variants};
 
 use std::fmt::Write;
 
@@ -46,14 +47,10 @@ pub fn push_debuginfo_type_name<'tcx>(
         ty::Foreign(def_id) => push_item_name(tcx, def_id, qualified, output),
         ty::Adt(def, substs) => {
             if def.is_enum() && cpp_like_names {
-                output.push_str("_enum<");
-            }
-
-            push_item_name(tcx, def.did, qualified, output);
-            push_type_params(tcx, substs, output, visited);
-
-            if def.is_enum() && cpp_like_names {
-                output.push('>');
+                msvc_enum_fallback(tcx, t, def, substs, output, visited);
+            } else {
+                push_item_name(tcx, def.did, qualified, output);
+                push_type_params(tcx, substs, output, visited);
             }
         }
         ty::Tuple(component_types) => {
@@ -241,6 +238,50 @@ pub fn push_debuginfo_type_name<'tcx>(
         }
     }
 
+    fn msvc_enum_fallback(
+        tcx: TyCtxt<'tcx>,
+        ty: Ty<'tcx>,
+        def: &AdtDef,
+        substs: SubstsRef<'tcx>,
+        output: &mut String,
+        visited: &mut FxHashSet<Ty<'tcx>>,
+    ) {
+        let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error");
+
+        if let Variants::Multiple {
+            tag_encoding: TagEncoding::Niche { dataful_variant, .. },
+            tag,
+            variants,
+            ..
+        } = &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.as_ref().unwrap().scalar.valid_range;
+
+            let min = dataful_discriminant_range.start();
+            let min = tag.value.size(&tcx).truncate(*min);
+
+            let max = dataful_discriminant_range.end();
+            let max = tag.value.size(&tcx).truncate(*max);
+
+            output.push_str("_enum<");
+            push_item_name(tcx, def.did, true, output);
+            push_type_params(tcx, substs, output, visited);
+
+            let dataful_variant_name = def.variants[*dataful_variant].ident.as_str();
+
+            output.push_str(&format!(", {}, {}, {}>", min, max, dataful_variant_name));
+        } else {
+            output.push_str("_enum<");
+            push_item_name(tcx, def.did, true, output);
+            push_type_params(tcx, substs, output, visited);
+            output.push('>');
+        }
+    }
+
     fn push_item_name(tcx: TyCtxt<'tcx>, def_id: DefId, qualified: bool, output: &mut String) {
         if qualified {
             output.push_str(&tcx.crate_name(def_id.krate).as_str());

src/etc/natvis/intrinsic.natvis

@@ -187,4 +187,18 @@
       <ExpandedItem Condition="tag() == 15" Optional="true">Variant15</ExpandedItem>
     </Expand>
   </Type>
+
+  <!-- $T1 is the name of the enum, $T2 is the low value of the dataful variant tag, $T3 is the high value of the dataful variant tag, $T4 is the name of the dataful variant -->
+  <Type Name="_enum&lt;*, *, *, *&gt;">
+    <Intrinsic Name="tag" Expression="discriminant$.discriminant" />
+    <Intrinsic Name="is_dataful" Expression="tag() &gt;= $T2 &amp;&amp; tag() &lt;= $T3" />
+    <DisplayString Condition="is_dataful()">{"$T4",sb}({dataful_variant})</DisplayString>
+    <DisplayString Condition="!is_dataful()">{discriminant$.discriminant,en}</DisplayString>
+    <Expand>
+      <ExpandedItem Condition="is_dataful()">dataful_variant</ExpandedItem>
+      <Synthetic Condition="is_dataful()" Name="[variant]">
+        <DisplayString>{"$T4",sb}</DisplayString>
+      </Synthetic>
+    </Expand>
+  </Type>
 </AutoVisualizer>