Skip to content

Separate DefIds for variants and their constructors #59382

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.

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

Merged
merged 9 commits into from
Mar 25, 2019
Prev Previous commit
Next Next commit
Remove VariantDef::parent_did
  • Loading branch information
petrochenkov committed Mar 24, 2019
commit 7f5a8dcdb95b07234d323cad8492bef96f2bee1b
2 changes: 1 addition & 1 deletion src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ impl<'tcx> cmt_<'tcx> {
};
let variant_def = match self.cat {
Categorization::Downcast(_, variant_did) => {
adt_def.variant_with_variant_id(variant_did)
adt_def.variant_with_id(variant_did)
}
_ => {
assert_eq!(adt_def.variants.len(), 1);
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2405,13 +2405,12 @@ impl<'tcx> Debug for Rvalue<'tcx> {

AggregateKind::Adt(adt_def, variant, substs, _user_ty, _) => {
let variant_def = &adt_def.variants[variant];
let did = variant_def.variant_did_or_parent_struct_did();

let f = &mut *fmt;
ty::tls::with(|tcx| {
let substs = tcx.lift(&substs).expect("could not lift for printing");
FmtPrinter::new(tcx, f, Namespace::ValueNS)
.print_def_path(did, substs)?;
.print_def_path(variant_def.def_id, substs)?;
Ok(())
})?;

Expand Down
29 changes: 1 addition & 28 deletions src/librustc/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,33 +104,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
fn ty_inhabitedness_forest(self, ty: Ty<'tcx>) -> DefIdForest {
ty.uninhabited_from(self)
}

pub fn is_enum_variant_uninhabited_from(self,
module: DefId,
variant: &'tcx VariantDef,
substs: SubstsRef<'tcx>)
-> bool
{
self.variant_inhabitedness_forest(variant, substs).contains(self, module)
}

pub fn is_variant_uninhabited_from_all_modules(self,
variant: &'tcx VariantDef,
substs: SubstsRef<'tcx>)
-> bool
{
!self.variant_inhabitedness_forest(variant, substs).is_empty()
}

fn variant_inhabitedness_forest(self, variant: &'tcx VariantDef, substs: SubstsRef<'tcx>)
-> DefIdForest {
// Determine the ADT kind:
let adt_def_id = self.adt_def_id_of_variant(variant);
let adt_kind = self.adt_def(adt_def_id).adt_kind();

// Compute inhabitedness forest:
variant.uninhabited_from(self, substs, adt_kind)
}
}

impl<'a, 'gcx, 'tcx> AdtDef {
Expand All @@ -148,7 +121,7 @@ impl<'a, 'gcx, 'tcx> AdtDef {

impl<'a, 'gcx, 'tcx> VariantDef {
/// Calculate the forest of DefIds from which this variant is visibly uninhabited.
fn uninhabited_from(
pub fn uninhabited_from(
&self,
tcx: TyCtxt<'a, 'gcx, 'tcx>,
substs: SubstsRef<'tcx>,
Expand Down
132 changes: 26 additions & 106 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1810,12 +1810,12 @@ bitflags! {
/// Definition of a variant -- a struct's fields or a enum variant.
#[derive(Debug)]
pub struct VariantDef {
/// `DefId` that identifies this enum variant. If this `VariantDef` is part of a struct or
/// union then this is `None`.
variant_did: Option<DefId>,
/// `DefId` that identifies this enum variant or struct's constructor. If this is a
/// `Struct`-variant then this is `None`.
ctor_did: Option<DefId>,
/// `DefId` that identifies the variant itself.
/// If this variant belongs to a struct or union, then this is a copy of its `DefId`.
pub def_id: DefId,
/// `DefId` that identifies the variant's constructor.
/// If this variant is a struct variant, then this is `None`.
pub ctor_def_id: Option<DefId>,
/// Variant or struct name.
pub ident: Ident,
/// Discriminant of this variant.
Expand All @@ -1824,11 +1824,6 @@ pub struct VariantDef {
pub fields: Vec<FieldDef>,
/// Type of constructor of variant.
pub ctor_kind: CtorKind,
/// `DefId` of the parent `AdtDef` representing the struct or enum. This is required as there
/// is a valid scenario where this type represents a `Struct`-struct and both `ctor_did` and
/// `variant_did` would be `None` and we would still want a way to get back to the original
/// `AdtDef`.
parent_did: DefId,
/// Flags of the variant (e.g. is field list non-exhaustive)?
flags: VariantFlags,
/// Recovered?
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the most redundant doc-comment I have ever seen.^^ It explains literally nothing...

If you have a one-sentence description of what this field does, I can add it in #63346. Or maybe it is easier for you to make a PR yourself. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that this comment isn't particularly useful. This PR was a while ago, I assume I just wanted to add some comment on each field as I was changing the type - I'm afraid I don't know what the recovered field is used for.

Expand Down Expand Up @@ -1856,7 +1851,7 @@ impl<'a, 'gcx, 'tcx> VariantDef {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
ident: Ident,
variant_did: Option<DefId>,
ctor_did: Option<DefId>,
ctor_def_id: Option<DefId>,
discr: VariantDiscr,
fields: Vec<FieldDef>,
ctor_kind: CtorKind,
Expand All @@ -1865,9 +1860,9 @@ impl<'a, 'gcx, 'tcx> VariantDef {
recovered: bool,
) -> Self {
debug!(
"VariantDef::new(ident = {:?}, variant_did = {:?}, ctor_did = {:?}, discr = {:?},
"VariantDef::new(ident = {:?}, variant_did = {:?}, ctor_def_id = {:?}, discr = {:?},
fields = {:?}, ctor_kind = {:?}, adt_kind = {:?}, parent_did = {:?})",
ident, variant_did, ctor_did, discr, fields, ctor_kind, adt_kind, parent_did,
ident, variant_did, ctor_def_id, discr, fields, ctor_kind, adt_kind, parent_did,
);

let mut flags = VariantFlags::NO_VARIANT_FLAGS;
Expand All @@ -1877,14 +1872,13 @@ impl<'a, 'gcx, 'tcx> VariantDef {
}

VariantDef {
variant_did,
ctor_did,
def_id: variant_did.unwrap_or(parent_did),
ctor_def_id,
ident,
discr,
fields,
ctor_kind,
flags,
parent_did,
recovered,
}
}
Expand All @@ -1894,62 +1888,16 @@ impl<'a, 'gcx, 'tcx> VariantDef {
pub fn is_field_list_non_exhaustive(&self) -> bool {
self.flags.intersects(VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE)
}

/// Returns `true` if this `VariantDef` represents a enum's variant.
#[inline]
pub fn is_enum_variant(&self) -> bool {
self.variant_did.is_some()
}

/// Returns `true` if this `VariantDef` represents a struct.
#[inline]
pub fn is_struct(&self) -> bool {
!self.is_enum_variant()
}

/// Returns the `DefId` of this variant if this `VariantDef` represents an enum's variant, or
/// returns the `DefId` of the parent struct.
#[inline]
pub fn variant_did_or_parent_struct_did(&self) -> DefId {
self.variant_did.unwrap_or(self.parent_did)
}

/// Returns `true` if the variant is defined in the local crate.
#[inline]
pub fn is_local(&self) -> bool {
self.variant_did_or_parent_struct_did().krate == LOCAL_CRATE
}

/// Returns the `DefId` of this variant if this `VariantDef` represents an enum's variant or
/// panics.
#[inline]
pub fn variant_did(&self) -> DefId {
self.variant_did.expect("enum variant without a variant id")
}

/// Returns the `DefId` of this variant's constructor if this is a unit or
/// tuple-variant/struct.
#[inline]
pub fn ctor_did(&self) -> Option<DefId> {
self.ctor_did
}

/// Returns the `AdtDef` representing the struct or enum associated with this `VariantDef`.
#[inline]
pub fn adt_def(&self, tcx: TyCtxt<'a, 'tcx, 'gcx>) -> &'tcx AdtDef {
tcx.adt_def(self.parent_did)
}
}

impl_stable_hash_for!(struct VariantDef {
variant_did,
ctor_did,
def_id,
ctor_def_id,
ident -> (ident.name),
discr,
fields,
ctor_kind,
flags,
parent_did,
recovered
});

Expand Down Expand Up @@ -2204,7 +2152,7 @@ impl<'a, 'gcx, 'tcx> AdtDef {
AdtKind::Struct => AdtFlags::IS_STRUCT,
};

if kind == AdtKind::Struct && variants[VariantIdx::new(0)].ctor_did.is_some() {
if kind == AdtKind::Struct && variants[VariantIdx::new(0)].ctor_def_id.is_some() {
flags |= AdtFlags::HAS_CTOR;
}

Expand Down Expand Up @@ -2351,51 +2299,29 @@ impl<'a, 'gcx, 'tcx> AdtDef {
self.variants.iter().all(|v| v.fields.is_empty())
}

pub fn variant_with_variant_id(&self, vid: DefId) -> &VariantDef {
self.variants
.iter()
.find(|v| v.variant_did.map(|did| did == vid).unwrap_or(false))
.expect("variant_with_variant_id: unknown variant")
pub fn variant_with_id(&self, vid: DefId) -> &VariantDef {
self.variants.iter().find(|v| v.def_id == vid)
.expect("variant_with_id: unknown variant")
}

pub fn variant_with_ctor_id(&self, cid: DefId) -> &VariantDef {
self.variants
.iter()
.find(|v| v.ctor_did.map(|did| did == cid).unwrap_or(false))
self.variants.iter().find(|v| v.ctor_def_id == Some(cid))
.expect("variant_with_ctor_id: unknown variant")
}

pub fn variant_index_with_variant_id(&self, vid: DefId) -> VariantIdx {
self.variants
.iter_enumerated()
.find(|(_, v)| v.variant_did.map(|did| did == vid).unwrap_or(false))
.expect("variant_index_with_variant_id: unknown variant")
.0
pub fn variant_index_with_id(&self, vid: DefId) -> VariantIdx {
self.variants.iter_enumerated().find(|(_, v)| v.def_id == vid)
.expect("variant_index_with_id: unknown variant").0
}

pub fn variant_index_with_ctor_id(&self, cid: DefId) -> VariantIdx {
self.variants
.iter_enumerated()
.find(|(_, v)| v.ctor_did.map(|did| did == cid).unwrap_or(false))
.expect("variant_index_with_ctor_id: unknown variant")
.0
}

pub fn variant_index_with_ctor_or_variant_id(&self, id: DefId) -> VariantIdx {
self.variants
.iter_enumerated()
.find(|(_, v)| {
let ctor = v.ctor_did.map(|did| did == id);
let variant = v.variant_did.map(|did| did == id);
ctor.or(variant).unwrap_or(false)
})
.expect("variant_index_with_ctor_or_variant_id: unknown variant")
.0
self.variants.iter_enumerated().find(|(_, v)| v.ctor_def_id == Some(cid))
.expect("variant_index_with_ctor_id: unknown variant").0
}

pub fn variant_of_def(&self, def: Def) -> &VariantDef {
match def {
Def::Variant(vid) => self.variant_with_variant_id(vid),
Def::Variant(vid) => self.variant_with_id(vid),
Def::Ctor(hir::CtorOf::Variant, cid, ..) => self.variant_with_ctor_id(cid),
Def::Struct(..) | Def::Ctor(..) | Def::Union(..) |
Def::TyAlias(..) | Def::AssociatedTy(..) | Def::SelfTy(..) |
Expand Down Expand Up @@ -2933,8 +2859,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {

pub fn find_field_index(self, ident: Ident, variant: &VariantDef) -> Option<usize> {
variant.fields.iter().position(|field| {
let did = variant.variant_did.unwrap_or(variant.parent_did);
self.adjust_ident(ident, did, hir::DUMMY_HIR_ID).0 == field.ident.modern()
self.adjust_ident(ident, variant.def_id, hir::DUMMY_HIR_ID).0 == field.ident.modern()
})
}

Expand Down Expand Up @@ -3011,7 +2936,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
match def {
Def::Variant(did) => {
let enum_did = self.parent(did).unwrap();
self.adt_def(enum_did).variant_with_variant_id(did)
self.adt_def(enum_did).variant_with_id(did)
}
Def::Struct(did) | Def::Union(did) => {
self.adt_def(did).non_enum_variant()
Expand All @@ -3029,11 +2954,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}
}

/// Given a `VariantDef`, returns the def-id of the `AdtDef` of which it is a part.
pub fn adt_def_id_of_variant(self, variant_def: &'tcx VariantDef) -> DefId {
variant_def.parent_did
}

pub fn item_name(self, id: DefId) -> InternedString {
if id.index == CRATE_DEF_INDEX {
self.original_crate_name(id.krate).as_interned_str()
Expand Down
21 changes: 9 additions & 12 deletions src/librustc_metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,13 +579,13 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
let tcx = self.tcx;
let def = tcx.adt_def(enum_did);
let variant = &def.variants[index];
let def_id = variant.variant_did();
let def_id = variant.def_id;
debug!("IsolatedEncoder::encode_enum_variant_info({:?})", def_id);

let data = VariantData {
ctor_kind: variant.ctor_kind,
discr: variant.discr,
ctor: variant.ctor_did().map(|did| did.index),
ctor: variant.ctor_def_id.map(|did| did.index),
ctor_sig: None,
};

Expand Down Expand Up @@ -628,7 +628,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
let tcx = self.tcx;
let def = tcx.adt_def(enum_did);
let variant = &def.variants[index];
let def_id = variant.ctor_did().unwrap();
let def_id = variant.ctor_def_id.unwrap();
debug!("IsolatedEncoder::encode_enum_variant_ctor({:?})", def_id);

let data = VariantData {
Expand Down Expand Up @@ -726,9 +726,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
let def_id = field.did;
debug!("IsolatedEncoder::encode_field({:?})", def_id);

let variant_id = tcx.hir()
.as_local_hir_id(variant.variant_did_or_parent_struct_did())
.unwrap();
let variant_id = tcx.hir().as_local_hir_id(variant.def_id).unwrap();
let variant_data = tcx.hir().expect_variant_data(variant_id);

Entry {
Expand Down Expand Up @@ -1218,9 +1216,8 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
hir::ItemKind::Enum(..) => {
let def = self.tcx.adt_def(def_id);
self.lazy_seq(def.variants.iter().map(|v| {
let did = v.variant_did();
assert!(did.is_local());
did.index
assert!(v.def_id.is_local());
v.def_id.index
}))
}
hir::ItemKind::Struct(..) |
Expand Down Expand Up @@ -1813,12 +1810,12 @@ impl<'a, 'b, 'tcx> IndexBuilder<'a, 'b, 'tcx> {

let def = self.tcx.adt_def(def_id);
for (i, variant) in def.variants.iter_enumerated() {
self.record(variant.variant_did(),
self.record(variant.def_id,
IsolatedEncoder::encode_enum_variant_info,
(def_id, Untracked(i)));

if let Some(ctor_hir_did) = variant.ctor_did() {
self.record(ctor_hir_did,
if let Some(ctor_def_id) = variant.ctor_def_id {
self.record(ctor_def_id,
IsolatedEncoder::encode_enum_variant_ctor,
(def_id, Untracked(i)));
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
i == variant_index || {
self.hir.tcx().features().never_type &&
self.hir.tcx().features().exhaustive_patterns &&
self.hir.tcx().is_variant_uninhabited_from_all_modules(v, substs)
!v.uninhabited_from(self.hir.tcx(), substs, adt_def.adt_kind()).is_empty()
}
});
if irrefutable {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/hair/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
Def::Variant(variant_id) => {
assert!(base.is_none());

let index = adt.variant_index_with_variant_id(variant_id);
let index = adt.variant_index_with_id(variant_id);
let user_provided_types = cx.tables().user_provided_types();
let user_ty = user_provided_types.get(expr.hir_id)
.map(|u_ty| *u_ty);
Expand Down
Loading