Skip to content

Commit

Permalink
Auto merge of #36814 - petrochenkov:def, r=eddyb
Browse files Browse the repository at this point in the history
Refactoring/bugfixing around definitions for struct/variant constructors

 d917c36 separates definitions for struct/variant constructors living in value namespace from struct/variant type definitions.

adfb378 fixes cross-crate resolution of reexports reexporting half-items, like struct constructors without struct type or types without constructor. Such reexports can appear due to glob shadowing.
Resolution now is not affected by the order in which items and reexports are decoded from metadata (cc #31337 (comment)). `try_define` is not used during building reduced graph anymore.
500 lines of this PR are tests for this exotic situation, the remaining line diff count is actually negative! :)

c695d0c (and partially aabf132) moves most of pattern resolution checks from typeck to resolve (except those checking for associated items), uses the same wording for pattern resolution error messages from both typeck and resolve and makes the messages more precise.

11e3524 fixes seemingly incorrectly set `NON_ZERO_SIZED` attributes for struct/variant ctors in const eval.

4586fea eliminates `ty::VariantKind` in favor of `def::CtorKind`. The logic is that variant kinds are irrelevant for types, they make sense only when we deal with constructor functions/constants. Despite that `VariantDefData` still keeps a copy of `CtorKind`, but it's used only for various kinds of pretty-printing (and for storing in metadata).

aabf132 is mostly a cleanup of various impossible or improperly used definitions, and other small definition cleanups.

cc @jseyfried
r? @eddyb
  • Loading branch information
bors authored Oct 4, 2016
2 parents 1a41928 + bc0eabd commit fd065a8
Show file tree
Hide file tree
Showing 70 changed files with 1,031 additions and 613 deletions.
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.
}

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",
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

0 comments on commit fd065a8

Please sign in to comment.