From c1cb5953820f26d4d0a614650bc8c50cbc5a3ce6 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 15 Dec 2020 15:37:37 +0100 Subject: Move to upstream `macro_rules!` model --- crates/hir_def/src/body/lower.rs | 134 ++++++++++++++++---------------- crates/hir_def/src/item_tree.rs | 19 ++++- crates/hir_def/src/item_tree/lower.rs | 24 ++++-- crates/hir_def/src/nameres/collector.rs | 65 +++++++--------- crates/hir_def/src/path/lower.rs | 8 +- 5 files changed, 132 insertions(+), 118 deletions(-) (limited to 'crates/hir_def/src') diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs index 6c0de3ee8..bdba4c33e 100644 --- a/crates/hir_def/src/body/lower.rs +++ b/crates/hir_def/src/body/lower.rs @@ -566,66 +566,52 @@ impl ExprCollector<'_> { syntax_ptr: AstPtr, mut collector: F, ) { - if let Some(name) = e.is_macro_rules().map(|it| it.as_name()) { - let mac = MacroDefId { - krate: Some(self.expander.module.krate), - ast_id: Some(self.expander.ast_id(&e)), - kind: MacroDefKind::Declarative, - local_inner: false, - }; - self.body.item_scope.define_legacy_macro(name, mac); + // File containing the macro call. Expansion errors will be attached here. + let outer_file = self.expander.current_file_id; - // FIXME: do we still need to allocate this as missing ? - collector(self, None); - } else { - // File containing the macro call. Expansion errors will be attached here. - let outer_file = self.expander.current_file_id; - - let macro_call = self.expander.to_source(AstPtr::new(&e)); - let res = self.expander.enter_expand(self.db, Some(&self.body.item_scope), e); - - match &res.err { - Some(ExpandError::UnresolvedProcMacro) => { - self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro( - UnresolvedProcMacro { - file: outer_file, - node: syntax_ptr.into(), - precise_location: None, - macro_name: None, - }, - )); - } - Some(err) => { - self.source_map.diagnostics.push(BodyDiagnostic::MacroError(MacroError { + let macro_call = self.expander.to_source(AstPtr::new(&e)); + let res = self.expander.enter_expand(self.db, Some(&self.body.item_scope), e); + + match &res.err { + Some(ExpandError::UnresolvedProcMacro) => { + self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro( + UnresolvedProcMacro { file: outer_file, node: syntax_ptr.into(), - message: err.to_string(), - })); - } - None => {} + precise_location: None, + macro_name: None, + }, + )); + } + Some(err) => { + self.source_map.diagnostics.push(BodyDiagnostic::MacroError(MacroError { + file: outer_file, + node: syntax_ptr.into(), + message: err.to_string(), + })); } + None => {} + } - match res.value { - Some((mark, expansion)) => { - // FIXME: Statements are too complicated to recover from error for now. - // It is because we don't have any hygenine for local variable expansion right now. - if T::can_cast(syntax::SyntaxKind::MACRO_STMTS) && res.err.is_some() { - self.expander.exit(self.db, mark); - collector(self, None); - } else { - self.source_map - .expansions - .insert(macro_call, self.expander.current_file_id); + match res.value { + Some((mark, expansion)) => { + // FIXME: Statements are too complicated to recover from error for now. + // It is because we don't have any hygenine for local variable expansion right now. + if T::can_cast(syntax::SyntaxKind::MACRO_STMTS) && res.err.is_some() { + self.expander.exit(self.db, mark); + collector(self, None); + } else { + self.source_map.expansions.insert(macro_call, self.expander.current_file_id); - let item_tree = self.db.item_tree(self.expander.current_file_id); - self.item_trees.insert(self.expander.current_file_id, item_tree); + let item_tree = self.db.item_tree(self.expander.current_file_id); + self.item_trees.insert(self.expander.current_file_id, item_tree); - collector(self, Some(expansion)); - self.expander.exit(self.db, mark); - } + let id = collector(self, Some(expansion)); + self.expander.exit(self.db, mark); + id } - None => collector(self, None), } + None => collector(self, None), } } @@ -785,26 +771,44 @@ impl ExprCollector<'_> { | ast::Item::ExternCrate(_) | ast::Item::Module(_) | ast::Item::MacroCall(_) => return None, + ast::Item::MacroRules(def) => { + return Some(Either::Right(def)); + } }; - Some((def, name)) + Some(Either::Left((def, name))) }) .collect::>(); - for (def, name) in items { - self.body.item_scope.define_def(def); - if let Some(name) = name { - let vis = crate::visibility::Visibility::Public; // FIXME determine correctly - let has_constructor = match def { - ModuleDefId::AdtId(AdtId::StructId(s)) => { - self.db.struct_data(s).variant_data.kind() != StructKind::Record + for either in items { + match either { + Either::Left((def, name)) => { + self.body.item_scope.define_def(def); + if let Some(name) = name { + let vis = crate::visibility::Visibility::Public; // FIXME determine correctly + let has_constructor = match def { + ModuleDefId::AdtId(AdtId::StructId(s)) => { + self.db.struct_data(s).variant_data.kind() != StructKind::Record + } + _ => true, + }; + self.body.item_scope.push_res( + name.as_name(), + crate::per_ns::PerNs::from_def(def, vis, has_constructor), + ); } - _ => true, - }; - self.body.item_scope.push_res( - name.as_name(), - crate::per_ns::PerNs::from_def(def, vis, has_constructor), - ); + } + Either::Right(e) => { + let mac = MacroDefId { + krate: Some(self.expander.module.krate), + ast_id: Some(self.expander.ast_id(&e)), + kind: MacroDefKind::Declarative, + local_inner: false, + }; + if let Some(name) = e.name() { + self.body.item_scope.define_legacy_macro(name.as_name(), mac); + } + } } } } diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs index 864fad170..1c9babf37 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs @@ -142,6 +142,7 @@ impl ItemTree { type_aliases, mods, macro_calls, + macro_rules, exprs, vis, generics, @@ -162,6 +163,7 @@ impl ItemTree { type_aliases.shrink_to_fit(); mods.shrink_to_fit(); macro_calls.shrink_to_fit(); + macro_rules.shrink_to_fit(); exprs.shrink_to_fit(); vis.arena.shrink_to_fit(); @@ -280,6 +282,7 @@ struct ItemTreeData { type_aliases: Arena, mods: Arena, macro_calls: Arena, + macro_rules: Arena, exprs: Arena, vis: ItemVisibilities, @@ -427,6 +430,7 @@ mod_items! { TypeAlias in type_aliases -> ast::TypeAlias, Mod in mods -> ast::Module, MacroCall in macro_calls -> ast::MacroCall, + MacroRules in macro_rules -> ast::MacroRules, } macro_rules! impl_index { @@ -629,17 +633,22 @@ pub enum ModKind { #[derive(Debug, Clone, Eq, PartialEq)] pub struct MacroCall { - /// For `macro_rules!` declarations, this is the name of the declared macro. - pub name: Option, /// Path to the called macro. pub path: ModPath, + pub ast_id: FileAstId, +} + +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct MacroRules { + /// For `macro_rules!` declarations, this is the name of the declared macro. + pub name: Name, /// Has `#[macro_export]`. pub is_export: bool, /// Has `#[macro_export(local_inner_macros)]`. pub is_local_inner: bool, /// Has `#[rustc_builtin_macro]`. pub is_builtin: bool, - pub ast_id: FileAstId, + pub ast_id: FileAstId, } // NB: There's no `FileAstId` for `Expr`. The only case where this would be useful is for array @@ -670,7 +679,8 @@ impl ModItem { | ModItem::Static(_) | ModItem::Trait(_) | ModItem::Impl(_) - | ModItem::Mod(_) => None, + | ModItem::Mod(_) + | ModItem::MacroRules(_) => None, ModItem::MacroCall(call) => Some(AssocItem::MacroCall(*call)), ModItem::Const(konst) => Some(AssocItem::Const(*konst)), ModItem::TypeAlias(alias) => Some(AssocItem::TypeAlias(*alias)), @@ -697,6 +707,7 @@ impl ModItem { ModItem::TypeAlias(it) => tree[it.index].ast_id().upcast(), ModItem::Mod(it) => tree[it.index].ast_id().upcast(), ModItem::MacroCall(it) => tree[it.index].ast_id().upcast(), + ModItem::MacroRules(it) => tree[it.index].ast_id().upcast(), } } } diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs index 2939c6b1e..b39d7fb7a 100644 --- a/crates/hir_def/src/item_tree/lower.rs +++ b/crates/hir_def/src/item_tree/lower.rs @@ -84,8 +84,7 @@ impl Ctx { | ast::Item::Fn(_) | ast::Item::TypeAlias(_) | ast::Item::Const(_) - | ast::Item::Static(_) - | ast::Item::MacroCall(_) => { + | ast::Item::Static(_) => { // Skip this if we're already collecting inner items. We'll descend into all nodes // already. if !inner { @@ -98,7 +97,11 @@ impl Ctx { ast::Item::Trait(_) | ast::Item::Impl(_) | ast::Item::ExternBlock(_) => {} // These don't have inner items. - ast::Item::Module(_) | ast::Item::ExternCrate(_) | ast::Item::Use(_) => {} + ast::Item::Module(_) + | ast::Item::ExternCrate(_) + | ast::Item::Use(_) + | ast::Item::MacroCall(_) + | ast::Item::MacroRules(_) => {} }; let attrs = Attrs::new(item, &self.hygiene); @@ -118,6 +121,7 @@ impl Ctx { )), ast::Item::ExternCrate(ast) => self.lower_extern_crate(ast).map(Into::into), ast::Item::MacroCall(ast) => self.lower_macro_call(ast).map(Into::into), + ast::Item::MacroRules(ast) => self.lower_macro_rules(ast).map(Into::into), ast::Item::ExternBlock(ast) => { Some(ModItems(self.lower_extern_block(ast).into_iter().collect::>())) } @@ -525,9 +529,15 @@ impl Ctx { } fn lower_macro_call(&mut self, m: &ast::MacroCall) -> Option> { - let name = m.name().map(|it| it.as_name()); - let attrs = Attrs::new(m, &self.hygiene); let path = ModPath::from_src(m.path()?, &self.hygiene)?; + let ast_id = self.source_ast_id_map.ast_id(m); + let res = MacroCall { path, ast_id }; + Some(id(self.data().macro_calls.alloc(res))) + } + + fn lower_macro_rules(&mut self, m: &ast::MacroRules) -> Option> { + let name = m.name().map(|it| it.as_name())?; + let attrs = Attrs::new(m, &self.hygiene); let ast_id = self.source_ast_id_map.ast_id(m); @@ -547,8 +557,8 @@ impl Ctx { }; let is_builtin = attrs.by_key("rustc_builtin_macro").exists(); - let res = MacroCall { name, path, is_export, is_builtin, is_local_inner, ast_id }; - Some(id(self.data().macro_calls.alloc(res))) + let res = MacroRules { name, is_export, is_builtin, is_local_inner, ast_id }; + Some(id(self.data().macro_rules.alloc(res))) } fn lower_extern_block(&mut self, block: &ast::ExternBlock) -> Vec { diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index 19cd713ba..85cc342c4 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -11,7 +11,7 @@ use hir_expand::{ ast_id_map::FileAstId, builtin_derive::find_builtin_derive, builtin_macro::find_builtin_macro, - name::{name, AsName, Name}, + name::{AsName, Name}, proc_macro::ProcMacroExpander, HirFileId, MacroCallId, MacroDefId, MacroDefKind, }; @@ -25,7 +25,9 @@ use crate::{ attr::Attrs, db::DefDatabase, item_scope::{ImportType, PerNsGlobImports}, - item_tree::{self, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind}, + item_tree::{ + self, ItemTree, ItemTreeId, MacroCall, MacroRules, Mod, ModItem, ModKind, StructDefKind, + }, nameres::{ diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint, BuiltinShadowMode, CrateDefMap, ModuleData, ModuleOrigin, ResolveMode, @@ -972,7 +974,8 @@ impl ModCollector<'_, '_> { status: PartialResolvedImport::Unresolved, }) } - ModItem::MacroCall(mac) => self.collect_macro(&self.item_tree[mac]), + ModItem::MacroCall(mac) => self.collect_macro_call(&self.item_tree[mac]), + ModItem::MacroRules(mac) => self.collect_macro_rules(&self.item_tree[mac]), ModItem::Impl(imp) => { let module = ModuleId { krate: self.def_collector.def_map.krate, @@ -1276,45 +1279,37 @@ impl ModCollector<'_, '_> { 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()); + fn collect_macro_rules(&mut self, mac: &MacroRules) { + let ast_id = InFile::new(self.file_id, mac.ast_id); - // Case 0: builtin macros + // Case 1: builtin macros if mac.is_builtin { - if let Some(name) = &mac.name { - let krate = self.def_collector.def_map.krate; - if let Some(macro_id) = find_builtin_macro(name, krate, ast_id.ast_id) { - self.def_collector.define_macro( - self.module_id, - name.clone(), - macro_id, - mac.is_export, - ); - return; - } - } - } - - // Case 1: macro rules, define a macro in crate-global mutable scope - if is_macro_rules(&mac.path) { - if let Some(name) = &mac.name { - let macro_id = MacroDefId { - ast_id: Some(ast_id.ast_id), - krate: Some(self.def_collector.def_map.krate), - kind: MacroDefKind::Declarative, - local_inner: mac.is_local_inner, - }; + let krate = self.def_collector.def_map.krate; + if let Some(macro_id) = find_builtin_macro(&mac.name, krate, ast_id) { self.def_collector.define_macro( self.module_id, - name.clone(), + mac.name.clone(), macro_id, mac.is_export, ); + return; } - return; } - // Case 2: try to resolve in legacy scope and expand macro_rules + // Case 2: normal `macro_rules!` macro + let macro_id = MacroDefId { + ast_id: Some(ast_id), + krate: Some(self.def_collector.def_map.krate), + kind: MacroDefKind::Declarative, + local_inner: mac.is_local_inner, + }; + self.def_collector.define_macro(self.module_id, mac.name.clone(), macro_id, mac.is_export); + } + + fn collect_macro_call(&mut self, mac: &MacroCall) { + let mut ast_id = AstIdWithPath::new(self.file_id, mac.ast_id, mac.path.clone()); + + // Case 1: try to resolve in legacy scope and expand macro_rules if let Some(macro_call_id) = ast_id.as_call_id(self.def_collector.db, self.def_collector.def_map.krate, |path| { path.as_ident().and_then(|name| { @@ -1332,7 +1327,7 @@ impl ModCollector<'_, '_> { return; } - // Case 3: resolve in module scope, expand during name resolution. + // Case 2: resolve in module scope, expand during name resolution. // We rewrite simple path `macro_name` to `self::macro_name` to force resolve in module scope only. if ast_id.path.is_ident() { ast_id.path.kind = PathKind::Super(0); @@ -1370,10 +1365,6 @@ impl ModCollector<'_, '_> { } } -fn is_macro_rules(path: &ModPath) -> bool { - path.as_ident() == Some(&name![macro_rules]) -} - #[cfg(test)] mod tests { use crate::{db::DefDatabase, test_db::TestDB}; diff --git a/crates/hir_def/src/path/lower.rs b/crates/hir_def/src/path/lower.rs index 60fa7646b..609925012 100644 --- a/crates/hir_def/src/path/lower.rs +++ b/crates/hir_def/src/path/lower.rs @@ -122,11 +122,9 @@ pub(super) fn lower_path(mut path: ast::Path, hygiene: &Hygiene) -> Option // https://github.com/rust-lang/rust/blob/614f273e9388ddd7804d5cbc80b8865068a3744e/src/librustc_resolve/macros.rs#L456 // We follow what it did anyway :) if segments.len() == 1 && kind == PathKind::Plain { - if let Some(macro_call) = path.syntax().parent().and_then(ast::MacroCall::cast) { - if macro_call.is_bang() { - if let Some(crate_id) = hygiene.local_inner_macros() { - kind = PathKind::DollarCrate(crate_id); - } + if let Some(_macro_call) = path.syntax().parent().and_then(ast::MacroCall::cast) { + if let Some(crate_id) = hygiene.local_inner_macros() { + kind = PathKind::DollarCrate(crate_id); } } } -- cgit v1.2.3