diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 95581727ad..668b436e01 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -638,7 +638,6 @@ impl<'a> AssocItemCollector<'a> { self.items.push((item.name.clone(), def.into())); } AssocItem::MacroCall(call) => { - // TODO: These are the wrong errors to report, report in collect_macro_items instead let file_id = self.expander.current_file_id(); let root = self.db.parse_or_expand(file_id); if let Some(root) = root { diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs index 8546d36d79..48c1baf308 100644 --- a/crates/hir-def/src/item_tree.rs +++ b/crates/hir-def/src/item_tree.rs @@ -101,6 +101,7 @@ pub struct ItemTree { top_level: SmallVec<[ModItem; 1]>, attrs: FxHashMap, + // FIXME: Remove this indirection, an item tree is almost always non-empty? data: Option>, } diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 55e1e91220..34d704942a 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -823,7 +823,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> { return Ok(ExpandResult::only_err(ExpandError::Other("malformed macro invocation".into()))); }; - macro_call_as_call_id( + macro_call_as_call_id_( db, &AstIdWithPath::new(ast_id.file_id, ast_id.value, path), expands_to, @@ -852,6 +852,16 @@ fn macro_call_as_call_id( expand_to: ExpandTo, krate: CrateId, resolver: impl Fn(path::ModPath) -> Option, +) -> Result, UnresolvedMacro> { + macro_call_as_call_id_(db, call, expand_to, krate, resolver).map(|res| res.value) +} + +fn macro_call_as_call_id_( + db: &dyn db::DefDatabase, + call: &AstIdWithPath, + expand_to: ExpandTo, + krate: CrateId, + resolver: impl Fn(path::ModPath) -> Option, ) -> Result>, UnresolvedMacro> { let def = resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?; diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 8ab0b3dbd1..6592c6b902 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1117,8 +1117,9 @@ impl DefCollector<'_> { self.def_map.krate, resolver_def_id, ); - if let Ok(ExpandResult { value: Some(call_id), .. }) = call_id { + if let Ok(Some(call_id)) = call_id { push_resolved(directive, call_id); + res = ReachedFixedPoint::No; return false; } @@ -1354,26 +1355,30 @@ impl DefCollector<'_> { let file_id = macro_call_id.as_file(); // First, fetch the raw expansion result for purposes of error reporting. This goes through - // `macro_expand_error` to avoid depending on the full expansion result (to improve + // `parse_macro_expansion_error` to avoid depending on the full expansion result (to improve // incrementality). - let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id); - let err = self.db.macro_expand_error(macro_call_id); + let ExpandResult { value, err } = self.db.parse_macro_expansion_error(macro_call_id); if let Some(err) = err { + let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id); let diag = match err { + // why is this reported here? hir_expand::ExpandError::UnresolvedProcMacro(krate) => { always!(krate == loc.def.krate); - // Missing proc macros are non-fatal, so they are handled specially. DefDiagnostic::unresolved_proc_macro(module_id, loc.kind.clone(), loc.def.krate) } - _ => DefDiagnostic::macro_error(module_id, loc.kind, err.to_string()), + _ => DefDiagnostic::macro_error(module_id, loc.kind.clone(), err.to_string()), }; self.def_map.diagnostics.push(diag); } + if let Some(errors) = value { + let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id); + let diag = DefDiagnostic::macro_expansion_parse_error(module_id, loc.kind, &errors); + self.def_map.diagnostics.push(diag); + } // Then, fetch and process the item tree. This will reuse the expansion result from above. let item_tree = self.db.file_item_tree(file_id); - // FIXME: report parse errors for the macro expansion here let mod_dir = self.mod_dirs[&module_id].clone(); ModCollector { @@ -1395,6 +1400,7 @@ impl DefCollector<'_> { for directive in &self.unresolved_macros { match &directive.kind { MacroDirectiveKind::FnLike { ast_id, expand_to } => { + // 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, @@ -2110,7 +2116,7 @@ impl ModCollector<'_, '_> { let ast_id = AstIdWithPath::new(self.file_id(), mac.ast_id, ModPath::clone(&mac.path)); // Case 1: try to resolve in legacy scope and expand macro_rules - match macro_call_as_call_id( + if let Ok(res) = macro_call_as_call_id( self.def_collector.db, &ast_id, mac.expand_to, @@ -2131,29 +2137,18 @@ impl ModCollector<'_, '_> { }) }, ) { - Ok(res) => { - // Legacy macros need to be expanded immediately, so that any macros they produce - // are in scope. - if let Some(val) = res.value { - self.def_collector.collect_macro_expansion( - self.module_id, - val, - self.macro_depth + 1, - container, - ); - } - - if let Some(err) = res.err { - self.def_collector.def_map.diagnostics.push(DefDiagnostic::macro_error( - self.module_id, - MacroCallKind::FnLike { ast_id: ast_id.ast_id, expand_to: mac.expand_to }, - err.to_string(), - )); - } - - return; + // Legacy macros need to be expanded immediately, so that any macros they produce + // are in scope. + if let Some(val) = res { + self.def_collector.collect_macro_expansion( + self.module_id, + val, + self.macro_depth + 1, + container, + ); } - Err(UnresolvedMacro { .. }) => (), + + return; } // Case 2: resolve in module scope, expand during name resolution. diff --git a/crates/hir-def/src/nameres/tests/incremental.rs b/crates/hir-def/src/nameres/tests/incremental.rs index 13e6825f82..b07462cde0 100644 --- a/crates/hir-def/src/nameres/tests/incremental.rs +++ b/crates/hir-def/src/nameres/tests/incremental.rs @@ -140,7 +140,7 @@ m!(Z); let n_recalculated_item_trees = events.iter().filter(|it| it.contains("item_tree")).count(); assert_eq!(n_recalculated_item_trees, 6); let n_reparsed_macros = - events.iter().filter(|it| it.contains("parse_macro_expansion")).count(); + events.iter().filter(|it| it.contains("parse_macro_expansion(")).count(); assert_eq!(n_reparsed_macros, 3); } diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 1749698387..afc2be0741 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -9,7 +9,7 @@ use mbe::syntax_node_to_token_tree; use rustc_hash::FxHashSet; use syntax::{ ast::{self, HasAttrs, HasDocComments}, - AstNode, GreenNode, Parse, SyntaxNode, SyntaxToken, T, + AstNode, GreenNode, Parse, SyntaxError, SyntaxNode, SyntaxToken, T, }; use crate::{ @@ -132,15 +132,18 @@ pub trait ExpandDatabase: SourceDatabase { /// just fetches procedural ones. fn macro_def(&self, id: MacroDefId) -> Result, mbe::ParseError>; - /// Expand macro call to a token tree. This query is LRUed (we keep 128 or so results in memory) + /// Expand macro call to a token tree. fn macro_expand(&self, macro_call: MacroCallId) -> ExpandResult>>; /// Special case of the previous query for procedural macros. We can't LRU /// proc macros, since they are not deterministic in general, and /// non-determinism breaks salsa in a very, very, very bad way. @edwin0cheng /// heroically debugged this once! fn expand_proc_macro(&self, call: MacroCallId) -> ExpandResult; - /// Firewall query that returns the error from the `macro_expand` query. - fn macro_expand_error(&self, macro_call: MacroCallId) -> Option; + /// Firewall query that returns the errors from the `parse_macro_expansion` query. + fn parse_macro_expansion_error( + &self, + macro_call: MacroCallId, + ) -> ExpandResult>>; fn hygiene_frame(&self, file_id: HirFileId) -> Arc; } @@ -448,6 +451,7 @@ fn macro_def( fn macro_expand( db: &dyn ExpandDatabase, id: MacroCallId, + // FIXME: Remove the OPtion if possible ) -> ExpandResult>> { let _p = profile::span("macro_expand"); let loc: MacroCallLoc = db.lookup_intern_macro_call(id); @@ -498,8 +502,12 @@ fn macro_expand( ExpandResult { value: Some(Arc::new(tt)), err } } -fn macro_expand_error(db: &dyn ExpandDatabase, macro_call: MacroCallId) -> Option { - db.macro_expand(macro_call).err +fn parse_macro_expansion_error( + db: &dyn ExpandDatabase, + macro_call_id: MacroCallId, +) -> ExpandResult>> { + db.parse_macro_expansion(MacroFile { macro_call_id }) + .map(|it| it.map(|(it, _)| it.errors().to_vec().into_boxed_slice())) } fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult { diff --git a/crates/hir/src/db.rs b/crates/hir/src/db.rs index 0935b5ea51..7ec27af04b 100644 --- a/crates/hir/src/db.rs +++ b/crates/hir/src/db.rs @@ -6,8 +6,8 @@ pub use hir_def::db::*; pub use hir_expand::db::{ AstIdMapQuery, ExpandDatabase, ExpandDatabaseStorage, ExpandProcMacroQuery, HygieneFrameQuery, - InternMacroCallQuery, MacroArgTextQuery, MacroDefQuery, MacroExpandErrorQuery, - MacroExpandQuery, ParseMacroExpansionQuery, + InternMacroCallQuery, MacroArgTextQuery, MacroDefQuery, MacroExpandQuery, + ParseMacroExpansionQuery, }; pub use hir_ty::db::*; diff --git a/crates/ide-db/src/apply_change.rs b/crates/ide-db/src/apply_change.rs index 3b8458980c..8d14371d03 100644 --- a/crates/ide-db/src/apply_change.rs +++ b/crates/ide-db/src/apply_change.rs @@ -80,7 +80,6 @@ impl RootDatabase { hir::db::MacroDefQuery hir::db::MacroExpandQuery hir::db::ExpandProcMacroQuery - hir::db::MacroExpandErrorQuery hir::db::HygieneFrameQuery // DefDatabase diff --git a/crates/ide-db/src/lib.rs b/crates/ide-db/src/lib.rs index 11b2a5e1c2..12b6e3c4bb 100644 --- a/crates/ide-db/src/lib.rs +++ b/crates/ide-db/src/lib.rs @@ -152,7 +152,6 @@ impl RootDatabase { let lru_capacity = lru_capacity.unwrap_or(base_db::DEFAULT_LRU_CAP); base_db::ParseQuery.in_db_mut(self).set_lru_capacity(lru_capacity); hir::db::ParseMacroExpansionQuery.in_db_mut(self).set_lru_capacity(lru_capacity); - hir::db::MacroExpandQuery.in_db_mut(self).set_lru_capacity(lru_capacity); } pub fn update_lru_capacities(&mut self, lru_capacities: &FxHashMap, usize>) { @@ -167,12 +166,6 @@ impl RootDatabase { .copied() .unwrap_or(base_db::DEFAULT_LRU_CAP), ); - hir_db::MacroExpandQuery.in_db_mut(self).set_lru_capacity( - lru_capacities - .get(stringify!(MacroExpandQuery)) - .copied() - .unwrap_or(base_db::DEFAULT_LRU_CAP), - ); macro_rules! update_lru_capacity_per_query { ($( $module:ident :: $query:ident )*) => {$( @@ -201,7 +194,6 @@ impl RootDatabase { hir_db::MacroDefQuery // hir_db::MacroExpandQuery hir_db::ExpandProcMacroQuery - hir_db::MacroExpandErrorQuery hir_db::HygieneFrameQuery // DefDatabase diff --git a/crates/ide-diagnostics/src/handlers/macro_error.rs b/crates/ide-diagnostics/src/handlers/macro_error.rs index af74015cf9..7547779a95 100644 --- a/crates/ide-diagnostics/src/handlers/macro_error.rs +++ b/crates/ide-diagnostics/src/handlers/macro_error.rs @@ -238,6 +238,22 @@ fn f() { //^^^ error: invalid macro definition: expected subtree } +"#, + ) + } + + #[test] + fn expansion_syntax_diagnostic() { + check_diagnostics( + r#" +macro_rules! foo { + () => { struct; }; +} + +fn f() { + foo!(); + //^^^ error: Syntax Error in Expansion: expected a name +} "#, ) }