about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMazdak Farrokhzad <twingoow@gmail.com>2019-09-15 02:29:43 +0200
committerGitHub <noreply@github.com>2019-09-15 02:29:43 +0200
commit8f55245fdc7d3ad26e2dff1e61d2b6efa293d3d0 (patch)
treef5a949dbf00869c7a1a5c051a5c42b2a5f32f948
parentb35ebac96102cd12406d9d87827b0838d129c278 (diff)
parentc681cf781b440620aca8cad4be9b76a477fc6c1a (diff)
downloadrust-8f55245fdc7d3ad26e2dff1e61d2b6efa293d3d0.tar.gz
rust-8f55245fdc7d3ad26e2dff1e61d2b6efa293d3d0.zip
Rollup merge of #64457 - petrochenkov:macunfield, r=matthewjasper
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 https://github.com/rust-lang/rust/issues/64385
-rw-r--r--src/librustc/hir/map/def_collector.rs48
-rw-r--r--src/librustc/hir/map/definitions.rs2
-rw-r--r--src/test/ui/attributes/unnamed-field-attributes.rs9
3 files changed, 38 insertions, 21 deletions
diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs
index bffb4df836e..d1cc7a8ce98 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<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;
@@ -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);
     }
@@ -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));
         }
     }
 
@@ -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);
     }
 }
diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs
index 651fe8449ac..187bc593324 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<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
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 00000000000..93f364047e9
--- /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() {}