Skip to content

Commit

Permalink
Auto merge of #28816 - petrochenkov:unistruct, r=nrc
Browse files Browse the repository at this point in the history
This patch uses the same data structures for structs and enum variants in AST and HIR. These changes in data structures lead to noticeable simplification in most of code dealing with them.
I didn't touch the top level, i.e. `ItemStruct` is still `ItemStruct` and not `ItemEnum` with one variant, like in the type checker.
As part of this patch, structures and variants get the `kind` field making distinction between "normal" structs, tuple structs and unit structs explicit instead of relying on the number of fields and presence of constructor `NodeId`. In particular, we can now distinguish empty tuple structs from unit structs, which was impossible before! Comprehensive tests for empty structs are added and some improvements to empty struct feature gates are made. Some tests don't pass due to issue #28692 , they are still there for completeness, but are commented out.
This patch fixes issue mentioned in #16819 (comment), now emptiness of tuple structs is checked after expansion.
It also touches #28750 by providing span for visit_struct_def
cc #28336

r? @nrc
  • Loading branch information
bors committed Oct 14, 2015
2 parents c0dc2cb + 607b8c3 commit 2939666
Show file tree
Hide file tree
Showing 64 changed files with 845 additions and 894 deletions.
2 changes: 1 addition & 1 deletion src/doc/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -2357,7 +2357,7 @@ The currently implemented features of the reference compiler are:
terms of encapsulation).
* - `default_type_parameter_fallback` - Allows type parameter defaults to
influence type inference.
* - `braced_empty_structs` - Allows use of empty structs with braces.
* - `braced_empty_structs` - Allows use of empty structs and enum variants with braces.

