expand: Simplify expansion of derives

src/librustc/hir/map/def_collector.rs

@@ -90,7 +90,7 @@ impl<'a> DefCollector<'a> {
         }
     }
 
-    pub fn visit_macro_invoc(&mut self, id: NodeId) {
+    fn visit_macro_invoc(&mut self, id: NodeId) {
         self.definitions.set_invocation_parent(id.placeholder_to_expn_id(), self.parent_def);
     }
 }

src/librustc_privacy/lib.rs

@@ -880,11 +880,11 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
             self.tcx,
             self.tcx.hir().local_def_id(md.hir_id)
         ).unwrap();
-        let mut module_id = self.tcx.hir().as_local_hir_id(macro_module_def_id).unwrap();
-        if !self.tcx.hir().is_hir_id_module(module_id) {
-            // `module_id` doesn't correspond to a `mod`, return early (#63164).
-            return;
-        }
+        let mut module_id = match self.tcx.hir().as_local_hir_id(macro_module_def_id) {
+            Some(module_id) if self.tcx.hir().is_hir_id_module(module_id) => module_id,
+            // `module_id` doesn't correspond to a `mod`, return early (#63164, #65252).
+            _ => return,
+        };
         let level = if md.vis.node.is_pub() { self.get(module_id) } else { None };
         let new_level = self.update(md.hir_id, level);
         if new_level.is_none() {

src/librustc_resolve/build_reduced_graph.rs

@@ -163,25 +163,15 @@ impl<'a> Resolver<'a> {
         Some(ext)
     }
 
-    // FIXME: `extra_placeholders` should be included into the `fragment` as regular placeholders.
     crate fn build_reduced_graph(
         &mut self,
         fragment: &AstFragment,
-        extra_placeholders: &[NodeId],
         parent_scope: ParentScope<'a>,
     ) -> LegacyScope<'a> {
         let mut def_collector = DefCollector::new(&mut self.definitions, parent_scope.expansion);
         fragment.visit_with(&mut def_collector);
-        for placeholder in extra_placeholders {
-            def_collector.visit_macro_invoc(*placeholder);
-        }
-
         let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope };
         fragment.visit_with(&mut visitor);
-        for placeholder in extra_placeholders {
-            visitor.parent_scope.legacy = visitor.visit_invoc(*placeholder);
-        }
-
         visitor.parent_scope.legacy
     }
 
@@ -1064,8 +1054,17 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         None
     }
 
+    // Mark the given macro as unused unless its name starts with `_`.
+    // Macro uses will remove items from this set, and the remaining
+    // items will be reported as `unused_macros`.
+    fn insert_unused_macro(&mut self, ident: Ident, node_id: NodeId, span: Span) {
+        if !ident.as_str().starts_with("_") {
+            self.r.unused_macros.insert(node_id, span);
+        }
+    }
+
     fn define_macro(&mut self, item: &ast::Item) -> LegacyScope<'a> {
