Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 8 commits into from
Jul 6, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3780684
Update directly tagged enums to visualize the same as niche-layout enums
wesleywiser Jun 24, 2021
f243558
Always show variant tag in niche-layout enums
wesleywiser Jun 24, 2021
d94f087
Show the variant name for univariant enums
wesleywiser Jun 25, 2021
9b5fe6e
Remove unnecessary visualizer
wesleywiser Jun 25, 2021
f6d2843
Respond to review feedback
wesleywiser Jul 1, 2021
9c31482
Refactor `msvc_enum_fallbck()` a bit
wesleywiser Jul 3, 2021
38b9061
Fix async-fn-debug-msvc and generator-debug-msvc codegen tests
wesleywiser Jul 5, 2021
457165e
Fix failing test on i686-pc-windows-msvc
wesleywiser Jul 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 35 additions & 90 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use self::EnumTagInfo::*;
use self::MemberDescriptionFactory::*;
use self::RecursiveTypeDescription::*;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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);

Expand All @@ -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,
Expand All @@ -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(),
Copy link
Member

@bjorn3 bjorn3 Jul 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe name this discriminant$? enum Foo { discriminant { bar: u8 } } is valid and would otherwise conflict with this I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Foo in your example is Variants::Single so it doesn't fall into this case. enum Foo2 { discriminant { bar: u8 }, baz { bat: u8 } } does fall into this case but is represented as

enum Foo2 {
  discriminant = 0,
  baz = 1,
}

union enum$<Foo2> {
  Foo2 discriminant;
  struct discriminant {
    byte bar;
  }  variant0;
  struct baz {
    byte bat;
  }  variant1;
}

So the discriminant name used here can't conflict with an identifier used in Rust source.

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, _)| {
Expand All @@ -1571,7 +1574,6 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
cx,
variant,
variant_info,
tag_info,
self_metadata,
self.span,
);
Expand Down Expand Up @@ -1605,6 +1607,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
source_info: variant_info.source_info(cx),
}
})
.chain(fallback_discr_variant.into_iter())
.collect()
}
Variants::Multiple {
Expand Down Expand Up @@ -1702,7 +1705,6 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
cx,
dataful_variant_layout,
variant_info,
Some(NicheTag),
self_metadata,
self.span,
);
Expand Down Expand Up @@ -1754,7 +1756,6 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
cx,
variant,
variant_info,
Some(NicheTag),
self_metadata,
self.span,
);
Expand Down Expand Up @@ -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,
}
Expand All @@ -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),
Expand Down Expand Up @@ -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>) {
Expand All @@ -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();
wesleywiser marked this conversation as resolved.
Show resolved Hide resolved

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)
}
Expand Down
24 changes: 14 additions & 10 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
}

Expand Down
26 changes: 23 additions & 3 deletions src/etc/natvis/intrinsic.natvis
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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" />
Expand All @@ -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>
8 changes: 0 additions & 8 deletions src/etc/natvis/libcore.natvis
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions src/test/codegen/async-fn-debug-msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/test/codegen/generator-debug-msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading