From dfa3a3f017c39f745f06d96de11359b97c76fc47 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 16:37:12 +0200 Subject: [PATCH 1/7] Add test --- crates/hir_def/src/nameres/tests/macros.rs | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/hir_def/src/nameres/tests/macros.rs b/crates/hir_def/src/nameres/tests/macros.rs index e0fb8bdef46..98cb5a0fdc3 100644 --- a/crates/hir_def/src/nameres/tests/macros.rs +++ b/crates/hir_def/src/nameres/tests/macros.rs @@ -667,3 +667,35 @@ macro_rules! b { "#]], ); } + +#[test] +fn resolves_proc_macros() { + check( + r" + struct TokenStream; + + #[proc_macro] + pub fn function_like_macro(args: TokenStream) -> TokenStream { + args + } + + #[proc_macro_attribute] + pub fn attribute_macro(_args: TokenStream, item: TokenStream) -> TokenStream { + item + } + + #[proc_macro_derive(DummyTrait)] + pub fn derive_macro(_item: TokenStream) -> TokenStream { + TokenStream + } + ", + expect![[r#" + crate + DummyTrait: m + TokenStream: t v + attribute_macro: v m + derive_macro: v + function_like_macro: v m + "#]], + ); +} From 5486b70bc0fa1b6260178fa7e547a534d91c3e04 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 16:43:50 +0200 Subject: [PATCH 2/7] Use hir_def to resolve proc macros --- crates/hir_def/src/attr.rs | 6 +++ crates/hir_def/src/nameres/collector.rs | 50 ++++++++++++++++++++++++- crates/hir_expand/src/proc_macro.rs | 35 +++++++++++------ 3 files changed, 77 insertions(+), 14 deletions(-) diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index dea552a605a..a841b97bfe4 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -171,6 +171,9 @@ pub struct AttrQuery<'a> { } impl<'a> AttrQuery<'a> { + /// For an attribute like `#[attr(value)]`, returns the `(value)` subtree. + /// + /// If the attribute does not have a token tree argument, returns `None`. pub fn tt_values(self) -> impl Iterator { self.attrs().filter_map(|attr| match attr.input.as_ref()? { AttrInput::TokenTree(it) => Some(it), @@ -178,6 +181,9 @@ pub fn tt_values(self) -> impl Iterator { }) } + /// For an attribute like `#[key = "value"]`, returns `"value"`. + /// + /// Returns `None` if the attribute does not have `key = "value"` form. pub fn string_value(self) -> Option<&'a SmolStr> { self.attrs().find_map(|attr| match attr.input.as_ref()? { AttrInput::Literal(it) => Some(it), diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 4c3993ff01b..42c0f053693 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -16,10 +16,10 @@ proc_macro::ProcMacroExpander, HirFileId, MacroCallId, MacroDefId, MacroDefKind, }; -use rustc_hash::FxHashMap; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use syntax::ast; use test_utils::mark; +use tt::{Leaf, TokenTree}; use crate::{ attr::Attrs, @@ -281,6 +281,25 @@ fn collect_proc_macro(&mut self) { } } + fn resolve_proc_macro(&mut self, name: &Name) { + let macro_def = match self.proc_macros.iter().find(|(n, _)| n == name) { + Some((_, expander)) => MacroDefId { + ast_id: None, + krate: Some(self.def_map.krate), + kind: MacroDefKind::ProcMacro(*expander), + local_inner: false, + }, + None => MacroDefId { + ast_id: None, + krate: Some(self.def_map.krate), + kind: MacroDefKind::ProcMacro(ProcMacroExpander::dummy(self.def_map.krate)), + local_inner: false, + }, + }; + + self.define_proc_macro(name.clone(), macro_def); + } + /// Define a macro with `macro_rules`. /// /// It will define the macro in legacy textual scope, and if it has `#[macro_export]`, @@ -917,6 +936,9 @@ fn collect(&mut self, items: &[ModItem]) { } ModItem::Function(id) => { let func = &self.item_tree[id]; + + self.collect_proc_macro_def(&func.name, attrs); + def = Some(DefData { id: FunctionLoc { container: container.into(), @@ -1177,6 +1199,30 @@ fn collect_derives(&mut self, attrs: &Attrs, ast_id: FileAstId) { } } + /// If `attrs` registers a procedural macro, collects its definition. + fn collect_proc_macro_def(&mut self, func_name: &Name, attrs: &Attrs) { + // FIXME: this should only be done in the root module of `proc-macro` crates, not everywhere + // FIXME: distinguish the type of macro + let macro_name = if attrs.by_key("proc_macro").exists() + || attrs.by_key("proc_macro_attribute").exists() + { + func_name.clone() + } else { + let derive = attrs.by_key("proc_macro_derive"); + if let Some(arg) = derive.tt_values().next() { + if let [TokenTree::Leaf(Leaf::Ident(trait_name))] = &*arg.token_trees { + trait_name.as_name() + } else { + return; + } + } else { + return; + } + }; + + self.def_collector.resolve_proc_macro(¯o_name); + } + fn collect_macro(&mut self, mac: &MacroCall) { let mut ast_id = AstIdWithPath::new(self.file_id, mac.ast_id, mac.path.clone()); diff --git a/crates/hir_expand/src/proc_macro.rs b/crates/hir_expand/src/proc_macro.rs index 80255ea3274..7505cb061bf 100644 --- a/crates/hir_expand/src/proc_macro.rs +++ b/crates/hir_expand/src/proc_macro.rs @@ -7,7 +7,7 @@ #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub struct ProcMacroExpander { krate: CrateId, - proc_macro_id: ProcMacroId, + proc_macro_id: Option, } macro_rules! err { @@ -20,8 +20,14 @@ macro_rules! err { } impl ProcMacroExpander { - pub fn new(krate: CrateId, proc_macro_id: ProcMacroId) -> ProcMacroExpander { - ProcMacroExpander { krate, proc_macro_id } + pub fn new(krate: CrateId, proc_macro_id: ProcMacroId) -> Self { + Self { krate, proc_macro_id: Some(proc_macro_id) } + } + + pub fn dummy(krate: CrateId) -> Self { + // FIXME: Should store the name for better errors + // FIXME: I think this is the second layer of "dummy" expansion, we should reduce that + Self { krate, proc_macro_id: None } } pub fn expand( @@ -30,17 +36,22 @@ pub fn expand( _id: LazyMacroId, tt: &tt::Subtree, ) -> Result { - let krate_graph = db.crate_graph(); - let proc_macro = krate_graph[self.krate] - .proc_macro - .get(self.proc_macro_id.0 as usize) - .clone() - .ok_or_else(|| err!("No derive macro found."))?; + match self.proc_macro_id { + Some(id) => { + let krate_graph = db.crate_graph(); + let proc_macro = krate_graph[self.krate] + .proc_macro + .get(id.0 as usize) + .clone() + .ok_or_else(|| err!("No derive macro found."))?; - let tt = remove_derive_attrs(tt) - .ok_or_else(|| err!("Fail to remove derive for custom derive"))?; + let tt = remove_derive_attrs(tt) + .ok_or_else(|| err!("Fail to remove derive for custom derive"))?; - proc_macro.expander.expand(&tt, None).map_err(mbe::ExpandError::from) + proc_macro.expander.expand(&tt, None).map_err(mbe::ExpandError::from) + } + None => Err(err!("Unresolved proc macro")), + } } } From 069045015c4b400754632c505f6ef19e32f9a4db Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 16:52:24 +0200 Subject: [PATCH 3/7] Remove obsolete proc macro collection code The new attribute-based resolution takes care of this --- crates/hir_def/src/nameres/collector.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 42c0f053693..28ef494881f 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -260,25 +260,6 @@ fn collect(&mut self) { self.record_resolved_import(directive) } self.unresolved_imports = unresolved_imports; - - // Record proc-macros - self.collect_proc_macro(); - } - - fn collect_proc_macro(&mut self) { - let proc_macros = std::mem::take(&mut self.proc_macros); - for (name, expander) in proc_macros { - let krate = self.def_map.krate; - - let macro_id = MacroDefId { - ast_id: None, - krate: Some(krate), - kind: MacroDefKind::ProcMacro(expander), - local_inner: false, - }; - - self.define_proc_macro(name.clone(), macro_id); - } } fn resolve_proc_macro(&mut self, name: &Name) { From baab72e611fa985c2e62e964f3a48ad31367220f Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 17:50:04 +0200 Subject: [PATCH 4/7] Reduce visibility of non-proc-macros proc-macro crates only export proc-macros, but currently other items are also considered public (and show up in completion) --- crates/hir_def/src/item_scope.rs | 25 +++++++++++++ crates/hir_def/src/nameres/collector.rs | 19 ++++++++++ crates/hir_def/src/nameres/tests/macros.rs | 41 ++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs index f1e9dfd5b14..99820c275cf 100644 --- a/crates/hir_def/src/item_scope.rs +++ b/crates/hir_def/src/item_scope.rs @@ -5,10 +5,12 @@ use base_db::CrateId; use hir_expand::name::Name; +use hir_expand::MacroDefKind; use once_cell::sync::Lazy; use rustc_hash::{FxHashMap, FxHashSet}; use test_utils::mark; +use crate::ModuleId; use crate::{ db::DefDatabase, per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, HasModule, ImplId, LocalModuleId, Lookup, MacroDefId, ModuleDefId, TraitId, @@ -265,6 +267,29 @@ pub(crate) fn resolutions<'a>(&'a self) -> impl Iterator, P pub(crate) fn collect_legacy_macros(&self) -> FxHashMap { self.legacy_macros.clone() } + + /// Marks everything that is not a procedural macro as private to `this_module`. + pub(crate) fn censor_non_proc_macros(&mut self, this_module: ModuleId) { + for vis in self + .types + .values_mut() + .chain(self.values.values_mut()) + .map(|(_, v)| v) + .chain(self.unnamed_trait_imports.values_mut()) + { + *vis = Visibility::Module(this_module); + } + + for (mac, vis) in self.macros.values_mut() { + if let MacroDefKind::ProcMacro(_) = mac.kind { + // FIXME: Technically this is insufficient since reexports of proc macros are also + // forbidden. Practically nobody does that. + continue; + } + + *vis = Visibility::Module(this_module); + } + } } impl PerNs { diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 28ef494881f..f98a42643af 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -87,6 +87,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: CrateDefMap) -> Cr mod_dirs: FxHashMap::default(), cfg_options, proc_macros, + exports_proc_macros: false, from_glob_import: Default::default(), }; collector.collect(); @@ -203,6 +204,7 @@ struct DefCollector<'a> { mod_dirs: FxHashMap, cfg_options: &'a CfgOptions, proc_macros: Vec<(Name, ProcMacroExpander)>, + exports_proc_macros: bool, from_glob_import: PerNsGlobImports, } @@ -260,9 +262,25 @@ fn collect(&mut self) { self.record_resolved_import(directive) } self.unresolved_imports = unresolved_imports; + + // FIXME: This condition should instead check if this is a `proc-macro` type crate. + if self.exports_proc_macros { + // A crate exporting procedural macros is not allowed to export anything else. + // + // Additionally, while the proc macro entry points must be `pub`, they are not publicly + // exported in type/value namespace. This function reduces the visibility of all items + // in the crate root that aren't proc macros. + let root = self.def_map.root; + let root = &mut self.def_map.modules[root]; + root.scope.censor_non_proc_macros(ModuleId { + krate: self.def_map.krate, + local_id: self.def_map.root, + }); + } } fn resolve_proc_macro(&mut self, name: &Name) { + self.exports_proc_macros = true; let macro_def = match self.proc_macros.iter().find(|(n, _)| n == name) { Some((_, expander)) => MacroDefId { ast_id: None, @@ -1310,6 +1328,7 @@ fn do_collect_defs(db: &dyn DefDatabase, def_map: CrateDefMap) -> CrateDefMap { mod_dirs: FxHashMap::default(), cfg_options: &CfgOptions::default(), proc_macros: Default::default(), + exports_proc_macros: false, from_glob_import: Default::default(), }; collector.collect(); diff --git a/crates/hir_def/src/nameres/tests/macros.rs b/crates/hir_def/src/nameres/tests/macros.rs index 98cb5a0fdc3..0851c3b7d89 100644 --- a/crates/hir_def/src/nameres/tests/macros.rs +++ b/crates/hir_def/src/nameres/tests/macros.rs @@ -699,3 +699,44 @@ pub fn derive_macro(_item: TokenStream) -> TokenStream { "#]], ); } + +#[test] +fn proc_macro_censoring() { + // Make sure that only proc macros are publicly exported from proc-macro crates. + + check( + r" + //- /main.rs crate:main deps:macros + pub use macros::*; + + //- /macros.rs crate:macros + pub struct TokenStream; + + #[proc_macro] + pub fn function_like_macro(args: TokenStream) -> TokenStream { + args + } + + #[proc_macro_attribute] + pub fn attribute_macro(_args: TokenStream, item: TokenStream) -> TokenStream { + item + } + + #[proc_macro_derive(DummyTrait)] + pub fn derive_macro(_item: TokenStream) -> TokenStream { + TokenStream + } + + #[macro_export] + macro_rules! mbe { + () => {}; + } + ", + expect![[r#" + crate + DummyTrait: m + attribute_macro: m + function_like_macro: m + "#]], + ); +} From 7474a42b001b0fea51f761173bd3e194b7901f75 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 18:09:47 +0200 Subject: [PATCH 5/7] Remove incorrect docs --- crates/hir_def/src/attr.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index a841b97bfe4..dea552a605a 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -171,9 +171,6 @@ pub struct AttrQuery<'a> { } impl<'a> AttrQuery<'a> { - /// For an attribute like `#[attr(value)]`, returns the `(value)` subtree. - /// - /// If the attribute does not have a token tree argument, returns `None`. pub fn tt_values(self) -> impl Iterator { self.attrs().filter_map(|attr| match attr.input.as_ref()? { AttrInput::TokenTree(it) => Some(it), @@ -181,9 +178,6 @@ pub fn tt_values(self) -> impl Iterator { }) } - /// For an attribute like `#[key = "value"]`, returns `"value"`. - /// - /// Returns `None` if the attribute does not have `key = "value"` form. pub fn string_value(self) -> Option<&'a SmolStr> { self.attrs().find_map(|attr| match attr.input.as_ref()? { AttrInput::Literal(it) => Some(it), From e799dbe5d7cc5d09ee5457595425cf2f42e1c113 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 28 Sep 2020 12:51:40 +0200 Subject: [PATCH 6/7] Simplify iterator chain --- crates/hir_def/src/item_scope.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs index 99820c275cf..12c24e1ca3f 100644 --- a/crates/hir_def/src/item_scope.rs +++ b/crates/hir_def/src/item_scope.rs @@ -270,15 +270,12 @@ pub(crate) fn collect_legacy_macros(&self) -> FxHashMap { /// Marks everything that is not a procedural macro as private to `this_module`. pub(crate) fn censor_non_proc_macros(&mut self, this_module: ModuleId) { - for vis in self - .types + self.types .values_mut() .chain(self.values.values_mut()) .map(|(_, v)| v) .chain(self.unnamed_trait_imports.values_mut()) - { - *vis = Visibility::Module(this_module); - } + .for_each(|vis| *vis = Visibility::Module(this_module)); for (mac, vis) in self.macros.values_mut() { if let MacroDefKind::ProcMacro(_) = mac.kind { From e88e4fbb7bb32065c6a7570057de248c2ea3a514 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 28 Sep 2020 13:02:28 +0200 Subject: [PATCH 7/7] Add more comments about proc macro resolution --- crates/hir_def/src/nameres/collector.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index f98a42643af..100e25ffcf0 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -203,6 +203,10 @@ struct DefCollector<'a> { unexpanded_attribute_macros: Vec, mod_dirs: FxHashMap, cfg_options: &'a CfgOptions, + /// List of procedural macros defined by this crate. This is read from the dynamic library + /// built by the build system, and is the list of proc. macros we can actually expand. It is + /// empty when proc. macro support is disabled (in which case we still do name resolution for + /// them). proc_macros: Vec<(Name, ProcMacroExpander)>, exports_proc_macros: bool, from_glob_import: PerNsGlobImports, @@ -279,6 +283,22 @@ fn collect(&mut self) { } } + /// Adds a definition of procedural macro `name` to the root module. + /// + /// # Notes on procedural macro resolution + /// + /// Procedural macro functionality is provided by the build system: It has to build the proc + /// macro and pass the resulting dynamic library to rust-analyzer. + /// + /// When procedural macro support is enabled, the list of proc macros exported by a crate is + /// known before we resolve names in the crate. This list is stored in `self.proc_macros` and is + /// derived from the dynamic library. + /// + /// However, we *also* would like to be able to at least *resolve* macros on our own, without + /// help by the build system. So, when the macro isn't found in `self.proc_macros`, we instead + /// use a dummy expander that always errors. This comes with the drawback of macros potentially + /// going out of sync with what the build system sees (since we resolve using VFS state, but + /// Cargo builds only on-disk files). We could and probably should add diagnostics for that. fn resolve_proc_macro(&mut self, name: &Name) { self.exports_proc_macros = true; let macro_def = match self.proc_macros.iter().find(|(n, _)| n == name) {