-        let parent_scope = &self.parent_scope;
+        let parent_scope = self.parent_scope;
         let expansion = parent_scope.expansion;
         let (ext, ident, span, is_legacy) = match &item.kind {
             ItemKind::MacroDef(def) => {
@@ -1105,7 +1104,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                             (res, vis, span, expansion, IsMacroExport));
             } else {
                 self.r.check_reserved_macro_name(ident, res);
-                self.r.unused_macros.insert(item.id, span);
+                self.insert_unused_macro(ident, item.id, span);
             }
             LegacyScope::Binding(self.r.arenas.alloc_legacy_binding(LegacyBinding {
                 parent_legacy_scope: parent_scope.legacy, binding, ident
@@ -1114,7 +1113,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
             let module = parent_scope.module;
             let vis = self.resolve_visibility(&item.vis);
             if vis != ty::Visibility::Public {
-                self.r.unused_macros.insert(item.id, span);
+                self.insert_unused_macro(ident, item.id, span);
             }
             self.r.define(module, ident, MacroNS, (res, vis, span, expansion));
             self.parent_scope.legacy

src/librustc_resolve/macros.rs

@@ -108,15 +108,11 @@ impl<'a> base::Resolver for Resolver<'a> {
         });
     }
 
-    // FIXME: `extra_placeholders` should be included into the `fragment` as regular placeholders.
-    fn visit_ast_fragment_with_placeholders(
-        &mut self, expansion: ExpnId, fragment: &AstFragment, extra_placeholders: &[NodeId]
-    ) {
+    fn visit_ast_fragment_with_placeholders(&mut self, expansion: ExpnId, fragment: &AstFragment) {
         // Integrate the new AST fragment into all the definition and module structures.
         // We are inside the `expansion` now, but other parent scope components are still the same.
         let parent_scope = ParentScope { expansion, ..self.invocation_parent_scopes[&expansion] };
-        let output_legacy_scope =
-            self.build_reduced_graph(fragment, extra_placeholders, parent_scope);
+        let output_legacy_scope = self.build_reduced_graph(fragment, parent_scope);
         self.output_legacy_scopes.insert(expansion, output_legacy_scope);
 
         parent_scope.module.unexpanded_invocations.borrow_mut().remove(&expansion);

src/libsyntax_expand/base.rs

@@ -851,8 +851,7 @@ pub trait Resolver {
     fn next_node_id(&mut self) -> NodeId;
 
     fn resolve_dollar_crates(&mut self);
-    fn visit_ast_fragment_with_placeholders(&mut self, expn_id: ExpnId, fragment: &AstFragment,
-                                            extra_placeholders: &[NodeId]);
+    fn visit_ast_fragment_with_placeholders(&mut self, expn_id: ExpnId, fragment: &AstFragment);
     fn register_builtin_macro(&mut self, ident: ast::Ident, ext: SyntaxExtension);
 
     fn expansion_for_ast_pass(

src/libsyntax_expand/expand.rs

@@ -26,7 +26,6 @@ use errors::{Applicability, FatalError};
 use smallvec::{smallvec, SmallVec};
 use syntax_pos::{Span, DUMMY_SP, FileName};
 
-use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::Lrc;
 use std::io::ErrorKind;
 use std::{iter, mem, slice};
@@ -75,6 +74,22 @@ macro_rules! ast_fragments {
         }
 
         impl AstFragment {
+            pub fn add_placeholders(&mut self, placeholders: &[NodeId]) {
+                if placeholders.is_empty() {
+                    return;
+                }
+                match self {
+                    $($(AstFragment::$Kind(ast) => ast.extend(placeholders.iter().flat_map(|id| {
+                        // We are repeating through arguments with `many`, to do that we have to
+                        // 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()
+                    })),)?)*
+                    _ => panic!("unexpected AST fragment kind")
+                }
+            }
+
             pub fn make_opt_expr(self) -> Option<P<ast::Expr>> {
                 match self {
                     AstFragment::OptExpr(expr) => expr,
@@ -342,7 +357,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
         // Unresolved macros produce dummy outputs as a recovery measure.
         invocations.reverse();
         let mut expanded_fragments = Vec::new();
-        let mut all_derive_placeholders: FxHashMap<ExpnId, Vec<_>> = FxHashMap::default();
         let mut undetermined_invocations = Vec::new();
         let (mut progress, mut force) = (false, !self.monotonic);
         loop {
@@ -420,9 +434,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                         self.cx.resolver.add_derives(invoc.expansion_data.id, SpecialDerives::COPY);
                     }
 
-                    let derive_placeholders =
-                        all_derive_placeholders.entry(invoc.expansion_data.id).or_default();
-                    derive_placeholders.reserve(derives.len());
+                    let mut derive_placeholders = Vec::with_capacity(derives.len());
                     invocations.reserve(derives.len());
                     for path in derives {
                         let expn_id = ExpnId::fresh(None);
@@ -438,7 +450,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                     }
                     let fragment = invoc.fragment_kind
                         .expect_from_annotatables(::std::iter::once(item));
-                    self.collect_invocations(fragment, derive_placeholders)
+                    self.collect_invocations(fragment, &derive_placeholders)
                 }
             };
 
@@ -457,10 +469,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
         let mut placeholder_expander = PlaceholderExpander::new(self.cx, self.monotonic);
         while let Some(expanded_fragments) = expanded_fragments.pop() {
             for (expn_id, expanded_fragment) in expanded_fragments.into_iter().rev() {
-                let derive_placeholders =
-                    all_derive_placeholders.remove(&expn_id).unwrap_or_else(Vec::new);
                 placeholder_expander.add(NodeId::placeholder_from_expn_id(expn_id),
-                                         expanded_fragment, derive_placeholders);
+                                         expanded_fragment);
             }
         }
         fragment_with_placeholders.mut_visit_with(&mut placeholder_expander);
@@ -493,13 +503,14 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 monotonic: self.monotonic,
             };
             fragment.mut_visit_with(&mut collector);
+            fragment.add_placeholders(extra_placeholders);
             collector.invocations
         };
 
-        // FIXME: Merge `extra_placeholders` into the `fragment` as regular placeholders.
         if self.monotonic {
             self.cx.resolver.visit_ast_fragment_with_placeholders(
-                self.cx.current_expansion.id, &fragment, extra_placeholders);
+                self.cx.current_expansion.id, &fragment
+            );
         }
 
         (fragment, invocations)

src/libsyntax_expand/lib.rs

@@ -1,4 +1,5 @@
 #![feature(crate_visibility_modifier)]
+#![feature(decl_macro)]
 #![feature(proc_macro_diagnostic)]
 #![feature(proc_macro_internals)]
 #![feature(proc_macro_span)]

src/libsyntax_expand/placeholders.rs

@@ -1,7 +1,7 @@
 use crate::base::ExtCtxt;
 use crate::expand::{AstFragment, AstFragmentKind};
 
-use syntax::ast::{self, NodeId};
+use syntax::ast;
 use syntax::source_map::{DUMMY_SP, dummy_spanned};
 use syntax::tokenstream::TokenStream;
 use syntax::mut_visit::*;
@@ -171,17 +171,8 @@ impl<'a, 'b> PlaceholderExpander<'a, 'b> {
         }
     }
 
-    pub fn add(&mut self, id: ast::NodeId, mut fragment: AstFragment, placeholders: Vec<NodeId>) {
+    pub fn add(&mut self, id: ast::NodeId, mut fragment: AstFragment) {
         fragment.mut_visit_with(self);
-        if let AstFragment::Items(mut items) = fragment {
-            for placeholder in placeholders {
-                match self.remove(placeholder) {
-                    AstFragment::Items(derived_items) => items.extend(derived_items),
-                    _ => unreachable!(),
-                }
-            }
-            fragment = AstFragment::Items(items);
-        }
         self.expanded_fragments.insert(id, fragment);
     }
 

src/test/rustdoc/macro-in-closure.rs

@@ -0,0 +1,9 @@
+// Regression issue for rustdoc ICE encountered in PR #65252.
+
+#![feature(decl_macro)]
+
+fn main() {
+    || {
+        macro m() {}
+    };
+}