From 780e1a365b10027c4bd4adcc939ab32da1d91492 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 8 Jun 2019 18:38:14 +0300 Subject: somewhat better name --- crates/ra_hir/src/resolve.rs | 6 +++++- crates/ra_hir/src/source_binder.rs | 4 ++-- crates/ra_hir/src/ty/infer.rs | 37 ++++++++++++++++++++----------------- crates/ra_hir/src/ty/lower.rs | 4 ++-- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/crates/ra_hir/src/resolve.rs b/crates/ra_hir/src/resolve.rs index d6956f45e..347bcf27d 100644 --- a/crates/ra_hir/src/resolve.rs +++ b/crates/ra_hir/src/resolve.rs @@ -165,7 +165,11 @@ impl Resolver { /// Returns the fully resolved path if we were able to resolve it. /// otherwise returns `PerNs::none` - pub(crate) fn resolve_path(&self, db: &impl HirDatabase, path: &Path) -> PerNs { + pub(crate) fn resolve_path_without_assoc_items( + &self, + db: &impl HirDatabase, + path: &Path, + ) -> PerNs { // into_fully_resolved() returns the fully resolved path or PerNs::none() otherwise self.resolve_path_segments(db, path).into_fully_resolved() } diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index 410064d45..63ec59314 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs @@ -277,7 +277,7 @@ impl SourceAnalyzer { db: &impl HirDatabase, path: &crate::Path, ) -> PerNs { - self.resolver.resolve_path(db, path) + self.resolver.resolve_path_without_assoc_items(db, path) } pub fn resolve_path(&self, db: &impl HirDatabase, path: &ast::Path) -> Option { @@ -294,7 +294,7 @@ impl SourceAnalyzer { } } let hir_path = crate::Path::from_ast(path)?; - let res = self.resolver.resolve_path(db, &hir_path); + let res = self.resolver.resolve_path_without_assoc_items(db, &hir_path); let res = res.clone().take_types().or_else(|| res.take_values())?; let res = match res { crate::Resolution::Def(it) => PathResolution::Def(it), diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs index 6cc5dbc6f..6aa727ea1 100644 --- a/crates/ra_hir/src/ty/infer.rs +++ b/crates/ra_hir/src/ty/infer.rs @@ -610,23 +610,26 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { None => return (Ty::Unknown, None), }; let resolver = &self.resolver; - let typable: Option = match resolver.resolve_path(self.db, &path).take_types() { - Some(Resolution::Def(def)) => def.into(), - Some(Resolution::LocalBinding(..)) => { - // this cannot happen - log::error!("path resolved to local binding in type ns"); - return (Ty::Unknown, None); - } - Some(Resolution::GenericParam(..)) => { - // generic params can't be used in struct literals - return (Ty::Unknown, None); - } - Some(Resolution::SelfType(..)) => { - // FIXME this is allowed in an impl for a struct, handle this - return (Ty::Unknown, None); - } - None => return (Ty::Unknown, None), - }; + let typable: Option = + // FIXME: this should resolve assoc items as well, see this example: + // https://play.rust-lang.org/?gist=087992e9e22495446c01c0d4e2d69521 + match resolver.resolve_path_without_assoc_items(self.db, &path).take_types() { + Some(Resolution::Def(def)) => def.into(), + Some(Resolution::LocalBinding(..)) => { + // this cannot happen + log::error!("path resolved to local binding in type ns"); + return (Ty::Unknown, None); + } + Some(Resolution::GenericParam(..)) => { + // generic params can't be used in struct literals + return (Ty::Unknown, None); + } + Some(Resolution::SelfType(..)) => { + // FIXME this is allowed in an impl for a struct, handle this + return (Ty::Unknown, None); + } + None => return (Ty::Unknown, None), + }; let def = match typable { None => return (Ty::Unknown, None), Some(it) => it, diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs index 71cd72234..26c213a41 100644 --- a/crates/ra_hir/src/ty/lower.rs +++ b/crates/ra_hir/src/ty/lower.rs @@ -65,7 +65,7 @@ impl Ty { pub(crate) fn from_hir_path(db: &impl HirDatabase, resolver: &Resolver, path: &Path) -> Self { // Resolve the path (in type namespace) - let resolution = resolver.resolve_path(db, path).take_types(); + let resolution = resolver.resolve_path_without_assoc_items(db, path).take_types(); let def = match resolution { Some(Resolution::Def(def)) => def, @@ -216,7 +216,7 @@ impl TraitRef { path: &Path, explicit_self_ty: Option, ) -> Option { - let resolved = match resolver.resolve_path(db, &path).take_types()? { + let resolved = match resolver.resolve_path_without_assoc_items(db, &path).take_types()? { Resolution::Def(ModuleDef::Trait(tr)) => tr, _ => return None, }; -- cgit v1.2.3 From caefa6982bc57195687de11137997f1d62d791fe Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 8 Jun 2019 20:42:02 +0300 Subject: remove some hacks from nameresolution for macros --- crates/ra_hir/src/expr.rs | 26 ++++++++++----------- crates/ra_hir/src/nameres.rs | 29 ++--------------------- crates/ra_hir/src/nameres/collector.rs | 38 +++++++++++++++++++++++-------- crates/ra_hir/src/nameres/tests/macros.rs | 13 +++++++---- crates/ra_hir/src/resolve.rs | 17 ++++++++------ crates/ra_hir/src/source_binder.rs | 5 ++-- crates/ra_hir/src/ty/tests.rs | 6 +++-- 7 files changed, 68 insertions(+), 66 deletions(-) diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index 9d9769859..012f374ec 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs @@ -827,25 +827,25 @@ where ast::ExprKind::IndexExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr), ast::ExprKind::RangeExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr), ast::ExprKind::MacroCall(e) => { - // very hacky.FIXME change to use the macro resolution - let path = e.path().and_then(Path::from_ast); - let ast_id = self .db .ast_id_map(self.current_file_id) .ast_id(e) .with_file_id(self.current_file_id); - if let Some(def) = self.resolver.resolve_macro_call(self.db, path) { - let call_id = MacroCallLoc { def, ast_id }.id(self.db); - let file_id = call_id.as_file(MacroFileKind::Expr); - if let Some(node) = self.db.parse_or_expand(file_id) { - if let Some(expr) = ast::Expr::cast(&*node) { - log::debug!("macro expansion {}", expr.syntax().debug_dump()); - let old_file_id = std::mem::replace(&mut self.current_file_id, file_id); - let id = self.collect_expr(&expr); - self.current_file_id = old_file_id; - return id; + if let Some(path) = e.path().and_then(Path::from_ast) { + if let Some(def) = self.resolver.resolve_path_as_macro(self.db, &path) { + let call_id = MacroCallLoc { def: def.id, ast_id }.id(self.db); + let file_id = call_id.as_file(MacroFileKind::Expr); + if let Some(node) = self.db.parse_or_expand(file_id) { + if let Some(expr) = ast::Expr::cast(&*node) { + log::debug!("macro expansion {}", expr.syntax().debug_dump()); + let old_file_id = + std::mem::replace(&mut self.current_file_id, file_id); + let id = self.collect_expr(&expr); + self.current_file_id = old_file_id; + return id; + } } } } diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs index dc0dd23c9..b5938fa03 100644 --- a/crates/ra_hir/src/nameres.rs +++ b/crates/ra_hir/src/nameres.rs @@ -92,7 +92,6 @@ pub struct CrateDefMap { extern_prelude: FxHashMap, root: CrateModuleId, modules: Arena, - public_macros: FxHashMap, /// Some macros are not well-behavior, which leads to infinite loop /// e.g. macro_rules! foo { ($ty:ty) => { foo!($ty); } } @@ -106,7 +105,6 @@ pub struct CrateDefMap { /// However, do we want to put it as a global variable? poison_macros: FxHashSet, - local_macros: FxHashMap, diagnostics: Vec, } @@ -249,9 +247,7 @@ impl CrateDefMap { prelude: None, root, modules, - public_macros: FxHashMap::default(), poison_macros: FxHashSet::default(), - local_macros: FxHashMap::default(), diagnostics: Vec::new(), } }; @@ -313,7 +309,7 @@ impl CrateDefMap { (res.resolved_def.left().unwrap_or_else(PerNs::none), res.segment_index) } - fn resolve_path_with_macro( + pub(crate) fn resolve_path_with_macro( &self, db: &impl DefDatabase, original_module: CrateModuleId, @@ -323,27 +319,6 @@ impl CrateDefMap { (res.resolved_def, res.segment_index) } - // FIXME: This seems to do the same work as `resolve_path_with_macro`, but - // using a completely different code path. Seems bad, huh? - pub(crate) fn find_macro( - &self, - db: &impl DefDatabase, - original_module: CrateModuleId, - path: &Path, - ) -> Option { - let name = path.expand_macro_expr()?; - // search local first - // FIXME: Remove public_macros check when we have a correct local_macors implementation - let local = - self.public_macros.get(&name).or_else(|| self.local_macros.get(&name)).map(|it| *it); - if local.is_some() { - return local; - } - - let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path); - res.resolved_def.right().map(|m| m.id) - } - // Returns Yes if we are sure that additions to `ItemMap` wouldn't change // the result. fn resolve_path_fp_with_macro( @@ -511,7 +486,7 @@ impl CrateDefMap { let from_scope = self[module] .scope .get_item_or_macro(name) - .unwrap_or_else(|| Either::Left(PerNs::none()));; + .unwrap_or_else(|| Either::Left(PerNs::none())); let from_extern_prelude = self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it)); let from_prelude = self.resolve_in_prelude(db, name); diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs index 3bfef799d..99110d58d 100644 --- a/crates/ra_hir/src/nameres/collector.rs +++ b/crates/ra_hir/src/nameres/collector.rs @@ -138,15 +138,35 @@ where } } - fn define_macro(&mut self, name: Name, macro_id: MacroDefId, export: bool) { + fn define_macro( + &mut self, + module_id: CrateModuleId, + name: Name, + macro_id: MacroDefId, + export: bool, + ) { + // macro-by-example in Rust have completely weird name resolution logic, + // unlike anything else in the language. We'd don't fully implement yet, + // just give a somewhat precise approximation. + // + // Specifically, we store a set of visible macros in each module, just + // like how we do with usual items. This is wrong, however, because + // macros can be shadowed and their scopes are mostly unrelated to + // modules. To paper over the second problem, we also maintain + // `global_macro_scope` which works when we construct `CrateDefMap`, but + // is completely ignored in expressions. + // + // What we should do is that, in CrateDefMap, we should maintain a + // separate tower of macro scopes, with ids. Then, for each item in the + // module, we need to store it's macro scope. + let def = Either::Right(MacroDef { id: macro_id }); + + // In Rust, `#[macro_export]` macros are unconditionally visible at the + // crate root, even if the parent modules is **not** visible. if export { - self.def_map.public_macros.insert(name.clone(), macro_id); - - let def = Either::Right(MacroDef { id: macro_id }); - self.update(self.def_map.root, None, &[(name.clone(), def)]); - } else { - self.def_map.local_macros.insert(name.clone(), macro_id); + self.update(self.def_map.root, None, &[(name.clone(), def.clone())]); } + self.update(module_id, None, &[(name.clone(), def)]); self.global_macro_scope.insert(name, macro_id); } @@ -589,7 +609,7 @@ where if is_macro_rules(&mac.path) { if let Some(name) = &mac.name { let macro_id = MacroDefId(mac.ast_id.with_file_id(self.file_id)); - self.def_collector.define_macro(name.clone(), macro_id, mac.export) + self.def_collector.define_macro(self.module_id, name.clone(), macro_id, mac.export) } return; } @@ -694,9 +714,7 @@ mod tests { prelude: None, root, modules, - public_macros: FxHashMap::default(), poison_macros: FxHashSet::default(), - local_macros: FxHashMap::default(), diagnostics: Vec::new(), } }; diff --git a/crates/ra_hir/src/nameres/tests/macros.rs b/crates/ra_hir/src/nameres/tests/macros.rs index 42241aeff..4e04740eb 100644 --- a/crates/ra_hir/src/nameres/tests/macros.rs +++ b/crates/ra_hir/src/nameres/tests/macros.rs @@ -21,6 +21,7 @@ fn macro_rules_are_globally_visible() { ⋮crate ⋮Foo: t v ⋮nested: t + ⋮structs: m ⋮ ⋮crate::nested ⋮Bar: t v @@ -46,6 +47,7 @@ fn macro_rules_can_define_modules() { ); assert_snapshot_matches!(map, @r###" ⋮crate + ⋮m: m ⋮n1: t ⋮ ⋮crate::n1 @@ -127,8 +129,11 @@ fn unexpanded_macro_should_expand_by_fixedpoint_loop() { "foo": ("/lib.rs", []), }, ); - assert_snapshot_matches!(map, @r###"crate -Foo: t v -bar: m -foo: m"###); + assert_snapshot_matches!(map, @r###" + ⋮crate + ⋮Foo: t v + ⋮bar: m + ⋮baz: m + ⋮foo: m + "###); } diff --git a/crates/ra_hir/src/resolve.rs b/crates/ra_hir/src/resolve.rs index 347bcf27d..1b987c1b6 100644 --- a/crates/ra_hir/src/resolve.rs +++ b/crates/ra_hir/src/resolve.rs @@ -2,11 +2,11 @@ use std::sync::Arc; use rustc_hash::{FxHashMap, FxHashSet}; +use either::Either; use crate::{ - ModuleDef, Trait, + ModuleDef, Trait, MacroDef, code_model::Crate, - MacroDefId, db::HirDatabase, name::{Name, KnownName}, nameres::{PerNs, CrateDefMap, CrateModuleId}, @@ -130,13 +130,16 @@ impl Resolver { resolution } - pub(crate) fn resolve_macro_call( + pub(crate) fn resolve_path_as_macro( &self, db: &impl HirDatabase, - path: Option, - ) -> Option { - let m = self.module()?; - m.0.find_macro(db, m.1, &path?) + path: &Path, + ) -> Option { + let (item_map, module) = self.module()?; + match item_map.resolve_path_with_macro(db, module, path) { + (Either::Right(macro_def), None) => Some(macro_def), + _ => None, + } } /// Returns the resolved path segments diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index 63ec59314..876ebe0e3 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs @@ -267,9 +267,8 @@ impl SourceAnalyzer { db: &impl HirDatabase, macro_call: &ast::MacroCall, ) -> Option { - let id = - self.resolver.resolve_macro_call(db, macro_call.path().and_then(Path::from_ast))?; - Some(MacroDef { id }) + let path = macro_call.path().and_then(Path::from_ast)?; + self.resolver.resolve_path_as_macro(db, &path) } pub fn resolve_hir_path( diff --git a/crates/ra_hir/src/ty/tests.rs b/crates/ra_hir/src/ty/tests.rs index c34e89af7..54b2a8c16 100644 --- a/crates/ra_hir/src/ty/tests.rs +++ b/crates/ra_hir/src/ty/tests.rs @@ -2481,8 +2481,10 @@ fn main() { } "#), @r###" -[156; 182) '{ ...,2); }': () -[166; 167) 'x': Foo"### + ⋮ + ⋮[156; 182) '{ ...,2); }': () + ⋮[166; 167) 'x': Foo + "### ); } -- cgit v1.2.3