From 3edc5dcea865e4c6ca774ed747b90bfb1ffbd877 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 9 Mar 2022 14:33:39 +0100 Subject: [PATCH] fix: Properly handle proc-macro crate types for nameres --- crates/base_db/src/fixture.rs | 4 + crates/base_db/src/input.rs | 15 ++- crates/hir_def/src/attr.rs | 130 ++++++++++++------------ crates/hir_def/src/nameres/collector.rs | 71 +++++++------ crates/hir_expand/src/name.rs | 1 + crates/ide/src/lib.rs | 1 + crates/ide/src/shuffle_crate_graph.rs | 1 + crates/project_model/src/workspace.rs | 7 ++ 8 files changed, 133 insertions(+), 97 deletions(-) diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs index af825a2e00..ccf2bf7600 100644 --- a/crates/base_db/src/fixture.rs +++ b/crates/base_db/src/fixture.rs @@ -160,6 +160,7 @@ impl ChangeFixture { meta.cfg, meta.env, Default::default(), + false, origin, ); let prev = crates.insert(crate_name.clone(), crate_id); @@ -194,6 +195,7 @@ impl ChangeFixture { default_cfg, Env::default(), Default::default(), + false, Default::default(), ); } else { @@ -230,6 +232,7 @@ impl ChangeFixture { CfgOptions::default(), Env::default(), Vec::new(), + false, CrateOrigin::Lang, ); @@ -266,6 +269,7 @@ impl ChangeFixture { CfgOptions::default(), Env::default(), proc_macro, + true, CrateOrigin::Lang, ); diff --git a/crates/base_db/src/input.rs b/crates/base_db/src/input.rs index 08d3bc93ff..6a7f063586 100644 --- a/crates/base_db/src/input.rs +++ b/crates/base_db/src/input.rs @@ -124,7 +124,7 @@ pub enum CrateOrigin { /// Crates that are provided by the language, like std, core, proc-macro, ... Lang, /// Crates that we don't know their origin. - // Idealy this enum should cover all cases, and then we remove this variant. + // Ideally this enum should cover all cases, and then we remove this variant. Unknown, } @@ -228,6 +228,7 @@ pub struct CrateData { pub dependencies: Vec, pub proc_macro: Vec, pub origin: CrateOrigin, + pub is_proc_macro: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -279,6 +280,7 @@ impl CrateGraph { potential_cfg_options: CfgOptions, env: Env, proc_macro: Vec, + is_proc_macro: bool, origin: CrateOrigin, ) -> CrateId { let data = CrateData { @@ -292,6 +294,7 @@ impl CrateGraph { proc_macro, dependencies: Vec::new(), origin, + is_proc_macro, }; let crate_id = CrateId(self.arena.len() as u32); let prev = self.arena.insert(crate_id, data); @@ -596,6 +599,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); let crate2 = graph.add_crate_root( @@ -607,6 +611,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); let crate3 = graph.add_crate_root( @@ -618,6 +623,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); assert!(graph @@ -643,6 +649,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); let crate2 = graph.add_crate_root( @@ -654,6 +661,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); assert!(graph @@ -676,6 +684,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); let crate2 = graph.add_crate_root( @@ -687,6 +696,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); let crate3 = graph.add_crate_root( @@ -698,6 +708,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); assert!(graph @@ -720,6 +731,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); let crate2 = graph.add_crate_root( @@ -731,6 +743,7 @@ mod tests { CfgOptions::default(), Env::default(), Default::default(), + false, Default::default(), ); assert!(graph diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 94b801736c..d080182554 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -154,18 +154,21 @@ impl RawAttrs { return smallvec![attr.clone()]; } - let subtree = match attr.input.as_deref() { - Some(AttrInput::TokenTree(it, _)) => it, + let subtree = match attr.token_tree_value() { + Some(it) => it, _ => return smallvec![attr.clone()], }; // Input subtree is: `(cfg, $(attr),+)` // Split it up into a `cfg` subtree and the `attr` subtrees. // FIXME: There should be a common API for this. - let mut parts = subtree.token_trees.split( - |tt| matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ','), - ); - let cfg = parts.next().unwrap(); + let mut parts = subtree.token_trees.split(|tt| { + matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. }))) + }); + let cfg = match parts.next() { + Some(it) => it, + None => return smallvec![], + }; let cfg = Subtree { delimiter: subtree.delimiter, token_trees: cfg.to_vec() }; let cfg = CfgExpr::parse(&cfg); let index = attr.id; @@ -259,17 +262,8 @@ impl Attrs { } pub fn docs(&self) -> Option { - let docs = self.by_key("doc").attrs().flat_map(|attr| match attr.input.as_deref()? { - AttrInput::Literal(s) => Some(s), - AttrInput::TokenTree(..) => None, - }); - let indent = docs - .clone() - .flat_map(|s| s.lines()) - .filter(|line| !line.chars().all(|c| c.is_whitespace())) - .map(|line| line.chars().take_while(|c| c.is_whitespace()).count()) - .min() - .unwrap_or(0); + let docs = self.by_key("doc").attrs().filter_map(|attr| attr.string_value()); + let indent = doc_indent(self); let mut buf = String::new(); for doc in docs { // str::lines doesn't yield anything for the empty string @@ -507,18 +501,9 @@ impl AttrsWithOwner { &self, db: &dyn DefDatabase, ) -> Option<(Documentation, DocsRangeMap)> { - // FIXME: code duplication in `docs` above - let docs = self.by_key("doc").attrs().flat_map(|attr| match attr.input.as_deref()? { - AttrInput::Literal(s) => Some((s, attr.id)), - AttrInput::TokenTree(..) => None, - }); - let indent = docs - .clone() - .flat_map(|(s, _)| s.lines()) - .filter(|line| !line.chars().all(|c| c.is_whitespace())) - .map(|line| line.chars().take_while(|c| c.is_whitespace()).count()) - .min() - .unwrap_or(0); + let docs = + self.by_key("doc").attrs().filter_map(|attr| attr.string_value().map(|s| (s, attr.id))); + let indent = doc_indent(self); let mut buf = String::new(); let mut mapping = Vec::new(); for (doc, idx) in docs { @@ -557,6 +542,18 @@ impl AttrsWithOwner { } } +fn doc_indent(attrs: &Attrs) -> usize { + attrs + .by_key("doc") + .attrs() + .filter_map(|attr| attr.string_value()) + .flat_map(|s| s.lines()) + .filter(|line| !line.chars().all(|c| c.is_whitespace())) + .map(|line| line.chars().take_while(|c| c.is_whitespace()).count()) + .min() + .unwrap_or(0) +} + fn inner_attributes( syntax: &SyntaxNode, ) -> Option>> { @@ -773,45 +770,58 @@ impl Attr { Self::from_src(db, ast, hygiene, id) } + pub fn path(&self) -> &ModPath { + &self.path + } +} + +impl Attr { + /// #[path = "string"] + pub fn string_value(&self) -> Option<&SmolStr> { + match self.input.as_deref()? { + AttrInput::Literal(it) => Some(it), + _ => None, + } + } + + /// #[path(ident)] + pub fn single_ident_value(&self) -> Option<&tt::Ident> { + match self.input.as_deref()? { + AttrInput::TokenTree(subtree, _) => match &*subtree.token_trees { + [tt::TokenTree::Leaf(tt::Leaf::Ident(ident))] => Some(ident), + _ => None, + }, + _ => None, + } + } + + /// #[path TokenTree] + pub fn token_tree_value(&self) -> Option<&Subtree> { + match self.input.as_deref()? { + AttrInput::TokenTree(subtree, _) => Some(subtree), + _ => None, + } + } + /// Parses this attribute as a token tree consisting of comma separated paths. pub fn parse_path_comma_token_tree(&self) -> Option + '_> { - let args = match self.input.as_deref() { - Some(AttrInput::TokenTree(args, _)) => args, - _ => return None, - }; + let args = self.token_tree_value()?; if args.delimiter_kind() != Some(DelimiterKind::Parenthesis) { return None; } let paths = args .token_trees - .iter() - .group_by(|tt| { - matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. }))) - }) - .into_iter() - .filter(|(comma, _)| !*comma) - .map(|(_, tts)| { - let segments = tts.filter_map(|tt| match tt { + .split(|tt| matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. })))) + .map(|tts| { + let segments = tts.iter().filter_map(|tt| match tt { tt::TokenTree::Leaf(tt::Leaf::Ident(id)) => Some(id.as_name()), _ => None, }); ModPath::from_segments(PathKind::Plain, segments) - }) - .collect::>(); + }); - Some(paths.into_iter()) - } - - pub fn path(&self) -> &ModPath { - &self.path - } - - pub fn string_value(&self) -> Option<&SmolStr> { - match self.input.as_deref()? { - AttrInput::Literal(it) => Some(it), - _ => None, - } + Some(paths) } } @@ -823,17 +833,11 @@ pub struct AttrQuery<'attr> { impl<'attr> AttrQuery<'attr> { pub fn tt_values(self) -> impl Iterator { - self.attrs().filter_map(|attr| match attr.input.as_deref()? { - AttrInput::TokenTree(it, _) => Some(it), - _ => None, - }) + self.attrs().filter_map(|attr| attr.token_tree_value()) } pub fn string_value(self) -> Option<&'attr SmolStr> { - self.attrs().find_map(|attr| match attr.input.as_deref()? { - AttrInput::Literal(it) => Some(it), - _ => None, - }) + self.attrs().find_map(|attr| attr.string_value()) } pub fn exists(self) -> bool { diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index e1a297a3fc..985789b70e 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -22,10 +22,10 @@ use itertools::Itertools; use la_arena::Idx; use limit::Limit; use rustc_hash::{FxHashMap, FxHashSet}; -use syntax::ast; +use syntax::{ast, SmolStr}; use crate::{ - attr::{Attr, AttrId, AttrInput, Attrs}, + attr::{Attr, AttrId, Attrs}, attr_macro_as_call_id, db::DefDatabase, derive_macro_as_call_id, @@ -61,7 +61,8 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T let mut deps = FxHashMap::default(); // populate external prelude and dependency list - for dep in &crate_graph[def_map.krate].dependencies { + let krate = &crate_graph[def_map.krate]; + for dep in &krate.dependencies { tracing::debug!("crate dep {:?} -> {:?}", dep.name, dep.crate_id); let dep_def_map = db.crate_def_map(dep.crate_id); let dep_root = dep_def_map.module_id(dep_def_map.root); @@ -73,9 +74,9 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T } } - let cfg_options = &crate_graph[def_map.krate].cfg_options; - let proc_macros = &crate_graph[def_map.krate].proc_macro; - let proc_macros = proc_macros + let cfg_options = &krate.cfg_options; + let proc_macros = krate + .proc_macro .iter() .enumerate() .map(|(idx, it)| { @@ -87,6 +88,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T ) }) .collect(); + let is_proc_macro = krate.is_proc_macro; let mut collector = DefCollector { db, @@ -103,6 +105,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T from_glob_import: Default::default(), skip_attrs: Default::default(), derive_helpers_in_scope: Default::default(), + is_proc_macro, }; if tree_id.is_block() { collector.seed_with_inner(tree_id); @@ -243,6 +246,7 @@ struct DefCollector<'a> { /// empty when proc. macro support is disabled (in which case we still do name resolution for /// them). proc_macros: Vec<(Name, ProcMacroExpander)>, + is_proc_macro: bool, exports_proc_macros: bool, from_glob_import: PerNsGlobImports, /// If we fail to resolve an attribute on a `ModItem`, we fall back to ignoring the attribute. @@ -277,27 +281,29 @@ impl DefCollector<'_> { }; if *attr_name == hir_expand::name![recursion_limit] { - if let Some(input) = &attr.input { - if let AttrInput::Literal(limit) = &**input { - if let Ok(limit) = limit.parse() { - self.def_map.recursion_limit = Some(limit); - } + if let Some(limit) = attr.string_value() { + if let Ok(limit) = limit.parse() { + self.def_map.recursion_limit = Some(limit); } } continue; } + if *attr_name == hir_expand::name![crate_type] { + if let Some("proc-macro") = attr.string_value().map(SmolStr::as_str) { + self.is_proc_macro = true; + } + continue; + } + let attr_is_register_like = *attr_name == hir_expand::name![register_attr] || *attr_name == hir_expand::name![register_tool]; if !attr_is_register_like { continue; } - let registered_name = match attr.input.as_deref() { - Some(AttrInput::TokenTree(subtree, _)) => match &*subtree.token_trees { - [tt::TokenTree::Leaf(tt::Leaf::Ident(name))] => name.as_name(), - _ => continue, - }, + let registered_name = match attr.single_ident_value() { + Some(ident) => ident.as_name(), _ => continue, }; @@ -404,8 +410,7 @@ impl DefCollector<'_> { } self.unresolved_imports = unresolved_imports; - // FIXME: This condition should instead check if this is a `proc-macro` type crate. - if self.exports_proc_macros { + if self.is_proc_macro { // 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 @@ -1555,22 +1560,21 @@ impl ModCollector<'_, '_> { let fn_id = FunctionLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db); - let is_proc_macro = attrs.parse_proc_macro_decl(&it.name); - let vis = match is_proc_macro { - Some(proc_macro) => { - // FIXME: this should only be done in the root module of `proc-macro` crates, not everywhere - let module_id = def_map.module_id(def_map.root()); - - self.def_collector.export_proc_macro( - proc_macro, - ItemTreeId::new(self.tree_id, id), - fn_id, - module_id, - ); - Visibility::Module(module_id) + let vis = resolve_vis(def_map, &self.item_tree[it.visibility]); + if self.def_collector.is_proc_macro { + if self.module_id == def_map.root { + if let Some(proc_macro) = attrs.parse_proc_macro_decl(&it.name) { + let crate_root = def_map.module_id(def_map.root); + self.def_collector.export_proc_macro( + proc_macro, + ItemTreeId::new(self.tree_id, id), + fn_id, + crate_root, + ); + } } - None => resolve_vis(def_map, &self.item_tree[it.visibility]), - }; + } + update_def(self.def_collector, fn_id.into(), &it.name, vis, false); } ModItem::Struct(id) => { @@ -2099,6 +2103,7 @@ mod tests { from_glob_import: Default::default(), skip_attrs: Default::default(), derive_helpers_in_scope: Default::default(), + is_proc_macro: false, }; collector.seed_with_top_level(); collector.collect(); diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs index f62c3ca0a0..f1bf665707 100644 --- a/crates/hir_expand/src/name.rs +++ b/crates/hir_expand/src/name.rs @@ -268,6 +268,7 @@ pub mod known { bench, cfg_accessible, cfg_eval, + crate_type, derive, global_allocator, test, diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 4028b0bc72..c8aad24ac0 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -231,6 +231,7 @@ impl Analysis { cfg_options, Env::default(), Default::default(), + false, Default::default(), ); change.change_file(file_id, Some(Arc::new(text))); diff --git a/crates/ide/src/shuffle_crate_graph.rs b/crates/ide/src/shuffle_crate_graph.rs index 9c3cce5dfe..a7720d8156 100644 --- a/crates/ide/src/shuffle_crate_graph.rs +++ b/crates/ide/src/shuffle_crate_graph.rs @@ -34,6 +34,7 @@ pub(crate) fn shuffle_crate_graph(db: &mut RootDatabase) { data.potential_cfg_options.clone(), data.env.clone(), data.proc_macro.clone(), + data.is_proc_macro, data.origin.clone(), ); map.insert(old_id, new_id); diff --git a/crates/project_model/src/workspace.rs b/crates/project_model/src/workspace.rs index 96522bf070..1a86d232eb 100644 --- a/crates/project_model/src/workspace.rs +++ b/crates/project_model/src/workspace.rs @@ -483,6 +483,7 @@ fn project_json_to_crate_graph( cfg_options, env, proc_macro.unwrap_or_default(), + krate.is_proc_macro, if krate.display_name.is_some() { CrateOrigin::CratesIo { repo: krate.repository.clone() } } else { @@ -592,6 +593,7 @@ fn cargo_to_crate_graph( &mut |path| load_proc_macro(&cargo[tgt].name, path), file_id, &cargo[tgt].name, + cargo[tgt].is_proc_macro, ); if cargo[tgt].kind == TargetKind::Lib { lib_tgt = Some((crate_id, cargo[tgt].name.clone())); @@ -707,6 +709,7 @@ fn detached_files_to_crate_graph( cfg_options.clone(), Env::default(), Vec::new(), + false, CrateOrigin::Unknown, ); @@ -759,6 +762,7 @@ fn handle_rustc_crates( &mut |path| load_proc_macro(&rustc_workspace[tgt].name, path), file_id, &rustc_workspace[tgt].name, + rustc_workspace[tgt].is_proc_macro, ); pkg_to_lib_crate.insert(pkg, crate_id); // Add dependencies on core / std / alloc for this crate @@ -813,6 +817,7 @@ fn add_target_crate_root( load_proc_macro: &mut dyn FnMut(&AbsPath) -> Vec, file_id: FileId, cargo_name: &str, + is_proc_macro: bool, ) -> CrateId { let edition = pkg.edition; let cfg_options = { @@ -857,6 +862,7 @@ fn add_target_crate_root( potential_cfg_options, env, proc_macro, + is_proc_macro, CrateOrigin::CratesIo { repo: pkg.repository.clone() }, ) } @@ -901,6 +907,7 @@ fn sysroot_to_crate_graph( cfg_options.clone(), env, proc_macro, + false, CrateOrigin::Lang, ); Some((krate, crate_id))