Auto merge of #86636 - wesleywiser:misc_enum_improvements, r=michaelw…

…oerister

[msvc] Consistently show active variant and fix visualization for single variant enums

Prior to this change, there were a few cases where inspecting an enum in either WinDbg or Visual Studio would not show the active variant name. After these changes, we now consistently show the active variant name as `[variant]` in the debugger.

We also didn't handle single variant enums very well. That is now also resolved.

Before:
![image](https://user-images.githubusercontent.com/831192/123480097-dc8b5f00-d5cf-11eb-93a8-9fc05a97029b.png)

After:
![image](https://user-images.githubusercontent.com/831192/123479966-aa79fd00-d5cf-11eb-955e-9798616a8829.png)

r? `@michaelwoerister`

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -1,4 +1,3 @@
-use self::EnumTagInfo::*;
 use self::MemberDescriptionFactory::*;
 use self::RecursiveTypeDescription::*;
 
@@ -28,7 +27,7 @@ use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
 use rustc_index::vec::{Idx, IndexVec};
 use rustc_middle::ich::NodeIdHashingMode;
-use rustc_middle::mir::{self, Field, GeneratorLayout};
+use rustc_middle::mir::{self, GeneratorLayout};
 use rustc_middle::ty::layout::{self, IntegerExt, PrimitiveExt, TyAndLayout};
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_middle::ty::Instance;
@@ -1188,7 +1187,7 @@ enum MemberDescriptionFactory<'ll, 'tcx> {
     TupleMDF(TupleMemberDescriptionFactory<'tcx>),
     EnumMDF(EnumMemberDescriptionFactory<'ll, 'tcx>),
     UnionMDF(UnionMemberDescriptionFactory<'tcx>),
-    VariantMDF(VariantMemberDescriptionFactory<'ll, 'tcx>),
+    VariantMDF(VariantMemberDescriptionFactory<'tcx>),
 }
 
 impl MemberDescriptionFactory<'ll, 'tcx> {
@@ -1505,14 +1504,8 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                 }
 
                 let variant_info = variant_info_for(index);
-                let (variant_type_metadata, member_description_factory) = describe_enum_variant(
-                    cx,
-                    self.layout,
-                    variant_info,
-                    None,
-                    self_metadata,
-                    self.span,
-                );
+                let (variant_type_metadata, member_description_factory) =
+                    describe_enum_variant(cx, self.layout, variant_info, self_metadata, self.span);
 
                 let member_descriptions = member_description_factory.create_member_descriptions(cx);
 
@@ -1524,7 +1517,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                     Some(&self.common_members),
                 );
                 vec![MemberDescription {
-                    name: if fallback { String::new() } else { variant_info.variant_name() },
+                    name: variant_info.variant_name(),
                     type_metadata: variant_type_metadata,
                     offset: Size::ZERO,
                     size: self.layout.size,
@@ -1540,28 +1533,38 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                 ref variants,
                 ..
             } => {
-                let tag_info = if fallback {
-                    // For MSVC, we generate a union of structs for each variant with an explicit
-                    // discriminant field roughly equivalent to the following C:
+                let fallback_discr_variant = if fallback {
+                    // For MSVC, we generate a union of structs for each variant and an
+                    // explicit discriminant field roughly equivalent to the following C:
                     // ```c
                     // union enum$<{name}> {
                     //   struct {variant 0 name} {
-                    //     tag$ variant$;
                     //     <variant 0 fields>
                     //   } variant0;
                     //   <other variant structs>
+                    //   {name} discriminant;
                     // }
                     // ```
-                    // The natvis in `intrinsic.nativs` then matches on `this.variant0.variant$` to
+                    // The natvis in `intrinsic.natvis` then matches on `this.discriminant` to
                     // determine which variant is active and then displays it.
-                    Some(DirectTag {
-                        tag_field: Field::from(tag_field),
-                        tag_type_metadata: self.tag_type_metadata.unwrap(),
+                    let enum_layout = self.layout;
+                    let offset = enum_layout.fields.offset(tag_field);
+                    let discr_ty = enum_layout.field(cx, tag_field).ty;
+                    let (size, align) = cx.size_and_align_of(discr_ty);
+                    Some(MemberDescription {
+                        name: "discriminant".into(),
+                        type_metadata: self.tag_type_metadata.unwrap(),
+                        offset,
+                        size,
+                        align,
+                        flags: DIFlags::FlagZero,
+                        discriminant: None,
+                        source_info: None,
                     })
                 } else {
-                    // This doesn't matter in this case.
                     None
                 };
+
                 variants
                     .iter_enumerated()
                     .map(|(i, _)| {
@@ -1571,7 +1574,6 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                             cx,
                             variant,
                             variant_info,
-                            tag_info,
                             self_metadata,
                             self.span,
                         );
@@ -1605,6 +1607,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                             source_info: variant_info.source_info(cx),
                         }
                     })
+                    .chain(fallback_discr_variant.into_iter())
                     .collect()
             }
             Variants::Multiple {
@@ -1702,7 +1705,6 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                         cx,
                         dataful_variant_layout,
                         variant_info,
-                        Some(NicheTag),
                         self_metadata,
                         self.span,
                     );
@@ -1754,7 +1756,6 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                                     cx,
                                     variant,
                                     variant_info,
-                                    Some(NicheTag),
                                     self_metadata,
                                     self.span,
                                 );
@@ -1791,39 +1792,27 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
 }
 
 // Creates `MemberDescription`s for the fields of a single enum variant.
-struct VariantMemberDescriptionFactory<'ll, 'tcx> {
+struct VariantMemberDescriptionFactory<'tcx> {
     /// Cloned from the `layout::Struct` describing the variant.
     offsets: Vec<Size>,
     args: Vec<(String, Ty<'tcx>)>,
-    tag_type_metadata: Option<&'ll DIType>,
     span: Span,
 }
 
-impl VariantMemberDescriptionFactory<'ll, 'tcx> {
+impl VariantMemberDescriptionFactory<'tcx> {
     fn create_member_descriptions(&self, cx: &CodegenCx<'ll, 'tcx>) -> Vec<MemberDescription<'ll>> {
         self.args
             .iter()
             .enumerate()
             .map(|(i, &(ref name, ty))| {
-                // Discriminant is always the first field of our variant
-                // when using the enum fallback.
-                let is_artificial_discr = use_enum_fallback(cx) && i == 0;
                 let (size, align) = cx.size_and_align_of(ty);
                 MemberDescription {
                     name: name.to_string(),
-                    type_metadata: if is_artificial_discr {
-                        self.tag_type_metadata.unwrap_or_else(|| type_metadata(cx, ty, self.span))
-                    } else {
-                        type_metadata(cx, ty, self.span)
-                    },
+                    type_metadata: type_metadata(cx, ty, self.span),
                     offset: self.offsets[i],
                     size,
                     align,
-                    flags: if is_artificial_discr {
-                        DIFlags::FlagArtificial
-                    } else {
-                        DIFlags::FlagZero
-                    },
+                    flags: DIFlags::FlagZero,
                     discriminant: None,
                     source_info: None,
                 }
@@ -1832,12 +1821,6 @@ impl VariantMemberDescriptionFactory<'ll, 'tcx> {
     }
 }
 
-#[derive(Copy, Clone)]
-enum EnumTagInfo<'ll> {
-    DirectTag { tag_field: Field, tag_type_metadata: &'ll DIType },
-    NicheTag,
-}
-
 #[derive(Copy, Clone)]
 enum VariantInfo<'a, 'tcx> {
     Adt(&'tcx ty::VariantDef),
@@ -1916,7 +1899,6 @@ fn describe_enum_variant(
     cx: &CodegenCx<'ll, 'tcx>,
     layout: layout::TyAndLayout<'tcx>,
     variant: VariantInfo<'_, 'tcx>,
-    discriminant_info: Option<EnumTagInfo<'ll>>,
     containing_scope: &'ll DIScope,
     span: Span,
 ) -> (&'ll DICompositeType, MemberDescriptionFactory<'ll, 'tcx>) {
@@ -1935,50 +1917,13 @@ fn describe_enum_variant(
         )
     });
 
-    // Build an array of (field name, field type) pairs to be captured in the factory closure.
-    let (offsets, args) = if use_enum_fallback(cx) {
-        // If this is not a univariant enum, there is also the discriminant field.
-        let (discr_offset, discr_arg) = match discriminant_info {
-            Some(DirectTag { tag_field, .. }) => {
-                // We have the layout of an enum variant, we need the layout of the outer enum
-                let enum_layout = cx.layout_of(layout.ty);
-                let offset = enum_layout.fields.offset(tag_field.as_usize());
-                let args = ("variant$".to_owned(), enum_layout.field(cx, tag_field.as_usize()).ty);
-                (Some(offset), Some(args))
-            }
-            _ => (None, None),
-        };
-        (
-            discr_offset
-                .into_iter()
-                .chain((0..layout.fields.count()).map(|i| layout.fields.offset(i)))
-                .collect(),
-            discr_arg
-                .into_iter()
-                .chain(
-                    (0..layout.fields.count())
-                        .map(|i| (variant.field_name(i), layout.field(cx, i).ty)),
-                )
-                .collect(),
-        )
-    } else {
-        (
-            (0..layout.fields.count()).map(|i| layout.fields.offset(i)).collect(),
-            (0..layout.fields.count())
-                .map(|i| (variant.field_name(i), layout.field(cx, i).ty))
-                .collect(),
-        )
-    };
+    let offsets = (0..layout.fields.count()).map(|i| layout.fields.offset(i)).collect();
+    let args = (0..layout.fields.count())
+        .map(|i| (variant.field_name(i), layout.field(cx, i).ty))
+        .collect();
 
-    let member_description_factory = VariantMDF(VariantMemberDescriptionFactory {
-        offsets,
-        args,
-        tag_type_metadata: match discriminant_info {
-            Some(DirectTag { tag_type_metadata, .. }) => Some(tag_type_metadata),
-            _ => None,
-        },
-        span,
-    });
+    let member_description_factory =
+        VariantMDF(VariantMemberDescriptionFactory { offsets, args, span });
 
     (metadata_stub, member_description_factory)
 }

compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs

@@ -367,6 +367,10 @@ pub fn push_debuginfo_type_name<'tcx>(
     ) {
         let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error");
 
+        output.push_str("enum$<");
+        push_item_name(tcx, def.did, true, output);
+        push_generic_params_internal(tcx, substs, output, visited);
+
         if let Variants::Multiple {
             tag_encoding: TagEncoding::Niche { dataful_variant, .. },
             tag,
@@ -386,19 +390,19 @@ pub fn push_debuginfo_type_name<'tcx>(
             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_generic_params_internal(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_generic_params_internal(tcx, substs, output, visited);
-            push_close_angle_bracket(tcx, output);
+            output.push_str(&format!(", {}, {}, {}", min, max, dataful_variant_name));
+        } else if let Variants::Single { index: variant_idx } = &layout.variants {
+            // Uninhabited enums can't be constructed and should never need to be visualized so
+            // skip this step for them.
+            if def.variants.len() != 0 {
+                let variant = def.variants[*variant_idx].ident.as_str();
+
+                output.push_str(&format!(", {}", variant));
+            }
         }
+        push_close_angle_bracket(tcx, output);
     }
 }
 

src/etc/natvis/intrinsic.natvis

@@ -149,8 +149,10 @@
       <Synthetic Name="[...]"><DisplayString>...</DisplayString></Synthetic>
     </Expand>
   </Type>
+
+  <!-- Directly tagged enums. $T1 is the type name -->
   <Type Name="enum$&lt;*&gt;">
-    <Intrinsic Name="tag" Expression="variant0.variant$" />
+    <Intrinsic Name="tag" Expression="discriminant" />
     <DisplayString Condition="tag() == 0">{tag(),en}</DisplayString>
     <DisplayString Condition="tag() == 1" Optional="true">{tag(),en}</DisplayString>
     <DisplayString Condition="tag() == 2" Optional="true">{tag(),en}</DisplayString>
@@ -169,6 +171,9 @@
     <DisplayString Condition="tag() == 15" Optional="true">{tag(),en}</DisplayString>
 
     <Expand>
+      <Synthetic Name="[variant]">
+        <DisplayString>{tag(),en}</DisplayString>
+      </Synthetic>
       <ExpandedItem Condition="tag() == 0">variant0</ExpandedItem>
       <ExpandedItem Condition="tag() == 1" Optional="true">variant1</ExpandedItem>
       <ExpandedItem Condition="tag() == 2" Optional="true">variant2</ExpandedItem>
@@ -188,8 +193,20 @@
     </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 -->
+  <!-- Single variant enums. $T1 is the name of the enum, $T2 is the name of the variant -->
+  <Type Name="enum$&lt;*, *&gt;">
+    <DisplayString>{"$T2",sb}</DisplayString>
+    <Expand>
+      <Synthetic Name="[variant]">
+        <DisplayString>{"$T2",sb}</DisplayString>
+      </Synthetic>
+      <ExpandedItem>$T2</ExpandedItem>
+    </Expand>
+  </Type>
+
+  <!-- Niche-layout enums. $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" />
     <Intrinsic Name="is_dataful" Expression="tag() &gt;= $T2 &amp;&amp; tag() &lt;= $T3" />
@@ -200,6 +217,9 @@
       <Synthetic Condition="is_dataful()" Name="[variant]">
         <DisplayString>{"$T4",sb}</DisplayString>
       </Synthetic>
+      <Synthetic Condition="!is_dataful()" Name="[variant]">
+        <DisplayString>{discriminant,en}</DisplayString>
+      </Synthetic>
     </Expand>
   </Type>
 </AutoVisualizer>

src/etc/natvis/libcore.natvis

@@ -14,14 +14,6 @@
     </Expand>
   </Type>
 
-  <Type Name="core::option::Option&lt;*&gt;" Priority="MediumLow">
-    <DisplayString Condition="*(void**)this == nullptr">None</DisplayString>
-    <DisplayString>Some({($T1 *)this})</DisplayString>
-    <Expand>
-      <Item Name="Some" ExcludeView="simple" Condition="*(void**)this != nullptr">($T1 *)this</Item>
-    </Expand>
-  </Type>
-
   <Type Name="core::ptr::non_null::NonNull&lt;*&gt;">
     <DisplayString>{(void*) pointer}</DisplayString>
     <Expand>

src/test/codegen/async-fn-debug-msvc.rs

@@ -43,11 +43,11 @@ async fn async_fn_test() {
 // CHECK:      [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]],
 // CHECK-NOT:  flags: DIFlagArtificial
 // CHECK-SAME: )
-// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant$", scope: [[S1]],
-// CHECK-SAME: flags: DIFlagArtificial
 // CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]]
 // CHECK-NOT:  flags: DIFlagArtificial
 // CHECK-SAME: )
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "discriminant", scope: [[GEN]],
+// CHECK-NOT: flags: DIFlagArtificial
 
 fn main() {
     let _dummy = async_fn_test();

src/test/codegen/generator-debug-msvc.rs

@@ -47,11 +47,11 @@ fn generator_test() -> impl Generator<Yield = i32, Return = ()> {
 // CHECK:      [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]],
 // CHECK-NOT:  flags: DIFlagArtificial
 // CHECK-SAME: )
-// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant$", scope: [[S1]],
-// CHECK-SAME: flags: DIFlagArtificial
 // CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]]
 // CHECK-NOT:  flags: DIFlagArtificial
 // CHECK-SAME: )
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "discriminant", scope: [[GEN]],
+// CHECK-NOT: flags: DIFlagArtificial
 
 fn main() {
     let _dummy = generator_test();