Auto merge of #68814 - Aaron1011:fix/proc-macro-order-two, r=petrochenkov

bors · bors · commit 2a0d1cbd4674 · 2020-02-16T01:46:05.000Z
Record proc macro harness order for use during metadata deserialization Fixes #68690 When we generate the proc macro harness, we now explicitly recorder the order in which we generate entries. We then use this ordering data to deserialize the correct proc-macro-data from the crate metadata.
diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs
@@ -140,6 +140,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
                 trait_impls: _,
                 body_ids: _,
                 modules: _,
+                proc_macros: _,
             } = *krate;
 
             alloc_hir_dep_nodes(
diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs
@@ -530,6 +530,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let module = self.lower_mod(&c.module);
         let attrs = self.lower_attrs(&c.attrs);
         let body_ids = body_ids(&self.bodies);
+        let proc_macros = c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id]).collect();
 
         self.resolver.definitions().init_node_id_to_hir_id_mapping(self.node_id_to_hir_id);
 
@@ -546,6 +547,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             body_ids,
             trait_impls: self.trait_impls,
             modules: self.modules,
+            proc_macros,
         }
     }
 
diff --git a/src/librustc_builtin_macros/proc_macro_harness.rs b/src/librustc_builtin_macros/proc_macro_harness.rs
@@ -8,13 +8,15 @@ use rustc_span::hygiene::AstPass;
 use rustc_span::symbol::{kw, sym};
 use rustc_span::{Span, DUMMY_SP};
 use smallvec::smallvec;