If a feature is promoted to a language feature, then all existing programs will
start to receive compilation warnings about `#![feature]` directives which enabled
Expand Down
29 changes: 9 additions & 20 deletions src/librustc/front/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,40 +134,29 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
ItemEnum(ref enum_definition, _) => {
for v in &enum_definition.variants {
let variant_def_index =
self.insert_def(v.node.id,
self.insert_def(v.node.data.id(),
NodeVariant(&**v),
DefPathData::EnumVariant(v.node.name));

match v.node.kind {
TupleVariantKind(ref args) => {
for arg in args {
self.create_def_with_parent(Some(variant_def_index),
arg.id,
DefPathData::PositionalField);
}
}
StructVariantKind(ref def) => {
for field in &def.fields {
self.create_def_with_parent(
Some(variant_def_index),
field.node.id,
DefPathData::Field(field.node.kind));
}
}
for field in v.node.data.fields() {
self.create_def_with_parent(
Some(variant_def_index),
field.node.id,
DefPathData::Field(field.node.kind));
}
}
}
ItemForeignMod(..) => {
}
ItemStruct(ref struct_def, _) => {
// If this is a tuple-like struct, register the constructor.
if let Some(ctor_id) = struct_def.ctor_id {
self.insert_def(ctor_id,
if !struct_def.is_struct() {
self.insert_def(struct_def.id(),
NodeStructCtor(&**struct_def),
DefPathData::StructCtor);
}

for field in &struct_def.fields {
for field in struct_def.fields() {
self.create_def(field.node.id, DefPathData::Field(field.node.kind));
}
}
Expand Down
15 changes: 8 additions & 7 deletions src/librustc/front/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub enum Node<'ast> {
NodeBlock(&'ast Block),

/// NodeStructCtor represents a tuple struct.
NodeStructCtor(&'ast StructDef),
NodeStructCtor(&'ast VariantData),

NodeLifetime(&'ast Lifetime),
NodeTyParam(&'ast TyParam)
Expand All @@ -149,7 +149,7 @@ pub enum MapEntry<'ast> {
EntryLocal(NodeId, &'ast Pat),
EntryPat(NodeId, &'ast Pat),
EntryBlock(NodeId, &'ast Block),
EntryStructCtor(NodeId, &'ast StructDef),
EntryStructCtor(NodeId, &'ast VariantData),
EntryLifetime(NodeId, &'ast Lifetime),
EntryTyParam(NodeId, &'ast TyParam),

Expand Down Expand Up @@ -471,18 +471,19 @@ impl<'ast> Map<'ast> {
}
}

pub fn expect_struct(&self, id: NodeId) -> &'ast StructDef {
pub fn expect_struct(&self, id: NodeId) -> &'ast VariantData {
match self.find(id) {
Some(NodeItem(i)) => {
match i.node {
ItemStruct(ref struct_def, _) => &**struct_def,
ItemStruct(ref struct_def, _) => struct_def,
_ => panic!("struct ID bound to non-struct")
}
}
Some(NodeVariant(variant)) => {
match variant.node.kind {
StructVariantKind(ref struct_def) => &**struct_def,
_ => panic!("struct ID bound to enum variant that isn't struct-like"),
if variant.node.data.is_struct() {
&variant.node.data
} else {
panic!("struct ID bound to enum variant that isn't struct-like")
}
}
_ => panic!(format!("expected struct, found {}", self.node_to_string(id))),
Expand Down
30 changes: 16 additions & 14 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,14 +661,15 @@ impl<'a, 'tcx, 'v> hir_visit::Visitor<'v> for LateContext<'a, 'tcx> {
hir_visit::walk_fn(self, fk, decl, body, span);
}

fn visit_struct_def(&mut self,
s: &hir::StructDef,
fn visit_variant_data(&mut self,
s: &hir::VariantData,
name: ast::Name,
g: &hir::Generics,
id: ast::NodeId) {
run_lints!(self, check_struct_def, late_passes, s, name, g, id);
item_id: ast::NodeId,
_: Span) {
run_lints!(self, check_struct_def, late_passes, s, name, g, item_id);
hir_visit::walk_struct_def(self, s);
run_lints!(self, check_struct_def_post, late_passes, s, name, g, id);
run_lints!(self, check_struct_def_post, late_passes, s, name, g, item_id);
}

fn visit_struct_field(&mut self, s: &hir::StructField) {
Expand All @@ -678,10 +679,10 @@ impl<'a, 'tcx, 'v> hir_visit::Visitor<'v> for LateContext<'a, 'tcx> {
})
}

fn visit_variant(&mut self, v: &hir::Variant, g: &hir::Generics) {
fn visit_variant(&mut self, v: &hir::Variant, g: &hir::Generics, item_id: ast::NodeId) {
self.with_lint_attrs(&v.node.attrs, |cx| {
run_lints!(cx, check_variant, late_passes, v, g);
hir_visit::walk_variant(cx, v, g);
hir_visit::walk_variant(cx, v, g, item_id);
run_lints!(cx, check_variant_post, late_passes, v, g);
})
}
Expand Down Expand Up @@ -810,14 +811,15 @@ impl<'a, 'v> ast_visit::Visitor<'v> for EarlyContext<'a> {
ast_visit::walk_fn(self, fk, decl, body, span);
}

fn visit_struct_def(&mut self,
s: &ast::StructDef,
fn visit_variant_data(&mut self,
s: &ast::VariantData,
ident: ast::Ident,
g: &ast::Generics,
id: ast::NodeId) {
run_lints!(self, check_struct_def, early_passes, s, ident, g, id);
item_id: ast::NodeId,
_: Span) {
run_lints!(self, check_struct_def, early_passes, s, ident, g, item_id);
ast_visit::walk_struct_def(self, s);
run_lints!(self, check_struct_def_post, early_passes, s, ident, g, id);
run_lints!(self, check_struct_def_post, early_passes, s, ident, g, item_id);
}

fn visit_struct_field(&mut self, s: &ast::StructField) {
Expand All @@ -827,10 +829,10 @@ impl<'a, 'v> ast_visit::Visitor<'v> for EarlyContext<'a> {
})
}

fn visit_variant(&mut self, v: &ast::Variant, g: &ast::Generics) {
fn visit_variant(&mut self, v: &ast::Variant, g: &ast::Generics, item_id: ast::NodeId) {
self.with_lint_attrs(&v.node.attrs, |cx| {
run_lints!(cx, check_variant, early_passes, v, g);
ast_visit::walk_variant(cx, v, g);
ast_visit::walk_variant(cx, v, g, item_id);
run_lints!(cx, check_variant_post, early_passes, v, g);
})
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ pub trait LateLintPass: LintPass {
fn check_trait_item(&mut self, _: &LateContext, _: &hir::TraitItem) { }
fn check_impl_item(&mut self, _: &LateContext, _: &hir::ImplItem) { }
fn check_struct_def(&mut self, _: &LateContext,
_: &hir::StructDef, _: ast::Name, _: &hir::Generics, _: ast::NodeId) { }
_: &hir::VariantData, _: ast::Name, _: &hir::Generics, _: ast::NodeId) { }
fn check_struct_def_post(&mut self, _: &LateContext,
_: &hir::StructDef, _: ast::Name, _: &hir::Generics, _: ast::NodeId) { }
_: &hir::VariantData, _: ast::Name, _: &hir::Generics, _: ast::NodeId) { }
fn check_struct_field(&mut self, _: &LateContext, _: &hir::StructField) { }
fn check_variant(&mut self, _: &LateContext, _: &hir::Variant, _: &hir::Generics) { }
fn check_variant_post(&mut self, _: &LateContext, _: &hir::Variant, _: &hir::Generics) { }
Expand Down Expand Up @@ -192,9 +192,9 @@ pub trait EarlyLintPass: LintPass {
fn check_trait_item(&mut self, _: &EarlyContext, _: &ast::TraitItem) { }
fn check_impl_item(&mut self, _: &EarlyContext, _: &ast::ImplItem) { }
fn check_struct_def(&mut self, _: &EarlyContext,
_: &ast::StructDef, _: ast::Ident, _: &ast::Generics, _: ast::NodeId) { }
_: &ast::VariantData, _: ast::Ident, _: &ast::Generics, _: ast::NodeId) { }
fn check_struct_def_post(&mut self, _: &EarlyContext,
_: &ast::StructDef, _: ast::Ident, _: &ast::Generics, _: ast::NodeId) { }
_: &ast::VariantData, _: ast::Ident, _: &ast::Generics, _: ast::NodeId) { }
fn check_struct_field(&mut self, _: &EarlyContext, _: &ast::StructField) { }
fn check_variant(&mut self, _: &EarlyContext, _: &ast::Variant, _: &ast::Generics) { }
fn check_variant_post(&mut self, _: &EarlyContext, _: &ast::Variant, _: &ast::Generics) { }
Expand Down
23 changes: 9 additions & 14 deletions src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ fn encode_enum_variant_info<'a, 'tcx>(ecx: &EncodeContext<'a, 'tcx>,
let vid = variant.did;
let variant_node_id = ecx.local_id(vid);

if let ty::VariantKind::Dict = variant.kind() {
if let ty::VariantKind::Struct = variant.kind() {
// tuple-like enum variant fields aren't really items so
// don't try to encode them.
for field in &variant.fields {
Expand All @@ -328,7 +328,7 @@ fn encode_enum_variant_info<'a, 'tcx>(ecx: &EncodeContext<'a, 'tcx>,
encode_def_id_and_key(ecx, rbml_w, vid);
encode_family(rbml_w, match variant.kind() {
ty::VariantKind::Unit | ty::VariantKind::Tuple => 'v',
ty::VariantKind::Dict => 'V'
ty::VariantKind::Struct => 'V'
});
encode_name(rbml_w, variant.name);
encode_parent_item(rbml_w, ecx.tcx.map.local_def_id(id));
Expand Down Expand Up @@ -381,12 +381,8 @@ fn each_auxiliary_node_id<F>(item: &hir::Item, callback: F) -> bool where
match item.node {
hir::ItemStruct(ref struct_def, _) => {
// If this is a newtype struct, return the constructor.
match struct_def.ctor_id {
Some(ctor_id) if !struct_def.fields.is_empty() &&
struct_def.fields[0].node.kind.is_unnamed() => {
continue_ = callback(ctor_id);
}
_ => {}
if struct_def.is_tuple() {
continue_ = callback(struct_def.id());
}
}
_ => {}
Expand Down Expand Up @@ -1023,7 +1019,7 @@ fn encode_info_for_item<'a, 'tcx>(ecx: &EncodeContext<'a, 'tcx>,
encode_attributes(rbml_w, &item.attrs);
encode_repr_attrs(rbml_w, ecx, &item.attrs);
for v in &enum_definition.variants {
encode_variant_id(rbml_w, ecx.tcx.map.local_def_id(v.node.id));
encode_variant_id(rbml_w, ecx.tcx.map.local_def_id(v.node.data.id()));
}
encode_inlined_item(ecx, rbml_w, InlinedItemRef::Item(item));
encode_path(rbml_w, path);
Expand Down Expand Up @@ -1072,8 +1068,8 @@ fn encode_info_for_item<'a, 'tcx>(ecx: &EncodeContext<'a, 'tcx>,
// Encode inherent implementations for this structure.
encode_inherent_implementations(ecx, rbml_w, def_id);

if let Some(ctor_id) = struct_def.ctor_id {
let ctor_did = ecx.tcx.map.local_def_id(ctor_id);
if !struct_def.is_struct() {
let ctor_did = ecx.tcx.map.local_def_id(struct_def.id());
rbml_w.wr_tagged_u64(tag_items_data_item_struct_ctor,
def_to_u64(ctor_did));
}
Expand All @@ -1085,9 +1081,8 @@ fn encode_info_for_item<'a, 'tcx>(ecx: &EncodeContext<'a, 'tcx>,
}

// If this is a tuple-like struct, encode the type of the constructor.
if let Some(ctor_id) = struct_def.ctor_id {
encode_info_for_struct_ctor(ecx, rbml_w, item.name,
ctor_id, index, item.id);
if !struct_def.is_struct() {
encode_info_for_struct_ctor(ecx, rbml_w, item.name, struct_def.id(), index, item.id);
}
}
hir::ItemDefaultImpl(unsafety, _) => {
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/middle/astencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1315,17 +1315,17 @@ fn copy_item_types(dcx: &DecodeContext, ii: &InlinedItem, orig_did: DefId) {
def.variants.iter().zip(orig_def.variants.iter())
{
debug!("astencode: copying variant {:?} => {:?}",
orig_variant.did, i_variant.node.id);
copy_item_type(dcx, i_variant.node.id, orig_variant.did);
orig_variant.did, i_variant.node.data.id());
copy_item_type(dcx, i_variant.node.data.id(), orig_variant.did);
}
}
hir::ItemStruct(ref def, _) => {
if let Some(ctor_id) = def.ctor_id {
if !def.is_struct() {
let ctor_did = dcx.tcx.lookup_adt_def(orig_did)
.struct_variant().did;
debug!("astencode: copying ctor {:?} => {:?}", ctor_did,
ctor_id);
copy_item_type(dcx, ctor_id, ctor_did);
def.id());
copy_item_type(dcx, def.id(), ctor_did);
}
}
_ => {}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ fn construct_witness<'a,'tcx>(cx: &MatchCheckCtxt<'a,'tcx>, ctor: &Constructor,

ty::TyEnum(adt, _) | ty::TyStruct(adt, _) => {
let v = adt.variant_of_ctor(ctor);
if let VariantKind::Dict = v.kind() {
if let VariantKind::Struct = v.kind() {
let field_pats: Vec<_> = v.fields.iter()
.zip(pats)
.filter(|&(_, ref pat)| pat.node != hir::PatWild(hir::PatWildSingle))
Expand Down
17 changes: 9 additions & 8 deletions src/librustc/middle/check_static_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckCrateVisitor<'a, 'ast> {
let mut recursion_visitor =
CheckItemRecursionVisitor::new(self, &variant.span);
recursion_visitor.populate_enum_discriminants(enum_def);
recursion_visitor.visit_variant(variant, generics);
recursion_visitor.visit_variant(variant, generics, it.id);
}
}
}
Expand Down Expand Up @@ -168,7 +168,7 @@ impl<'a, 'ast: 'a> CheckItemRecursionVisitor<'a, 'ast> {
let mut discriminant_map = self.discriminant_map.borrow_mut();
match enum_definition.variants.first() {
None => { return; }
Some(variant) if discriminant_map.contains_key(&variant.node.id) => {
Some(variant) if discriminant_map.contains_key(&variant.node.data.id()) => {
return;
}
_ => {}
Expand All @@ -177,7 +177,7 @@ impl<'a, 'ast: 'a> CheckItemRecursionVisitor<'a, 'ast> {
// Go through all the variants.
let mut variant_stack: Vec<ast::NodeId> = Vec::new();
for variant in enum_definition.variants.iter().rev() {
variant_stack.push(variant.node.id);
variant_stack.push(variant.node.data.id());
// When we find an expression, every variant currently on the stack
// is affected by that expression.
if let Some(ref expr) = variant.node.disr_expr {
Expand All @@ -201,14 +201,14 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
}

fn visit_enum_def(&mut self, enum_definition: &'ast hir::EnumDef,
generics: &'ast hir::Generics) {
generics: &'ast hir::Generics, item_id: ast::NodeId) {
self.populate_enum_discriminants(enum_definition);
visit::walk_enum_def(self, enum_definition, generics);
visit::walk_enum_def(self, enum_definition, generics, item_id);
}

fn visit_variant(&mut self, variant: &'ast hir::Variant,
_: &'ast hir::Generics) {
let variant_id = variant.node.id;
_: &'ast hir::Generics, _: ast::NodeId) {
let variant_id = variant.node.data.id();
let maybe_expr;
if let Some(get_expr) = self.discriminant_map.borrow().get(&variant_id) {
// This is necessary because we need to let the `discriminant_map`
Expand Down Expand Up @@ -269,9 +269,10 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
self.ast_map.expect_item(enum_node_id).node
{
self.populate_enum_discriminants(enum_def);
let enum_id = self.ast_map.as_local_node_id(enum_id).unwrap();
let variant_id = self.ast_map.as_local_node_id(variant_id).unwrap();
let variant = self.ast_map.expect_variant(variant_id);
self.visit_variant(variant, generics);
self.visit_variant(variant, generics, enum_id);
} else {
self.sess.span_bug(e.span,
"`check_static_recursion` found \
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ fn lookup_variant_by_id<'a>(tcx: &'a ty::ctxt,
fn variant_expr<'a>(variants: &'a [P<hir::Variant>], id: ast::NodeId)
-> Option<&'a Expr> {
for variant in variants {
if variant.node.id == id {
if variant.node.data.id() == id {
return variant.node.disr_expr.as_ref().map(|e| &**e);
}
}
Expand Down
Loading

0 comments on commit 2939666

Please sign in to comment.