From 5d43e752ad8d92f968364e5eeb343e89000308bd Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 2 May 2025 17:26:48 +0200 Subject: [PATCH] refactor: Simplify macro call id construction --- Cargo.toml | 1 + crates/hir-def/src/expr_store/expander.rs | 31 +++++- crates/hir-def/src/lib.rs | 99 ++----------------- .../hir-def/src/macro_expansion_tests/mbe.rs | 10 +- .../hir-def/src/macro_expansion_tests/mod.rs | 85 ++++++++-------- crates/hir-def/src/nameres/assoc.rs | 18 ++-- crates/hir-def/src/nameres/collector.rs | 42 +++----- crates/syntax/Cargo.toml | 2 +- 8 files changed, 113 insertions(+), 175 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dd39cf59bb44..c4c2fdf34bae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -131,6 +131,7 @@ process-wrap = { version = "8.2.0", features = ["std"] } pulldown-cmark-to-cmark = "10.0.4" pulldown-cmark = { version = "0.9.6", default-features = false } rayon = "1.10.0" +rowan = "=0.15.15" salsa = { version = "0.21.1", default-features = false, features = ["rayon","salsa_unstable"] } salsa-macros = "0.21.1" semver = "1.0.26" diff --git a/crates/hir-def/src/expr_store/expander.rs b/crates/hir-def/src/expr_store/expander.rs index d24f4b7028d7..3823fb5a1e75 100644 --- a/crates/hir-def/src/expr_store/expander.rs +++ b/crates/hir-def/src/expr_store/expander.rs @@ -5,20 +5,22 @@ use std::mem; use base_db::Crate; use cfg::CfgOptions; use drop_bomb::DropBomb; +use hir_expand::AstId; use hir_expand::{ ExpandError, ExpandErrorKind, ExpandResult, HirFileId, InFile, Lookup, MacroCallId, eager::EagerCallBackFn, mod_path::ModPath, span_map::SpanMap, }; use span::{AstIdMap, Edition, SyntaxContext}; use syntax::ast::HasAttrs; -use syntax::{Parse, ast}; +use syntax::{AstNode, Parse, ast}; use triomphe::Arc; use tt::TextRange; use crate::attr::Attrs; use crate::expr_store::HygieneId; +use crate::macro_call_as_call_id; use crate::nameres::DefMap; -use crate::{AsMacroCall, MacroId, UnresolvedMacro, db::DefDatabase}; +use crate::{MacroId, UnresolvedMacro, db::DefDatabase}; #[derive(Debug)] pub(super) struct Expander { @@ -92,8 +94,31 @@ impl Expander { let result = self.within_limit(db, |this| { let macro_call = this.in_file(¯o_call); - match macro_call.as_call_id_with_errors( + + let expands_to = hir_expand::ExpandTo::from_call_site(macro_call.value); + let ast_id = AstId::new(macro_call.file_id, this.ast_id_map().ast_id(macro_call.value)); + let path = macro_call.value.path().and_then(|path| { + let range = path.syntax().text_range(); + let mod_path = ModPath::from_src(db, path, &mut |range| { + this.span_map.span_for_range(range).ctx + })?; + let call_site = this.span_map.span_for_range(range); + Some((call_site, mod_path)) + }); + + let Some((call_site, path)) = path else { + return ExpandResult::only_err(ExpandError::other( + this.span_map.span_for_range(macro_call.value.syntax().text_range()), + "malformed macro invocation", + )); + }; + + match macro_call_as_call_id( db, + ast_id, + &path, + call_site.ctx, + expands_to, krate, |path| resolver(path).map(|it| db.macro_def(it)), eager_callback, diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index cf65e6355925..28011bda7c54 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -64,8 +64,8 @@ use std::hash::{Hash, Hasher}; use base_db::{Crate, impl_intern_key}; use hir_expand::{ - AstId, ExpandError, ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, - MacroDefId, MacroDefKind, + AstId, ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, + MacroDefKind, builtin::{BuiltinAttrExpander, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerExpander}, db::ExpandDatabase, eager::expand_eager_macro_input, @@ -79,7 +79,7 @@ use la_arena::Idx; use nameres::DefMap; use span::{AstIdNode, Edition, FileAstId, SyntaxContext}; use stdx::impl_from; -use syntax::{AstNode, ast}; +use syntax::ast; pub use hir_expand::{Intern, Lookup, tt}; @@ -1166,66 +1166,6 @@ impl ModuleDefId { }) } } - -// FIXME: Replace this with a plain function, it only has one impl -/// A helper trait for converting to MacroCallId -trait AsMacroCall { - fn as_call_id_with_errors( - &self, - db: &dyn ExpandDatabase, - krate: Crate, - resolver: impl Fn(&ModPath) -> Option + Copy, - eager_callback: &mut dyn FnMut( - InFile<(syntax::AstPtr, span::FileAstId)>, - MacroCallId, - ), - ) -> Result>, UnresolvedMacro>; -} - -impl AsMacroCall for InFile<&ast::MacroCall> { - fn as_call_id_with_errors( - &self, - db: &dyn ExpandDatabase, - krate: Crate, - resolver: impl Fn(&ModPath) -> Option + Copy, - eager_callback: &mut dyn FnMut( - InFile<(syntax::AstPtr, span::FileAstId)>, - MacroCallId, - ), - ) -> Result>, UnresolvedMacro> { - let expands_to = hir_expand::ExpandTo::from_call_site(self.value); - let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value)); - let span_map = db.span_map(self.file_id); - let path = self.value.path().and_then(|path| { - let range = path.syntax().text_range(); - let mod_path = ModPath::from_src(db, path, &mut |range| { - span_map.as_ref().span_for_range(range).ctx - })?; - let call_site = span_map.span_for_range(range); - Some((call_site, mod_path)) - }); - - let Some((call_site, path)) = path else { - return Ok(ExpandResult::only_err(ExpandError::other( - span_map.span_for_range(self.value.syntax().text_range()), - "malformed macro invocation", - ))); - }; - - macro_call_as_call_id_with_eager( - db, - ast_id, - &path, - call_site.ctx, - expands_to, - krate, - resolver, - resolver, - eager_callback, - ) - } -} - /// Helper wrapper for `AstId` with `ModPath` #[derive(Clone, Debug, Eq, PartialEq)] struct AstIdWithPath { @@ -1239,41 +1179,14 @@ impl AstIdWithPath { } } -fn macro_call_as_call_id( - db: &dyn ExpandDatabase, - call: &AstIdWithPath, - call_site: SyntaxContext, - expand_to: ExpandTo, - krate: Crate, - resolver: impl Fn(&ModPath) -> Option + Copy, - eager_callback: &mut dyn FnMut( - InFile<(syntax::AstPtr, span::FileAstId)>, - MacroCallId, - ), -) -> Result, UnresolvedMacro> { - macro_call_as_call_id_with_eager( - db, - call.ast_id, - &call.path, - call_site, - expand_to, - krate, - resolver, - resolver, - eager_callback, - ) - .map(|res| res.value) -} - -fn macro_call_as_call_id_with_eager( +pub fn macro_call_as_call_id( db: &dyn ExpandDatabase, ast_id: AstId, path: &ModPath, call_site: SyntaxContext, expand_to: ExpandTo, krate: Crate, - resolver: impl FnOnce(&ModPath) -> Option, - eager_resolver: impl Fn(&ModPath) -> Option, + resolver: impl Fn(&ModPath) -> Option + Copy, eager_callback: &mut dyn FnMut( InFile<(syntax::AstPtr, span::FileAstId)>, MacroCallId, @@ -1289,7 +1202,7 @@ fn macro_call_as_call_id_with_eager( ast_id, def, call_site, - &|path| eager_resolver(path).filter(MacroDefId::is_fn_like), + &|path| resolver(path).filter(MacroDefId::is_fn_like), eager_callback, ), _ if def.is_fn_like() => ExpandResult { diff --git a/crates/hir-def/src/macro_expansion_tests/mbe.rs b/crates/hir-def/src/macro_expansion_tests/mbe.rs index abb5bd5ed726..38fc4b3d118a 100644 --- a/crates/hir-def/src/macro_expansion_tests/mbe.rs +++ b/crates/hir-def/src/macro_expansion_tests/mbe.rs @@ -2001,8 +2001,9 @@ macro_rules! bug { true }; } - -let _ = bug!(a;;;test); +fn f() { + let _ = bug!(a;;;test); +} "#, expect![[r#" macro_rules! bug { @@ -2022,8 +2023,9 @@ macro_rules! bug { true }; } - -let _ = true; +fn f() { + let _ = true; +} "#]], ); } diff --git a/crates/hir-def/src/macro_expansion_tests/mod.rs b/crates/hir-def/src/macro_expansion_tests/mod.rs index 143b5df77305..800c96ebdae0 100644 --- a/crates/hir-def/src/macro_expansion_tests/mod.rs +++ b/crates/hir-def/src/macro_expansion_tests/mod.rs @@ -19,7 +19,7 @@ use std::{iter, ops::Range, sync}; use base_db::RootQueryDb; use expect_test::Expect; use hir_expand::{ - InFile, MacroCallKind, MacroKind, + AstId, InFile, MacroCallId, MacroCallKind, MacroKind, db::ExpandDatabase, proc_macro::{ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacroKind}, span_map::SpanMapRef, @@ -29,7 +29,7 @@ use itertools::Itertools; use span::{Edition, Span}; use stdx::{format_to, format_to_acc}; use syntax::{ - AstNode, + AstNode, AstPtr, SyntaxKind::{COMMENT, EOF, IDENT, LIFETIME_IDENT}, SyntaxNode, T, ast::{self, edit::IndentLevel}, @@ -37,10 +37,9 @@ use syntax::{ use test_fixture::WithFixture; use crate::{ - AdtId, AsMacroCall, Lookup, ModuleDefId, + AdtId, Lookup, ModuleDefId, db::DefDatabase, - nameres::{DefMap, MacroSubNs, ModuleSource}, - resolver::HasResolver, + nameres::{DefMap, ModuleSource}, src::HasSource, test_db::TestDB, tt::TopSubtree, @@ -78,7 +77,6 @@ fn check_errors(#[rust_analyzer::rust_fixture] ra_fixture: &str, expect: Expect) expect.assert_eq(&errors); } -#[track_caller] fn check(#[rust_analyzer::rust_fixture] ra_fixture: &str, mut expect: Expect) { let extra_proc_macros = vec![( r#" @@ -95,54 +93,59 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream disabled: false, }, )]; + + fn resolve( + db: &dyn DefDatabase, + def_map: &DefMap, + ast_id: AstId, + ast_ptr: InFile>, + ) -> Option { + def_map.modules().find_map(|module| { + for decl in + module.1.scope.declarations().chain(module.1.scope.unnamed_consts().map(Into::into)) + { + let body = match decl { + ModuleDefId::FunctionId(it) => it.into(), + ModuleDefId::ConstId(it) => it.into(), + ModuleDefId::StaticId(it) => it.into(), + _ => continue, + }; + + let (body, sm) = db.body_with_source_map(body); + if let Some(it) = + body.blocks(db).find_map(|block| resolve(db, &block.1, ast_id, ast_ptr)) + { + return Some(it); + } + if let Some((_, res)) = sm.macro_calls().find(|it| it.0 == ast_ptr) { + return Some(res); + } + } + module.1.scope.macro_invoc(ast_id) + }) + } + let db = TestDB::with_files_extra_proc_macros(ra_fixture, extra_proc_macros); let krate = db.fetch_test_crate(); let def_map = db.crate_def_map(krate); let local_id = DefMap::ROOT; - let module = def_map.module_id(local_id); - let resolver = module.resolver(&db); let source = def_map[local_id].definition_source(&db); let source_file = match source.value { ModuleSource::SourceFile(it) => it, ModuleSource::Module(_) | ModuleSource::BlockExpr(_) => panic!(), }; - // What we want to do is to replace all macros (fn-like, derive, attr) with - // their expansions. Turns out, we don't actually store enough information - // to do this precisely though! Specifically, if a macro expands to nothing, - // it leaves zero traces in def-map, so we can't get its expansion after the - // fact. - // - // This is the usual - // - // resolve/record tension! - // - // So here we try to do a resolve, which is necessary a heuristic. For macro - // calls, we use `as_call_id_with_errors`. For derives, we look at the impls - // in the module and assume that, if impls's source is a different - // `HirFileId`, than it came from macro expansion. - let mut text_edits = Vec::new(); let mut expansions = Vec::new(); - for macro_call in source_file.syntax().descendants().filter_map(ast::MacroCall::cast) { - let macro_call = InFile::new(source.file_id, ¯o_call); - let res = macro_call - .as_call_id_with_errors( - &db, - krate, - |path| { - resolver - .resolve_path_as_macro(&db, path, Some(MacroSubNs::Bang)) - .map(|(it, _)| db.macro_def(it)) - }, - &mut |_, _| (), - ) - .unwrap(); - let macro_call_id = res.value.unwrap(); - let mut expansion_result = db.parse_macro_expansion(macro_call_id); - expansion_result.err = expansion_result.err.or(res.err); - expansions.push((macro_call.value.clone(), expansion_result)); + for macro_call_node in source_file.syntax().descendants().filter_map(ast::MacroCall::cast) { + let ast_id = db.ast_id_map(source.file_id).ast_id(¯o_call_node); + let ast_id = InFile::new(source.file_id, ast_id); + let ptr = InFile::new(source.file_id, AstPtr::new(¯o_call_node)); + let macro_call_id = resolve(&db, &def_map, ast_id, ptr) + .unwrap_or_else(|| panic!("unable to find semantic macro call {macro_call_node}")); + let expansion_result = db.parse_macro_expansion(macro_call_id); + expansions.push((macro_call_node.clone(), expansion_result)); } for (call, exp) in expansions.into_iter().rev() { diff --git a/crates/hir-def/src/nameres/assoc.rs b/crates/hir-def/src/nameres/assoc.rs index b09706552947..448b908936a2 100644 --- a/crates/hir-def/src/nameres/assoc.rs +++ b/crates/hir-def/src/nameres/assoc.rs @@ -259,7 +259,8 @@ impl<'a> AssocItemCollector<'a> { }; match macro_call_as_call_id( self.db, - &AstIdWithPath::new(tree_id.file_id(), ast_id, Clone::clone(path)), + InFile::new(tree_id.file_id(), ast_id), + path, ctxt, expand_to, self.module_id.krate(), @@ -268,12 +269,15 @@ impl<'a> AssocItemCollector<'a> { self.macro_calls.push((ptr.map(|(_, it)| it.upcast()), call_id)) }, ) { - Ok(Some(call_id)) => { - self.macro_calls - .push((InFile::new(tree_id.file_id(), ast_id.upcast()), call_id)); - self.collect_macro_items(call_id); - } - Ok(None) => (), + // FIXME: Expansion error? + Ok(call_id) => match call_id.value { + Some(call_id) => { + self.macro_calls + .push((InFile::new(tree_id.file_id(), ast_id.upcast()), call_id)); + self.collect_macro_items(call_id); + } + None => (), + }, Err(_) => { self.diagnostics.push(DefDiagnostic::unresolved_macro_call( self.module_id.local_id, diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 77effbcc8800..8df0f092cd0b 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -39,7 +39,7 @@ use crate::{ ItemTreeId, ItemTreeNode, Macro2, MacroCall, MacroRules, Mod, ModItem, ModKind, TreeId, UseTreeKind, }, - macro_call_as_call_id, macro_call_as_call_id_with_eager, + macro_call_as_call_id, nameres::{ BuiltinShadowMode, DefMap, LocalDefMap, MacroSubNs, ModuleData, ModuleOrigin, ResolveMode, attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id}, @@ -1256,7 +1256,8 @@ impl DefCollector<'_> { MacroDirectiveKind::FnLike { ast_id, expand_to, ctxt: call_site } => { let call_id = macro_call_as_call_id( self.db, - ast_id, + ast_id.ast_id, + &ast_id.path, *call_site, *expand_to, self.def_map.krate, @@ -1265,15 +1266,18 @@ impl DefCollector<'_> { eager_callback_buffer.push((directive.module_id, ptr, call_id)); }, ); - if let Ok(Some(call_id)) = call_id { - self.def_map.modules[directive.module_id] - .scope - .add_macro_invoc(ast_id.ast_id, call_id); + if let Ok(call_id) = call_id { + // FIXME: Expansion error + if let Some(call_id) = call_id.value { + self.def_map.modules[directive.module_id] + .scope + .add_macro_invoc(ast_id.ast_id, call_id); - push_resolved(directive, call_id); + push_resolved(directive, call_id); - res = ReachedFixedPoint::No; - return Resolved::Yes; + res = ReachedFixedPoint::No; + return Resolved::Yes; + } } } MacroDirectiveKind::Derive { @@ -1542,7 +1546,8 @@ impl DefCollector<'_> { // FIXME: we shouldn't need to re-resolve the macro here just to get the unresolved error! let macro_call_as_call_id = macro_call_as_call_id( self.db, - ast_id, + ast_id.ast_id, + &ast_id.path, *call_site, *expand_to, self.def_map.krate, @@ -2420,7 +2425,7 @@ impl ModCollector<'_, '_> { let mut eager_callback_buffer = vec![]; // Case 1: try to resolve macro calls with single-segment name and expand macro_rules - if let Ok(res) = macro_call_as_call_id_with_eager( + if let Ok(res) = macro_call_as_call_id( db, ast_id.ast_id, &ast_id.path, @@ -2445,21 +2450,6 @@ impl ModCollector<'_, '_> { .map(|it| self.def_collector.db.macro_def(it)) }) }, - |path| { - let resolved_res = self.def_collector.def_map.resolve_path_fp_with_macro( - self.def_collector - .crate_local_def_map - .as_deref() - .unwrap_or(&self.def_collector.local_def_map), - db, - ResolveMode::Other, - self.module_id, - path, - BuiltinShadowMode::Module, - Some(MacroSubNs::Bang), - ); - resolved_res.resolved_def.take_macros().map(|it| db.macro_def(it)) - }, &mut |ptr, call_id| eager_callback_buffer.push((ptr, call_id)), ) { for (ptr, call_id) in eager_callback_buffer { diff --git a/crates/syntax/Cargo.toml b/crates/syntax/Cargo.toml index 510d44d00917..4c7704803ef3 100644 --- a/crates/syntax/Cargo.toml +++ b/crates/syntax/Cargo.toml @@ -14,7 +14,7 @@ rust-version.workspace = true [dependencies] either.workspace = true itertools.workspace = true -rowan = "=0.15.15" +rowan.workspace = true rustc-hash.workspace = true rustc-literal-escaper.workspace = true smol_str.workspace = true