-use syntax::ast::{self, Ident};
+use std::cell::RefCell;
+use syntax::ast::{self, Ident, NodeId};
 use syntax::attr;
 use syntax::expand::is_proc_macro_attr;
 use syntax::ptr::P;
 use syntax::visit::{self, Visitor};
 
 struct ProcMacroDerive {
+    id: NodeId,
     trait_name: ast::Name,
     function_name: Ident,
     span: Span,
@@ -27,6 +29,7 @@ enum ProcMacroDefType {
 }
 
 struct ProcMacroDef {
+    id: NodeId,
     function_name: Ident,
     span: Span,
     def_type: ProcMacroDefType,
@@ -69,9 +72,6 @@ pub fn inject(
     if has_proc_macro_decls || is_proc_macro_crate {
         visit::walk_crate(&mut collect, &krate);
     }
-    // NOTE: If you change the order of macros in this vec
-    // for any reason, you must also update 'raw_proc_macro'
-    // in src/librustc_metadata/decoder.rs
     let macros = collect.macros;
 
     if !is_proc_macro_crate {
@@ -86,7 +86,8 @@ pub fn inject(
         return krate;
     }
 
-    krate.module.items.push(mk_decls(&mut cx, &macros));
+    let decls = mk_decls(&mut krate, &mut cx, &macros);
+    krate.module.items.push(decls);
 
     krate
 }
@@ -181,6 +182,7 @@ impl<'a> CollectProcMacros<'a> {
 
         if self.in_root && item.vis.node.is_pub() {
             self.macros.push(ProcMacro::Derive(ProcMacroDerive {
+                id: item.id,
                 span: item.span,
                 trait_name: trait_ident.name,
                 function_name: item.ident,
@@ -200,6 +202,7 @@ impl<'a> CollectProcMacros<'a> {
     fn collect_attr_proc_macro(&mut self, item: &'a ast::Item) {
         if self.in_root && item.vis.node.is_pub() {
             self.macros.push(ProcMacro::Def(ProcMacroDef {
+                id: item.id,
                 span: item.span,
                 function_name: item.ident,
                 def_type: ProcMacroDefType::Attr,
@@ -218,6 +221,7 @@ impl<'a> CollectProcMacros<'a> {
     fn collect_bang_proc_macro(&mut self, item: &'a ast::Item) {
         if self.in_root && item.vis.node.is_pub() {
             self.macros.push(ProcMacro::Def(ProcMacroDef {
+                id: item.id,
                 span: item.span,
                 function_name: item.ident,
                 def_type: ProcMacroDefType::Bang,
@@ -357,7 +361,15 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
 //              // ...
 //          ];
 //      }
-fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
+fn mk_decls(
+    ast_krate: &mut ast::Crate,
+    cx: &mut ExtCtxt<'_>,
+    macros: &[ProcMacro],
+) -> P<ast::Item> {
+    // We're the ones filling in this Vec,
+    // so it should be empty to start with
+    assert!(ast_krate.proc_macros.is_empty());
+
     let expn_id = cx.resolver.expansion_for_ast_pass(
         DUMMY_SP,
         AstPass::ProcMacroHarness,
@@ -376,6 +388,12 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
     let attr = cx.ident_of("attr", span);
     let bang = cx.ident_of("bang", span);
 
+    let krate_ref = RefCell::new(ast_krate);
+
+    // We add NodeIds to 'krate.proc_macros' in the order
+    // that we generate expressions. The position of each NodeId
+    // in the 'proc_macros' Vec corresponds to its position
+    // in the static array that will be generated
     let decls = {
         let local_path =
             |sp: Span, name| cx.expr_path(cx.path(sp.with_ctxt(span.ctxt()), vec![name]));
@@ -385,19 +403,26 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
         macros
             .iter()
             .map(|m| match m {
-                ProcMacro::Derive(cd) => cx.expr_call(
-                    span,
-                    proc_macro_ty_method_path(custom_derive),
-                    vec![
-                        cx.expr_str(cd.span, cd.trait_name),
-                        cx.expr_vec_slice(
-                            span,
-                            cd.attrs.iter().map(|&s| cx.expr_str(cd.span, s)).collect::<Vec<_>>(),
-                        ),
-                        local_path(cd.span, cd.function_name),
-                    ],
-                ),
+                ProcMacro::Derive(cd) => {
+                    krate_ref.borrow_mut().proc_macros.push(cd.id);
+                    cx.expr_call(
+                        span,
+                        proc_macro_ty_method_path(custom_derive),
+                        vec![
+                            cx.expr_str(cd.span, cd.trait_name),
+                            cx.expr_vec_slice(
+                                span,
+                                cd.attrs
+                                    .iter()
+                                    .map(|&s| cx.expr_str(cd.span, s))
+                                    .collect::<Vec<_>>(),
+                            ),
+                            local_path(cd.span, cd.function_name),
+                        ],
+                    )
+                }
                 ProcMacro::Def(ca) => {
+                    krate_ref.borrow_mut().proc_macros.push(ca.id);
                     let ident = match ca.def_type {
                         ProcMacroDefType::Attr => attr,
                         ProcMacroDefType::Bang => bang,
diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs
@@ -635,6 +635,9 @@ pub struct Crate<'hir> {
     /// A list of modules written out in the order in which they
     /// appear in the crate. This includes the main crate module.
     pub modules: BTreeMap<HirId, ModuleItems>,
+    /// A list of proc macro HirIds, written out in the order in which
+    /// they are declared in the static array generated by proc_macro_harness.
+    pub proc_macros: Vec<HirId>,
 }
 
 impl Crate<'hir> {
diff --git a/src/librustc_metadata/rmeta/decoder.rs b/src/librustc_metadata/rmeta/decoder.rs
@@ -637,10 +637,6 @@ impl<'a, 'tcx> CrateMetadata {
     fn raw_proc_macro(&self, id: DefIndex) -> &ProcMacro {
         // DefIndex's in root.proc_macro_data have a one-to-one correspondence
         // with items in 'raw_proc_macros'.
-        // NOTE: If you update the order of macros in 'proc_macro_data' for any reason,
-        // you must also update src/librustc_builtin_macros/proc_macro_harness.rs
-        // Failing to do so will result in incorrect data being associated
-        // with proc macros when deserialized.
         let pos = self.root.proc_macro_data.unwrap().decode(self).position(|i| i == id).unwrap();
         &self.raw_proc_macros.unwrap()[pos]
     }
diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs
@@ -34,7 +34,6 @@ use std::path::Path;
 use std::u32;
 use syntax::ast;
 use syntax::attr;
-use syntax::expand::is_proc_macro_attr;
 
 use rustc_hir as hir;
 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
@@ -1328,13 +1327,7 @@ impl EncodeContext<'tcx> {
         let is_proc_macro = self.tcx.sess.crate_types.borrow().contains(&CrateType::ProcMacro);
         if is_proc_macro {
             let tcx = self.tcx;
-            Some(self.lazy(tcx.hir().krate().items.values().filter_map(|item| {
-                if item.attrs.iter().any(|attr| is_proc_macro_attr(attr)) {
-                    Some(item.hir_id.owner)
-                } else {
-                    None
-                }
-            })))
+            Some(self.lazy(tcx.hir().krate().proc_macros.iter().map(|p| p.owner)))
         } else {
             None
         }
diff --git a/src/librustc_parse/parser/module.rs b/src/librustc_parse/parser/module.rs
@@ -35,6 +35,8 @@ impl<'a> Parser<'a> {
             attrs: self.parse_inner_attributes()?,
             module: self.parse_mod_items(&token::Eof, lo)?,
             span: lo.to(self.token.span),
+            // Filled in by proc_macro_harness::inject()
+            proc_macros: Vec::new(),
         });
         krate
     }
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
@@ -429,6 +429,13 @@ pub struct Crate {
     pub module: Mod,
     pub attrs: Vec<Attribute>,
     pub span: Span,
+    /// The order of items in the HIR is unrelated to the order of
+    /// items in the AST. However, we generate proc macro harnesses
+    /// based on the AST order, and later refer to these harnesses
+    /// from the HIR. This field keeps track of the order in which
+    /// we generated proc macros harnesses, so that we can map
+    /// HIR proc macros items back to their harness items.
+    pub proc_macros: Vec<NodeId>,
 }
 
 /// Possible values inside of compile-time attribute lists.
diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs
@@ -989,7 +989,7 @@ pub fn noop_visit_mod<T: MutVisitor>(Mod { inner, items, inline: _ }: &mut Mod,
 }
 
 pub fn noop_visit_crate<T: MutVisitor>(krate: &mut Crate, vis: &mut T) {
-    visit_clobber(krate, |Crate { module, attrs, span }| {
+    visit_clobber(krate, |Crate { module, attrs, span, proc_macros }| {
         let item = P(Item {
             ident: Ident::invalid(),
             attrs,
@@ -1004,11 +1004,11 @@ pub fn noop_visit_crate<T: MutVisitor>(krate: &mut Crate, vis: &mut T) {
         let len = items.len();
         if len == 0 {
             let module = Mod { inner: span, items: vec![], inline: true };
-            Crate { module, attrs: vec![], span }
+            Crate { module, attrs: vec![], span, proc_macros }
         } else if len == 1 {
             let Item { attrs, span, kind, .. } = items.into_iter().next().unwrap().into_inner();
             match kind {
-                ItemKind::Mod(module) => Crate { module, attrs, span },
+                ItemKind::Mod(module) => Crate { module, attrs, span, proc_macros },
                 _ => panic!("visitor converted a module to not a module"),
             }
         } else {
diff --git a/src/test/rustdoc/inline_cross/auxiliary/proc_macro.rs b/src/test/rustdoc/inline_cross/auxiliary/proc_macro.rs
@@ -9,6 +9,19 @@ extern crate proc_macro;
 
 use proc_macro::TokenStream;
 
+macro_rules! make_attr_macro {
+    ($name:ident) => {
+        /// Generated doc comment
+        #[proc_macro_attribute]
+        pub fn $name(args: TokenStream, input: TokenStream) -> TokenStream {
+            panic!()
+        }
+    }
+}
+
+make_attr_macro!(first_attr);
+make_attr_macro!(second_attr);
+
 /// a proc-macro that swallows its input and does nothing.
 #[proc_macro]
 pub fn some_proc_macro(_input: TokenStream) -> TokenStream {
diff --git a/src/test/rustdoc/inline_cross/proc_macro.rs b/src/test/rustdoc/inline_cross/proc_macro.rs
@@ -26,3 +26,11 @@ pub use some_macros::some_proc_attr;
 // @has proc_macro/derive.SomeDerive.html
 // @has - 'a derive attribute that adds nothing to its input.'
 pub use some_macros::SomeDerive;
+
+// @has proc_macro/attr.first_attr.html
+// @has - 'Generated doc comment'
+pub use some_macros::first_attr;
+
+// @has proc_macro/attr.second_attr.html
+// @has - 'Generated doc comment'
+pub use some_macros::second_attr;
diff --git a/src/test/ui/ast-json/ast-json-output.stdout b/src/test/ui/ast-json/ast-json-output.stdout
@@ -1 +1 @@
-{"module":{"inner":{"lo":0,"hi":0},"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"node":"Inherited","span":{"lo":0,"hi":0}},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":{"_field0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["extern",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["core",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":"Semi","span":{"lo":0,"hi":0}}]},"NonJoint"]]}}],"inline":true},"attrs":[],"span":{"lo":0,"hi":0}}
+{"module":{"inner":{"lo":0,"hi":0},"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"node":"Inherited","span":{"lo":0,"hi":0}},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":{"_field0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["extern",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["core",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":"Semi","span":{"lo":0,"hi":0}}]},"NonJoint"]]}}],"inline":true},"attrs":[],"span":{"lo":0,"hi":0},"proc_macros":[]}

Original file line number	Diff line number	Diff line change
`@@ -530,6 +530,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {`
`530`	`530`	`let module = self.lower_mod(&c.module);`
`531`	`531`	`let attrs = self.lower_attrs(&c.attrs);`
`532`	`532`	`let body_ids = body_ids(&self.bodies);`
	`533`	`+ let proc_macros = c.proc_macros.iter().map(\|id\| self.node_id_to_hir_id[*id]).collect();`
`533`	`534`
`534`	`535`	`self.resolver.definitions().init_node_id_to_hir_id_mapping(self.node_id_to_hir_id);`
`535`	`536`
`@@ -546,6 +547,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {`
`546`	`547`	`body_ids,`
`547`	`548`	`trait_impls: self.trait_impls,`
`548`	`549`	`modules: self.modules,`
	`550`	`+ proc_macros,`
`549`	`551`	`}`
`550`	`552`	`}`
`551`	`553`
Original file line number	Diff line number	Diff line change
`@@ -635,6 +635,9 @@ pub struct Crate<'hir> {`
`635`	`635`	`/// A list of modules written out in the order in which they`
`636`	`636`	`/// appear in the crate. This includes the main crate module.`
`637`	`637`	`pub modules: BTreeMap<HirId, ModuleItems>,`
	`638`	`+ /// A list of proc macro HirIds, written out in the order in which`
	`639`	`+ /// they are declared in the static array generated by proc_macro_harness.`
	`640`	`+ pub proc_macros: Vec<HirId>,`
`638`	`641`	`}`
`639`	`642`
`640`	`643`	`impl Crate<'hir> {`
Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,8 @@ impl<'a> Parser<'a> {`
`35`	`35`	`attrs: self.parse_inner_attributes()?,`
`36`	`36`	`module: self.parse_mod_items(&token::Eof, lo)?,`
`37`	`37`	`span: lo.to(self.token.span),`
	`38`	`+ // Filled in by proc_macro_harness::inject()`
	`39`	`+ proc_macros: Vec::new(),`
`38`	`40`	`});`
`39`	`41`	`krate`
`40`	`42`	`}`