Auto merge of rust-lang#66669 - petrochenkov:tup2attr, r=matthewjasper

Fix some issues with attributes on unnamed fields

Fixes rust-lang#66487
Fixes rust-lang#66555

Author: bors

src/librustc_resolve/build_reduced_graph.rs

@@ -746,6 +746,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
 
                 // Record field names for error reporting.
                 let field_names = struct_def.fields().iter().map(|field| {
+                    // NOTE: The field may be an expansion placeholder, but expansion sets correct
+                    // visibilities for unnamed field placeholders specifically, so the constructor
+                    // visibility should still be determined correctly.
                     let field_vis = self.resolve_visibility(&field.vis);
                     if ctor_vis.is_at_least(field_vis, &*self.r) {
                         ctor_vis = field_vis;

src/librustc_resolve/def_collector.rs

@@ -80,15 +80,16 @@ impl<'a> DefCollector<'a> {
     }
 
     fn collect_field(&mut self, field: &'a StructField, index: Option<usize>) {
+        let index = |this: &Self| index.unwrap_or_else(|| {
+            let node_id = NodeId::placeholder_from_expn_id(this.expansion);
+            this.definitions.placeholder_field_index(node_id)
+        });
+
         if field.is_placeholder {
+            self.definitions.set_placeholder_field_index(field.id, index(self));
             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_index(node_id))
-                });
+            let name = field.ident.map_or_else(|| sym::integer(index(self)), |ident| ident.name);
             let def = self.create_def(field.id, DefPathData::ValueNs(name), field.span);
             self.with_parent(def, |this| visit::walk_struct_field(this, field));
         }
@@ -190,9 +191,6 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
         // and every such attribute expands into a single field after it's resolved.
         for (index, field) in data.fields().iter().enumerate() {
             self.collect_field(field, Some(index));
-            if field.is_placeholder && field.ident.is_none() {
-                self.definitions.set_placeholder_field_index(field.id, index);
-            }
         }
     }
 

src/libsyntax_expand/expand.rs

@@ -86,7 +86,7 @@ macro_rules! ast_fragments {
                         // mention some macro variable from those arguments even if it's not used.
                         #[cfg_attr(bootstrap, allow(unused_macros))]
                         macro _repeating($flat_map_ast_elt) {}
-                        placeholder(AstFragmentKind::$Kind, *id).$make_ast()
+                        placeholder(AstFragmentKind::$Kind, *id, None).$make_ast()
                     })),)?)*
                     _ => panic!("unexpected AST fragment kind")
                 }
@@ -275,6 +275,23 @@ pub enum InvocationKind {
     },
 }
 
+impl InvocationKind {
+    fn placeholder_visibility(&self) -> Option<ast::Visibility> {
+        // HACK: For unnamed fields placeholders should have the same visibility as the actual
+        // fields because for tuple structs/variants resolve determines visibilities of their
+        // constructor using these field visibilities before attributes on them are are expanded.
+        // The assumption is that the attribute expansion cannot change field visibilities,
+        // and it holds because only inert attributes are supported in this position.
+        match self {
+            InvocationKind::Attr { item: Annotatable::StructField(field), .. } |
+            InvocationKind::Derive { item: Annotatable::StructField(field), .. } |
+            InvocationKind::DeriveContainer { item: Annotatable::StructField(field), .. }
+                if field.ident.is_none() => Some(field.vis.clone()),
+            _ => None,
+        }
+    }
+}
+
 impl Invocation {
     pub fn span(&self) -> Span {
         match &self.kind {
@@ -931,6 +948,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
             _ => None,
         };
         let expn_id = ExpnId::fresh(expn_data);
+        let vis = kind.placeholder_visibility();
         self.invocations.push(Invocation {
             kind,
             fragment_kind,
@@ -940,7 +958,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                 ..self.cx.current_expansion.clone()
             },
         });
-        placeholder(fragment_kind, NodeId::placeholder_from_expn_id(expn_id))
+        placeholder(fragment_kind, NodeId::placeholder_from_expn_id(expn_id), vis)
     }
 
     fn collect_bang(&mut self, mac: ast::Mac, span: Span, kind: AstFragmentKind) -> AstFragment {

src/libsyntax_expand/placeholders.rs

@@ -12,7 +12,8 @@ use smallvec::{smallvec, SmallVec};
 
 use rustc_data_structures::fx::FxHashMap;
 
-pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId) -> AstFragment {
+pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId, vis: Option<ast::Visibility>)
+                   -> AstFragment {
     fn mac_placeholder() -> ast::Mac {
         ast::Mac {
             path: ast::Path { span: DUMMY_SP, segments: Vec::new() },
@@ -26,7 +27,7 @@ pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId) -> AstFragment {
     let ident = ast::Ident::invalid();
     let attrs = Vec::new();
     let generics = ast::Generics::default();
-    let vis = dummy_spanned(ast::VisibilityKind::Inherited);
+    let vis = vis.unwrap_or_else(|| dummy_spanned(ast::VisibilityKind::Inherited));
     let span = DUMMY_SP;
     let expr_placeholder = || P(ast::Expr {
         id, span,

src/test/ui/attributes/unnamed-field-attributes-dup.rs

@@ -0,0 +1,11 @@
+// Duplicate non-builtin attributes can be used on unnamed fields.
+
+// check-pass
+
+struct S (
+    #[rustfmt::skip]
+    #[rustfmt::skip]
+    u8
+);
+
+fn main() {}

src/test/ui/attributes/unnamed-field-attributes-vis.rs

@@ -0,0 +1,11 @@
+// Unnamed fields don't lose their visibility due to non-builtin attributes on them.
+
+// check-pass
+
+mod m {
+    pub struct S(#[rustfmt::skip] pub u8);
+}
+
+fn main() {
+    m::S(0);
+}