From 3780684f5aaceb5f79c00f0c0ce5a96d8edd00ee Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 24 Jun 2021 16:37:03 -0400 Subject: [PATCH] Update directly tagged enums to visualize the same as niche-layout enums Previously, directly tagged enums had a `variant$` field which would show the name of the active variant. We now show the variant using a `[variant]` synthetic item just like we do for niche-layout enums. --- .../src/debuginfo/metadata.rs | 123 +++++------------- src/etc/natvis/intrinsic.natvis | 5 +- src/test/debuginfo/msvc-pretty-enums.rs | 20 ++- src/test/debuginfo/pretty-std.rs | 8 +- 4 files changed, 61 insertions(+), 95 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 0e42931b29a..11fca11350d 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/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_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 @@ fn create_member_descriptions(&self, cx: &CodegenCx<'ll, 'tcx>) -> Vec) -> Vec { - 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$; // // } variant0; // + // {name} discriminant; // } // ``` - // The natvis in `intrinsic.nativs` then matches on `this.variant0.variant$` to + // The natvis in `intrinsic.nativs` 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 @@ fn create_member_descriptions(&self, cx: &CodegenCx<'ll, 'tcx>) -> Vec) -> Vec) -> Vec) -> Vec) -> Vec { +struct VariantMemberDescriptionFactory<'tcx> { /// Cloned from the `layout::Struct` describing the variant. offsets: Vec, 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> { 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 @@ fn create_member_descriptions(&self, cx: &CodegenCx<'ll, 'tcx>) -> Vec { - 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>, 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) } diff --git a/src/etc/natvis/intrinsic.natvis b/src/etc/natvis/intrinsic.natvis index cf887ffb0c0..8bf51732a2b 100644 --- a/src/etc/natvis/intrinsic.natvis +++ b/src/etc/natvis/intrinsic.natvis @@ -150,7 +150,7 @@ - + {tag(),en} {tag(),en} {tag(),en} @@ -169,6 +169,9 @@ {tag(),en} + + {tag(),en} + variant0 variant1 variant2 diff --git a/src/test/debuginfo/msvc-pretty-enums.rs b/src/test/debuginfo/msvc-pretty-enums.rs index cf3be2e7196..67a67863f34 100644 --- a/src/test/debuginfo/msvc-pretty-enums.rs +++ b/src/test/debuginfo/msvc-pretty-enums.rs @@ -48,14 +48,30 @@ // cdb-check: [+0x000] __0 : 0x0 [Type: unsigned int *] // cdb-check: [+0x000] discriminant : None (0x0) [Type: enum$ >, 1, [...], Some>::Discriminant$] +// cdb-command: dx -r2 h,! +// cdb-check:h,! : Some [Type: enum$ >] +// cdb-check: [+0x000] variant0 [Type: enum$ >::None] +// cdb-check: [+0x000] variant1 [Type: enum$ >::Some] +// cdb-check: [+0x004] __0 : 0xc [Type: unsigned int] +// cdb-check: [+0x000] discriminant : Some (0x1) [Type: core::option::Option] + // cdb-command: dx h // cdb-check:h : Some [Type: enum$ >] -// cdb-check: [+0x000] variant$ : Some (0x1) [Type: core::option::Option] +// cdb-check: [] [Type: enum$ >] +// cdb-check: [variant] : Some // cdb-check: [+0x004] __0 : 0xc [Type: unsigned int] +// cdb-command: dx -r2 i,! +// cdb-check:i,! : None [Type: enum$ >] +// cdb-check: [+0x000] variant0 [Type: enum$ >::None] +// cdb-check: [+0x000] variant1 [Type: enum$ >::Some] +// cdb-check: [+0x004] __0 : 0x5c0065 [Type: unsigned int] +// cdb-check: [+0x000] discriminant : None (0x0) [Type: core::option::Option] + // cdb-command: dx i // cdb-check:i : None [Type: enum$ >] -// cdb-check: [+0x000] variant$ : None (0x0) [Type: core::option::Option] +// cdb-check: [] [Type: enum$ >] +// cdb-check: [variant] : None // cdb-command: dx j // cdb-check:j : High (0x10) [Type: msvc_pretty_enums::CStyleEnum] diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index a7f384c0500..7ed76beb8c6 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/src/test/debuginfo/pretty-std.rs @@ -116,12 +116,14 @@ // cdb-command: dx some // cdb-check:some : Some [Type: enum$ >] -// cdb-check: [...] variant$ : Some (0x1) [Type: core::option::Option] -// cdb-check: [...] __0 : 8 [Type: short] +// cdb-check: [] [Type: enum$ >] +// cdb-check: [variant] : Some +// cdb-check: [+0x002] __0 : 8 [Type: short] // cdb-command: dx none // cdb-check:none : None [Type: enum$ >] -// cdb-check: [...] variant$ : None (0x0) [Type: core::option::Option] +// cdb-check: [] [Type: enum$ >] +// cdb-check: [variant] : None // cdb-command: dx some_string // NOTE: cdb fails to interpret debug info of Option enums on i686.