Skip to content

Commit

Permalink
Rollup merge of #64457 - petrochenkov:macunfield, r=matthewjasper
Browse files Browse the repository at this point in the history
def_collector: Do not ICE on attributes on unnamed fields

The primary issue here is that the expansion infra needs to visit a field in isolation, and fields don't know their own indices during expansion, so they have to be kept in some other place (e.g. `struct Definitions`).

Fixes #64385
  • Loading branch information
Centril authored Sep 15, 2019
2 parents b35ebac + c681cf7 commit 8f55245
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 21 deletions.
48 changes: 27 additions & 21 deletions src/librustc/hir/map/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl<'a> DefCollector<'a> {
self.definitions.create_def_with_parent(parent_def, node_id, data, self.expansion, span)
}

pub fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_def: DefIndex, f: F) {
fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_def: DefIndex, f: F) {
let orig_parent_def = std::mem::replace(&mut self.parent_def, parent_def);
f(self);
self.parent_def = orig_parent_def;
Expand Down Expand Up @@ -74,6 +74,22 @@ impl<'a> DefCollector<'a> {
})
}

fn collect_field(&mut self, field: &'a StructField, index: Option<usize>) {
if field.is_placeholder {
self.visit_macro_invoc(field.id);
} else {
let name = field.ident.map(|ident| ident.name)
.or_else(|| index.map(sym::integer))
.unwrap_or_else(|| {
let node_id = NodeId::placeholder_from_expn_id(self.expansion);
sym::integer(self.definitions.placeholder_field_indices[&node_id])
})
.as_interned_str();
let def = self.create_def(field.id, DefPathData::ValueNs(name), field.span);
self.with_parent(def, |this| visit::walk_struct_field(this, field));
}
}

pub fn visit_macro_invoc(&mut self, id: NodeId) {
self.definitions.set_invocation_parent(id.placeholder_to_expn_id(), self.parent_def);
}
Expand Down Expand Up @@ -170,17 +186,14 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
}

fn visit_variant_data(&mut self, data: &'a VariantData) {
// The assumption here is that non-`cfg` macro expansion cannot change field indices.
// It currently holds because only inert attributes are accepted on fields,
// and every such attribute expands into a single field after it's resolved.
for (index, field) in data.fields().iter().enumerate() {
if field.is_placeholder {
self.visit_macro_invoc(field.id);
continue;
self.collect_field(field, Some(index));
if field.is_placeholder && field.ident.is_none() {
self.definitions.placeholder_field_indices.insert(field.id, index);
}
let name = field.ident.map(|ident| ident.name)
.unwrap_or_else(|| sym::integer(index));
let def = self.create_def(field.id,
DefPathData::ValueNs(name.as_interned_str()),
field.span);
self.with_parent(def, |this| visit::walk_struct_field(this, field));
}
}

Expand Down Expand Up @@ -338,16 +351,9 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
}
}

fn visit_struct_field(&mut self, sf: &'a StructField) {
if sf.is_placeholder {
self.visit_macro_invoc(sf.id)
} else {
let name = sf.ident.map(|ident| ident.name)
.unwrap_or_else(|| panic!("don't know the field number in this context"));
let def = self.create_def(sf.id,
DefPathData::ValueNs(name.as_interned_str()),
sf.span);
self.with_parent(def, |this| visit::walk_struct_field(this, sf));
}
// This method is called only when we are visiting an individual field
// after expanding an attribute on it.
fn visit_struct_field(&mut self, field: &'a StructField) {
self.collect_field(field, None);
}
}
2 changes: 2 additions & 0 deletions src/librustc/hir/map/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ pub struct Definitions {
/// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId`
/// we know what parent node that fragment should be attached to thanks to this table.
invocation_parents: FxHashMap<ExpnId, DefIndex>,
/// Indices of unnamed struct or variant fields with unresolved attributes.
pub(super) placeholder_field_indices: NodeMap<usize>,
}

/// A unique identifier that we can use to lookup a definition
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/attributes/unnamed-field-attributes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// check-pass

struct S(
#[rustfmt::skip] u8,
u16,
#[rustfmt::skip] u32,
);

fn main() {}

0 comments on commit 8f55245

Please sign in to comment.