From fc9f13e42470da7e3b1e2f870d1719895484292f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 14 Sep 2019 13:56:44 +0300 Subject: [PATCH 1/2] def_collector: Do not ICE on attributes on unnamed fields --- src/librustc/hir/map/def_collector.rs | 15 +++++++++++++-- src/librustc/hir/map/definitions.rs | 2 ++ .../ui/attributes/unnamed-field-attributes.rs | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/attributes/unnamed-field-attributes.rs diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index bffb4df836e3b..b42fbd5c0a1ae 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -170,9 +170,13 @@ 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); + self.definitions.placeholder_field_indices.insert(field.id, index); continue; } let name = field.ident.map(|ident| ident.name) @@ -338,12 +342,19 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { } } + // This method is called only when we are visiting an individual field + // after expanding an attribute on it. 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 name = sf.ident.map_or_else( + || { + let expn_id = NodeId::placeholder_from_expn_id(self.expansion); + sym::integer(self.definitions.placeholder_field_indices[&expn_id]) + }, + |ident| ident.name, + ); let def = self.create_def(sf.id, DefPathData::ValueNs(name.as_interned_str()), sf.span); diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index 651fe8449ac93..187bc59332460 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -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, + /// Indices of unnamed struct or variant fields with unresolved attributes. + pub(super) placeholder_field_indices: NodeMap, } /// A unique identifier that we can use to lookup a definition diff --git a/src/test/ui/attributes/unnamed-field-attributes.rs b/src/test/ui/attributes/unnamed-field-attributes.rs new file mode 100644 index 0000000000000..93f364047e9a5 --- /dev/null +++ b/src/test/ui/attributes/unnamed-field-attributes.rs @@ -0,0 +1,9 @@ +// check-pass + +struct S( + #[rustfmt::skip] u8, + u16, + #[rustfmt::skip] u32, +); + +fn main() {} From c681cf781b440620aca8cad4be9b76a477fc6c1a Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 14 Sep 2019 17:36:27 +0300 Subject: [PATCH 2/2] def_collector: Factor out common field handling code --- src/librustc/hir/map/def_collector.rs | 47 ++++++++++++--------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index b42fbd5c0a1ae..d1cc7a8ce988f 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -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(&mut self, parent_def: DefIndex, f: F) { + fn with_parent(&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; @@ -74,6 +74,22 @@ impl<'a> DefCollector<'a> { }) } + fn collect_field(&mut self, field: &'a StructField, index: Option) { + 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); } @@ -174,17 +190,10 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { // 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); + self.collect_field(field, Some(index)); + if field.is_placeholder && field.ident.is_none() { self.definitions.placeholder_field_indices.insert(field.id, index); - continue; } - 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)); } } @@ -344,21 +353,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { // This method is called only when we are visiting an individual field // after expanding an attribute on it. - 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_or_else( - || { - let expn_id = NodeId::placeholder_from_expn_id(self.expansion); - sym::integer(self.definitions.placeholder_field_indices[&expn_id]) - }, - |ident| ident.name, - ); - 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)); - } + fn visit_struct_field(&mut self, field: &'a StructField) { + self.collect_field(field, None); } }