From 51a16e574aa1b20dbaa94436065ce80f3ba725d1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 3 Feb 2020 18:34:36 -0500 Subject: [PATCH 1/2] 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. --- src/librustc/hir/map/collector.rs | 1 + src/librustc_ast_lowering/lib.rs | 2 + .../proc_macro_harness.rs | 61 +++++++++++++------ src/librustc_hir/hir.rs | 3 + src/librustc_metadata/rmeta/decoder.rs | 4 -- src/librustc_metadata/rmeta/encoder.rs | 9 +-- src/librustc_parse/parser/module.rs | 2 + src/libsyntax/ast.rs | 1 + src/libsyntax/mut_visit.rs | 6 +- .../inline_cross/auxiliary/proc_macro.rs | 13 ++++ src/test/rustdoc/inline_cross/proc_macro.rs | 8 +++ src/test/ui/ast-json/ast-json-output.stdout | 2 +- 12 files changed, 78 insertions(+), 34 deletions(-) diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index 4c922654e02d5..bf1fc09649a58 100644 --- 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 index 99de4b88fd3c4..30fe7de5df4fa 100644 --- 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 index 222456d8fe0d9..b925cad9fdcdc 100644 --- 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, ¯os)); + let decls = mk_decls(&mut krate, &mut cx, ¯os); + 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 { +fn mk_decls( + ast_krate: &mut ast::Crate, + cx: &mut ExtCtxt<'_>, + macros: &[ProcMacro], +) -> P { + // 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 { 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 { 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::>(), - ), - 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::>(), + ), + 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 index 07a2c48225349..f13e2f186043a 100644 --- 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, + /// 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, } impl Crate<'hir> { diff --git a/src/librustc_metadata/rmeta/decoder.rs b/src/librustc_metadata/rmeta/decoder.rs index 58cf142ab3a36..01fd637b20e66 100644 --- 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 index 4133047af78fe..41fc5ed843fd3 100644 --- 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 index 754923ae55e29..9dba813c9a721 100644 --- 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 index 72430fa9c17e4..1973733288bce 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -429,6 +429,7 @@ pub struct Crate { pub module: Mod, pub attrs: Vec, pub span: Span, + pub proc_macros: Vec, } /// Possible values inside of compile-time attribute lists. diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index e0180d451936f..f130b0a2ee494 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -989,7 +989,7 @@ pub fn noop_visit_mod(Mod { inner, items, inline: _ }: &mut Mod, } pub fn noop_visit_crate(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(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 index 37465ccf1c27e..d8e5746f3f6f5 100644 --- 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 index 3dc8de3fe579d..532a295c0f3f7 100644 --- 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 index b299fc558410d..35e418696f17c 100644 --- 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":[]} From 516459870cf0fe13979aaa568bfc413e2e5bba18 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 15 Feb 2020 15:51:40 -0500 Subject: [PATCH 2/2] Add additional comment --- src/libsyntax/ast.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 1973733288bce..d101473d76ba6 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -429,6 +429,12 @@ pub struct Crate { pub module: Mod, pub attrs: Vec, 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, }