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

[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`
This commit is contained in:
bors 2021-07-06 19:31:24 +00:00
commit 885399992c
8 changed files with 112 additions and 121 deletions

View File

@ -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)
}

View File

@ -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);
}
}

View File

@ -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>

View File

@ -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>

View File

@ -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();

View File

@ -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();

View File

@ -1,10 +1,14 @@
// only-cdb
// ignore-tidy-linelength
// compile-flags:-g
// cdb-command: g
// Note: The natvis used to visualize niche-layout enums don't work correctly in cdb
// so the best we can do is to make sure we are generating the right debuginfo
// so the best we can do is to make sure we are generating the right debuginfo.
// Therefore, we use the `!` [format specifier](https://docs.microsoft.com/en-us/visualstudio/debugger/format-specifiers-in-cpp?view=vs-2019#BKMK_Visual_Studio_2012_format_specifiers)
// to disable the natvis for a given expression. We also provide the `-r2` flag
// to expand the expression 2 levels.
// cdb-command: dx -r2 a,!
// cdb-check:a,! [Type: enum$<core::option::Option<enum$<msvc_pretty_enums::CStyleEnum> >, 2, 16, Some>]
@ -48,14 +52,30 @@
// cdb-check: [+0x000] __0 : 0x0 [Type: unsigned int *]
// cdb-check: [+0x000] discriminant : None (0x0) [Type: enum$<core::option::Option<ref$<u32> >, 1, [...], Some>::Discriminant$]
// cdb-command: dx -r2 h,!
// cdb-check:h,! : Some [Type: enum$<core::option::Option<u32> >]
// cdb-check: [+0x000] variant0 [Type: enum$<core::option::Option<u32> >::None]
// cdb-check: [+0x000] variant1 [Type: enum$<core::option::Option<u32> >::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$<core::option::Option<u32> >]
// cdb-check: [+0x000] variant$ : Some (0x1) [Type: core::option::Option]
// cdb-check: [<Raw View>] [Type: enum$<core::option::Option<u32> >]
// cdb-check: [variant] : Some
// cdb-check: [+0x004] __0 : 0xc [Type: unsigned int]
// cdb-command: dx -r2 i,!
// cdb-check:i,! : None [Type: enum$<core::option::Option<u32> >]
// cdb-check: [+0x000] variant0 [Type: enum$<core::option::Option<u32> >::None]
// cdb-check: [+0x000] variant1 [Type: enum$<core::option::Option<u32> >::Some]
// cdb-check: [+0x004] __0 : 0x[...] [Type: unsigned int]
// cdb-check: [+0x000] discriminant : None (0x0) [Type: core::option::Option]
// cdb-command: dx i
// cdb-check:i : None [Type: enum$<core::option::Option<u32> >]
// cdb-check: [+0x000] variant$ : None (0x0) [Type: core::option::Option]
// cdb-check: [<Raw View>] [Type: enum$<core::option::Option<u32> >]
// cdb-check: [variant] : None
// cdb-command: dx j
// cdb-check:j : High (0x10) [Type: msvc_pretty_enums::CStyleEnum]
@ -66,6 +86,11 @@
// cdb-check: [+0x000] __0 [Type: alloc::string::String]
// cdb-check: [+0x000] discriminant : 0x[...] [Type: enum$<core::option::Option<alloc::string::String>, 1, [...], Some>::Discriminant$]
// cdb-command: dx -r2 l,!
// cdb-check:l,! : $T2 [Type: enum$<core::result::Result<u32, enum$<msvc_pretty_enums::Empty> >, Ok>]
// cdb-check: [+0x000] Ok [Type: enum$<core::result::Result<u32, enum$<msvc_pretty_enums::Empty> >, Ok>::Ok]
// cdb-check: [+0x000] __0 : 0x2a [Type: unsigned int]
pub enum CStyleEnum {
Low = 2,
High = 16,
@ -77,6 +102,8 @@ pub enum NicheLayoutEnum {
Tag2,
}
pub enum Empty { }
fn main() {
let a = Some(CStyleEnum::Low);
let b = Option::<CStyleEnum>::None;
@ -89,6 +116,7 @@ fn main() {
let i = Option::<u32>::None;
let j = CStyleEnum::High;
let k = Some("IAMA optional string!".to_string());
let l = Result::<u32, Empty>::Ok(42);
zzz(); // #break
}

View File

@ -116,12 +116,14 @@
// cdb-command: dx some
// cdb-check:some : Some [Type: enum$<core::option::Option<i16> >]
// cdb-check: [...] variant$ : Some (0x1) [Type: core::option::Option]
// cdb-check: [...] __0 : 8 [Type: short]
// cdb-check: [<Raw View>] [Type: enum$<core::option::Option<i16> >]
// cdb-check: [variant] : Some
// cdb-check: [+0x002] __0 : 8 [Type: short]
// cdb-command: dx none
// cdb-check:none : None [Type: enum$<core::option::Option<i64> >]
// cdb-check: [...] variant$ : None (0x0) [Type: core::option::Option]
// cdb-check: [<Raw View>] [Type: enum$<core::option::Option<i64> >]
// cdb-check: [variant] : None
// cdb-command: dx some_string
// NOTE: cdb fails to interpret debug info of Option enums on i686.