From dfa3a3f017c39f745f06d96de11359b97c76fc47 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 16:37:12 +0200 Subject: Add test --- crates/hir_def/src/nameres/tests/macros.rs | 32 ++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) (limited to 'crates') diff --git a/crates/hir_def/src/nameres/tests/macros.rs b/crates/hir_def/src/nameres/tests/macros.rs index e0fb8bdef..98cb5a0fd 100644 --- a/crates/hir_def/src/nameres/tests/macros.rs +++ b/crates/hir_def/src/nameres/tests/macros.rs @@ -667,3 +667,35 @@ b! { static = #[] (); } "#]], ); } + +#[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 + "#]], + ); +} -- cgit v1.2.3 From 5486b70bc0fa1b6260178fa7e547a534d91c3e04 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 16:43:50 +0200 Subject: 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 | 39 ++++++++++++++++--------- 3 files changed, 79 insertions(+), 16 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index dea552a60..a841b97bf 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 @@ impl<'a> AttrQuery<'a> { }) } + /// 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 4c3993ff0..42c0f0536 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -16,10 +16,10 @@ use hir_expand::{ 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 @@ impl DefCollector<'_> { } } + 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 @@ impl ModCollector<'_, '_> { } 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 @@ impl ModCollector<'_, '_> { } } + /// 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 80255ea32..7505cb061 100644 --- a/crates/hir_expand/src/proc_macro.rs +++ b/crates/hir_expand/src/proc_macro.rs @@ -7,7 +7,7 @@ use tt::buffer::{Cursor, TokenBuffer}; #[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 @@ impl ProcMacroExpander { _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."))?; - - 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) + 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"))?; + + proc_macro.expander.expand(&tt, None).map_err(mbe::ExpandError::from) + } + None => Err(err!("Unresolved proc macro")), + } } } -- cgit v1.2.3 From 069045015c4b400754632c505f6ef19e32f9a4db Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 16:52:24 +0200 Subject: 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(-) (limited to 'crates') diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 42c0f0536..28ef49488 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -260,25 +260,6 @@ impl DefCollector<'_> { 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) { -- cgit v1.2.3 From baab72e611fa985c2e62e964f3a48ad31367220f Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 17:50:04 +0200 Subject: 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(+) (limited to 'crates') diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs index f1e9dfd5b..99820c275 100644 --- a/crates/hir_def/src/item_scope.rs +++ b/crates/hir_def/src/item_scope.rs @@ -5,10 +5,12 @@ use std::collections::hash_map::Entry; 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 @@ impl ItemScope { 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 28ef49488..f98a42643 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 @@ impl DefCollector<'_> { 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 @@ mod tests { 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 98cb5a0fd..0851c3b7d 100644 --- a/crates/hir_def/src/nameres/tests/macros.rs +++ b/crates/hir_def/src/nameres/tests/macros.rs @@ -699,3 +699,44 @@ fn resolves_proc_macros() { "#]], ); } + +#[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 + "#]], + ); +} -- cgit v1.2.3 From 7474a42b001b0fea51f761173bd3e194b7901f75 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 18 Sep 2020 18:09:47 +0200 Subject: Remove incorrect docs --- crates/hir_def/src/attr.rs | 6 ------ 1 file changed, 6 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index a841b97bf..dea552a60 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 @@ impl<'a> AttrQuery<'a> { }) } - /// 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), -- cgit v1.2.3 From e799dbe5d7cc5d09ee5457595425cf2f42e1c113 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 28 Sep 2020 12:51:40 +0200 Subject: Simplify iterator chain --- crates/hir_def/src/item_scope.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs index 99820c275..12c24e1ca 100644 --- a/crates/hir_def/src/item_scope.rs +++ b/crates/hir_def/src/item_scope.rs @@ -270,15 +270,12 @@ impl ItemScope { /// 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 { -- cgit v1.2.3 From e88e4fbb7bb32065c6a7570057de248c2ea3a514 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 28 Sep 2020 13:02:28 +0200 Subject: Add more comments about proc macro resolution --- crates/hir_def/src/nameres/collector.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'crates') diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index f98a42643..100e25ffc 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 @@ impl DefCollector<'_> { } } + /// 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) { -- cgit v1.2.3