Skip to content

Refactoring/bugfixing around definitions for struct/variant constructors #36814

New issue

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

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

Already on GitHub? # to your account

Merged
merged 8 commits into from
Oct 5, 2016
85 changes: 60 additions & 25 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,46 @@ use util::nodemap::NodeMap;
use syntax::ast;
use hir;

#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub enum CtorKind {
// Constructor function automatically created by a tuple struct/variant.
Fn,
// Constructor constant automatically created by a unit struct/variant.
Const,
// Unusable name in value namespace created by a struct variant.
Fictive,
}

#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub enum Def {
Fn(DefId),
SelfTy(Option<DefId> /* trait */, Option<DefId> /* impl */),
// Type namespace
Mod(DefId),
Static(DefId, bool /* is_mutbl */),
Const(DefId),
AssociatedConst(DefId),
Local(DefId),
Variant(DefId),
Struct(DefId), // DefId refers to NodeId of the struct itself
Union(DefId),
Enum(DefId),
Variant(DefId),
Trait(DefId),
TyAlias(DefId),
AssociatedTy(DefId),
Trait(DefId),
PrimTy(hir::PrimTy),
TyParam(DefId),
Upvar(DefId, // def id of closed over local
usize, // index in the freevars list of the closure
ast::NodeId), // expr node that creates the closure
SelfTy(Option<DefId> /* trait */, Option<DefId> /* impl */),

// If Def::Struct lives in type namespace it denotes a struct item and its DefId refers
// to NodeId of the struct itself.
// If Def::Struct lives in value namespace (e.g. tuple struct, unit struct expressions)
// it denotes a constructor and its DefId refers to NodeId of the struct's constructor.
Struct(DefId),
Union(DefId),
Label(ast::NodeId),
// Value namespace
Fn(DefId),
Const(DefId),
Static(DefId, bool /* is_mutbl */),
StructCtor(DefId, CtorKind), // DefId refers to NodeId of the struct's constructor
VariantCtor(DefId, CtorKind),
Method(DefId),
AssociatedConst(DefId),
Local(DefId),
Upvar(DefId, // def id of closed over local
usize, // index in the freevars list of the closure
ast::NodeId), // expr node that creates the closure
Label(ast::NodeId),

// Both namespaces
Err,
}

Expand Down Expand Up @@ -93,18 +105,35 @@ pub type ExportMap = NodeMap<Vec<Export>>;

#[derive(Copy, Clone, RustcEncodable, RustcDecodable)]
pub struct Export {
pub name: ast::Name, // The name of the target.
pub def_id: DefId, // The definition of the target.
pub name: ast::Name, // The name of the target.
pub def: Def, // The definition of the target.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DefId alone is not enough for describing a reexport precisely.
Variants can be exported separately in type and value namespaces, but have a single DefId (and NodeId).

}

impl CtorKind {
pub fn from_ast(vdata: &ast::VariantData) -> CtorKind {
match *vdata {
ast::VariantData::Tuple(..) => CtorKind::Fn,
ast::VariantData::Unit(..) => CtorKind::Const,
ast::VariantData::Struct(..) => CtorKind::Fictive,
}
}
pub fn from_hir(vdata: &hir::VariantData) -> CtorKind {
match *vdata {
hir::VariantData::Tuple(..) => CtorKind::Fn,
hir::VariantData::Unit(..) => CtorKind::Const,
hir::VariantData::Struct(..) => CtorKind::Fictive,
}
}
}

impl Def {
pub fn def_id(&self) -> DefId {
match *self {
Def::Fn(id) | Def::Mod(id) | Def::Static(id, _) |
Def::Variant(id) | Def::Enum(id) | Def::TyAlias(id) | Def::AssociatedTy(id) |
Def::TyParam(id) | Def::Struct(id) | Def::Union(id) | Def::Trait(id) |
Def::Method(id) | Def::Const(id) | Def::AssociatedConst(id) |
Def::Local(id) | Def::Upvar(id, ..) => {
Def::Variant(id) | Def::VariantCtor(id, ..) | Def::Enum(id) | Def::TyAlias(id) |
Def::AssociatedTy(id) | Def::TyParam(id) | Def::Struct(id) | Def::StructCtor(id, ..) |
Def::Union(id) | Def::Trait(id) | Def::Method(id) | Def::Const(id) |
Def::AssociatedConst(id) | Def::Local(id) | Def::Upvar(id, ..) => {
id
}

Expand All @@ -123,10 +152,16 @@ impl Def {
Def::Mod(..) => "module",
Def::Static(..) => "static",
Def::Variant(..) => "variant",
Def::VariantCtor(.., CtorKind::Fn) => "tuple variant",
Def::VariantCtor(.., CtorKind::Const) => "unit variant",
Def::VariantCtor(.., CtorKind::Fictive) => "struct variant",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with more traditional, but less precise descriptions.
Something like "variant's constructor functions" and "fictive variant constructor" would be more appropriate, but they are a bit long and not very user friendly.

Def::Enum(..) => "enum",
Def::TyAlias(..) => "type",
Def::TyAlias(..) => "type alias",
Def::AssociatedTy(..) => "associated type",
Def::Struct(..) => "struct",
Def::StructCtor(.., CtorKind::Fn) => "tuple struct",
Def::StructCtor(.., CtorKind::Const) => "unit struct",
Def::StructCtor(.., CtorKind::Fictive) => bug!("impossible struct constructor"),
Def::Union(..) => "union",
Def::Trait(..) => "trait",
Def::Method(..) => "method",
Expand Down
24 changes: 19 additions & 5 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ use hir;
use hir::map::Definitions;
use hir::map::definitions::DefPathData;
use hir::def_id::{DefIndex, DefId};
use hir::def::{Def, PathResolution};
use hir::def::{Def, CtorKind, PathResolution};
use session::Session;
use lint;

use std::collections::BTreeMap;
use std::iter;
Expand Down Expand Up @@ -855,10 +856,23 @@ impl<'a> LoweringContext<'a> {
})
}
PatKind::Lit(ref e) => hir::PatKind::Lit(self.lower_expr(e)),
PatKind::TupleStruct(ref pth, ref pats, ddpos) => {
hir::PatKind::TupleStruct(self.lower_path(pth),
pats.iter().map(|x| self.lower_pat(x)).collect(),
ddpos)
PatKind::TupleStruct(ref path, ref pats, ddpos) => {
match self.resolver.get_resolution(p.id).map(|d| d.base_def) {
Some(def @ Def::StructCtor(_, CtorKind::Const)) |
Some(def @ Def::VariantCtor(_, CtorKind::Const)) => {
// Temporarily lower `UnitVariant(..)` into `UnitVariant`
// for backward compatibility.
let msg = format!("expected tuple struct/variant, found {} `{}`",
def.kind_name(), path);
self.sess.add_lint(
lint::builtin::MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
p.id, p.span, msg
);
hir::PatKind::Path(None, self.lower_path(path))
}
_ => hir::PatKind::TupleStruct(self.lower_path(path),
pats.iter().map(|x| self.lower_pat(x)).collect(), ddpos)
}
}
PatKind::Path(ref opt_qself, ref path) => {
let opt_qself = opt_qself.as_ref().map(|qself| {
Expand Down
9 changes: 4 additions & 5 deletions src/librustc/hir/pat_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub fn pat_is_refutable(dm: &DefMap, pat: &hir::Pat) -> bool {
PatKind::Path(..) |
PatKind::Struct(..) => {
match dm.get(&pat.id).map(|d| d.full_def()) {
Some(Def::Variant(..)) => true,
Some(Def::Variant(..)) | Some(Def::VariantCtor(..)) => true,
_ => false
}
}
Expand Down Expand Up @@ -173,10 +173,9 @@ pub fn necessary_variants(dm: &DefMap, pat: &hir::Pat) -> Vec<DefId> {
PatKind::TupleStruct(..) |
PatKind::Path(..) |
PatKind::Struct(..) => {
match dm.get(&p.id) {
Some(&PathResolution { base_def: Def::Variant(id), .. }) => {
variants.push(id);
}
match dm.get(&p.id).map(|d| d.full_def()) {
Some(Def::Variant(id)) |
Some(Def::VariantCtor(id, ..)) => variants.push(id),
_ => ()
}
}
Expand Down
5 changes: 0 additions & 5 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,6 @@ pub trait CrateStore<'tcx> {
-> Option<DefIndex>;
fn def_key(&self, def: DefId) -> hir_map::DefKey;
fn relative_def_path(&self, def: DefId) -> Option<hir_map::DefPath>;
fn variant_kind(&self, def_id: DefId) -> Option<ty::VariantKind>;
fn struct_ctor_def_id(&self, struct_def_id: DefId) -> Option<DefId>;
fn struct_field_names(&self, def: DefId) -> Vec<ast::Name>;
fn item_children(&self, did: DefId) -> Vec<def::Export>;

Expand Down Expand Up @@ -378,9 +376,6 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
fn relative_def_path(&self, def: DefId) -> Option<hir_map::DefPath> {
bug!("relative_def_path")
}
fn variant_kind(&self, def_id: DefId) -> Option<ty::VariantKind> { bug!("variant_kind") }
fn struct_ctor_def_id(&self, struct_def_id: DefId) -> Option<DefId>
{ bug!("struct_ctor_def_id") }
fn struct_field_names(&self, def: DefId) -> Vec<ast::Name> { bug!("struct_field_names") }
fn item_children(&self, did: DefId) -> Vec<def::Export> { bug!("item_children") }

Expand Down
5 changes: 2 additions & 3 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,8 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
self.check_def_id(def.def_id());
}
_ if self.ignore_non_const_paths => (),
Def::PrimTy(_) => (),
Def::SelfTy(..) => (),
Def::Variant(variant_id) => {
Def::PrimTy(..) | Def::SelfTy(..) => (),
Def::Variant(variant_id) | Def::VariantCtor(variant_id, ..) => {
if let Some(enum_id) = self.tcx.parent_def_id(variant_id) {
self.check_def_id(enum_id);
}
Expand Down
9 changes: 6 additions & 3 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,8 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
// the leaves of the pattern tree structure.
return_if_err!(mc.cat_pattern(cmt_discr, pat, |mc, cmt_pat, pat| {
match tcx.expect_def_or_none(pat.id) {
Some(Def::Variant(variant_did)) => {
Some(Def::Variant(variant_did)) |
Some(Def::VariantCtor(variant_did, ..)) => {
let enum_did = tcx.parent_def_id(variant_did).unwrap();
let downcast_cmt = if tcx.lookup_adt_def(enum_did).is_univariant() {
cmt_pat
Expand All @@ -1015,12 +1016,14 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
debug!("variant downcast_cmt={:?} pat={:?}", downcast_cmt, pat);
delegate.matched_pat(pat, downcast_cmt, match_mode);
}
Some(Def::Struct(..)) | Some(Def::Union(..)) |
Some(Def::Struct(..)) | Some(Def::StructCtor(..)) | Some(Def::Union(..)) |
Some(Def::TyAlias(..)) | Some(Def::AssociatedTy(..)) => {
debug!("struct cmt_pat={:?} pat={:?}", cmt_pat, pat);
delegate.matched_pat(pat, cmt_pat, match_mode);
}
_ => {}
None | Some(Def::Local(..)) |
Some(Def::Const(..)) | Some(Def::AssociatedConst(..)) => {}
def => bug!("unexpected definition: {:?}", def)
}
}));
}
Expand Down
22 changes: 7 additions & 15 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ use hir::def_id::DefId;
use hir::map as ast_map;
use infer::InferCtxt;
use middle::const_qualif::ConstQualif;
use hir::def::Def;
use hir::def::{Def, CtorKind};
use ty::adjustment;
use ty::{self, Ty, TyCtxt};

Expand Down Expand Up @@ -524,20 +524,11 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
id, expr_ty, def);

match def {
Def::Struct(..) | Def::Union(..) | Def::Variant(..) | Def::Const(..) |
Def::StructCtor(..) | Def::VariantCtor(..) | Def::Const(..) |
Def::AssociatedConst(..) | Def::Fn(..) | Def::Method(..) => {
Ok(self.cat_rvalue_node(id, span, expr_ty))
}

Def::Mod(_) |
Def::Trait(_) | Def::Enum(..) | Def::TyAlias(..) | Def::PrimTy(_) |
Def::TyParam(..) |
Def::Label(_) | Def::SelfTy(..) |
Def::AssociatedTy(..) => {
span_bug!(span, "Unexpected definition in \
memory categorization: {:?}", def);
}

Def::Static(_, mutbl) => {
Ok(Rc::new(cmt_ {
id:id,
Expand Down Expand Up @@ -598,7 +589,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
}))
}

Def::Err => bug!("Def::Err in memory categorization")
def => span_bug!(span, "unexpected definition in memory categorization: {:?}", def)
}
}

Expand Down Expand Up @@ -1077,7 +1068,8 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
// alone) because PatKind::Struct can also refer to variants.
let cmt = match self.tcx().expect_def_or_none(pat.id) {
Some(Def::Err) => return Err(()),
Some(Def::Variant(variant_did)) => {
Some(Def::Variant(variant_did)) |
Some(Def::VariantCtor(variant_did, ..)) => {
// univariant enums do not need downcasts
let enum_did = self.tcx().parent_def_id(variant_did).unwrap();
if !self.tcx().lookup_adt_def(enum_did).is_univariant() {
Expand All @@ -1092,11 +1084,11 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
match pat.node {
PatKind::TupleStruct(_, ref subpats, ddpos) => {
let expected_len = match self.tcx().expect_def(pat.id) {
Def::Variant(def_id) => {
Def::VariantCtor(def_id, CtorKind::Fn) => {
let enum_def = self.tcx().parent_def_id(def_id).unwrap();
self.tcx().lookup_adt_def(enum_def).variant_with_id(def_id).fields.len()
}
Def::Struct(..) => {
Def::StructCtor(_, CtorKind::Fn) => {
match self.pat_ty(&pat)?.sty {
ty::TyAdt(adt_def, _) => {
adt_def.struct_variant().fields.len()
Expand Down
15 changes: 3 additions & 12 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,12 +617,8 @@ pub fn check_path<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
&Option<DeprecationEntry>)) {
// Paths in import prefixes may have no resolution.
match tcx.expect_def_or_none(id) {
Some(Def::PrimTy(..)) => {}
Some(Def::SelfTy(..)) => {}
Some(def) => {
maybe_do_stability_check(tcx, def.def_id(), path.span, cb);
}
None => {}
None | Some(Def::PrimTy(..)) | Some(Def::SelfTy(..)) => {}
Some(def) => maybe_do_stability_check(tcx, def.def_id(), path.span, cb)
}
}

Expand All @@ -631,12 +627,7 @@ pub fn check_path_list_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
cb: &mut FnMut(DefId, Span,
&Option<&Stability>,
&Option<DeprecationEntry>)) {
match tcx.expect_def(item.node.id) {
Def::PrimTy(..) => {}
def => {
maybe_do_stability_check(tcx, def.def_id(), item.span, cb);
}
}
maybe_do_stability_check(tcx, tcx.expect_def(item.node.id).def_id(), item.span, cb);
}

pub fn check_pat<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, pat: &hir::Pat,
Expand Down
9 changes: 5 additions & 4 deletions src/librustc/mir/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rustc_data_structures::indexed_vec::{IndexVec, Idx};
use rustc_data_structures::control_flow_graph::dominators::{Dominators, dominators};
use rustc_data_structures::control_flow_graph::{GraphPredecessors, GraphSuccessors};
use rustc_data_structures::control_flow_graph::ControlFlowGraph;
use hir::def::CtorKind;
use hir::def_id::DefId;
use ty::subst::Substs;
use ty::{self, AdtDef, ClosureSubsts, Region, Ty};
Expand Down Expand Up @@ -1140,10 +1141,10 @@ impl<'tcx> Debug for Rvalue<'tcx> {
ppaux::parameterized(fmt, substs, variant_def.did,
ppaux::Ns::Value, &[])?;

match variant_def.kind {
ty::VariantKind::Unit => Ok(()),
ty::VariantKind::Tuple => fmt_tuple(fmt, lvs),
ty::VariantKind::Struct => {
match variant_def.ctor_kind {
CtorKind::Const => Ok(()),
CtorKind::Fn => fmt_tuple(fmt, lvs),
CtorKind::Fictive => {
let mut struct_fmt = fmt.debug_struct("");
for (field, lv) in variant_def.fields.iter().zip(lvs) {
struct_fmt.field(&field.name.as_str(), lv);
Expand Down
Loading