Auto merge of #14585 - Veykril:macro-def-err, r=Veykril

Make `ExpandDatabase::parse_macro_expansion` and `ExpandDatabase::parse_or_expand` infallible

Author: bors

crates/hir-def/src/attr.rs

@@ -498,7 +498,7 @@ impl AttrsWithOwner {
             AttrDefId::FieldId(id) => {
                 let map = db.fields_attrs_source_map(id.parent);
                 let file_id = id.parent.file_id(db);
-                let root = db.parse_or_expand(file_id).unwrap();
+                let root = db.parse_or_expand(file_id);
                 let owner = match &map[id.local_id] {
                     Either::Left(it) => ast::AnyHasAttrs::new(it.to_node(&root)),
                     Either::Right(it) => ast::AnyHasAttrs::new(it.to_node(&root)),
@@ -514,7 +514,7 @@ impl AttrsWithOwner {
             AttrDefId::EnumVariantId(id) => {
                 let map = db.variants_attrs_source_map(id.parent);
                 let file_id = id.parent.lookup(db).id.file_id();
-                let root = db.parse_or_expand(file_id).unwrap();
+                let root = db.parse_or_expand(file_id);
                 InFile::new(file_id, ast::AnyHasAttrs::new(map[id.local_id].to_node(&root)))
             }
             AttrDefId::StaticId(id) => id.lookup(db).source(db).map(ast::AnyHasAttrs::new),

crates/hir-def/src/body.rs

@@ -174,30 +174,12 @@ impl Expander {
     fn enter_expand_inner(
         db: &dyn DefDatabase,
         call_id: MacroCallId,
-        mut error: Option<ExpandError>,
+        error: Option<ExpandError>,
     ) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
         let file_id = call_id.as_file();
         let ExpandResult { value, err } = db.parse_or_expand_with_err(file_id);
 
-        if error.is_none() {
-            error = err;
-        }
-
-        let parse = match value {
-            Some(it) => it,
-            None => {
-                // Only `None` if the macro expansion produced no usable AST.
-                if error.is_none() {
-                    tracing::warn!("no error despite `parse_or_expand` failing");
-                }
-
-                return ExpandResult::only_err(error.unwrap_or_else(|| {
-                    ExpandError::Other("failed to parse macro invocation".into())
-                }));
-            }
-        };
-
-        ExpandResult { value: Some(InFile::new(file_id, parse)), err: error }
+        ExpandResult { value: Some(InFile::new(file_id, value)), err: error.or(err) }
     }
 
     pub fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {

crates/hir-def/src/data.rs

@@ -640,22 +640,20 @@ impl<'a> AssocItemCollector<'a> {
                 AssocItem::MacroCall(call) => {
                     let file_id = self.expander.current_file_id();
                     let root = self.db.parse_or_expand(file_id);
-                    if let Some(root) = root {
-                        let call = &item_tree[call];
-
-                        let ast_id_map = self.db.ast_id_map(file_id);
-                        let macro_call = ast_id_map.get(call.ast_id).to_node(&root);
-                        let _cx = stdx::panic_context::enter(format!(
-                            "collect_items MacroCall: {macro_call}"
-                        ));
-                        if let Ok(res) =
-                            self.expander.enter_expand::<ast::MacroItems>(self.db, macro_call)
-                        {
-                            self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike {
-                                ast_id: InFile::new(file_id, call.ast_id),
-                                expand_to: hir_expand::ExpandTo::Items,
-                            });
-                        }
+                    let call = &item_tree[call];
+
+                    let ast_id_map = self.db.ast_id_map(file_id);
+                    let macro_call = ast_id_map.get(call.ast_id).to_node(&root);
+                    let _cx = stdx::panic_context::enter(format!(
+                        "collect_items MacroCall: {macro_call}"
+                    ));
+                    if let Ok(res) =
+                        self.expander.enter_expand::<ast::MacroItems>(self.db, macro_call)
+                    {
+                        self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike {
+                            ast_id: InFile::new(file_id, call.ast_id),
+                            expand_to: hir_expand::ExpandTo::Items,
+                        });
                     }
                 }
             }

crates/hir-def/src/item_tree.rs

@@ -108,10 +108,7 @@ pub struct ItemTree {
 impl ItemTree {
     pub(crate) fn file_item_tree_query(db: &dyn DefDatabase, file_id: HirFileId) -> Arc<ItemTree> {
         let _p = profile::span("file_item_tree_query").detail(|| format!("{file_id:?}"));
-        let syntax = match db.parse_or_expand(file_id) {
-            Some(node) => node,
-            None => return Default::default(),
-        };
+        let syntax = db.parse_or_expand(file_id);
         if never!(syntax.kind() == SyntaxKind::ERROR, "{:?} from {:?} {}", file_id, syntax, syntax)
         {
             // FIXME: not 100% sure why these crop up, but return an empty tree to avoid a panic

crates/hir-def/src/macro_expansion_tests/mbe/matching.rs

@@ -33,7 +33,7 @@ m!(&k");
 "#,
         expect![[r#"
 macro_rules! m { ($i:literal) => {}; }
-/* error: Failed to lower macro args to token tree */"#]],
+/* error: invalid token tree */"#]],
     );
 }
 

crates/hir-def/src/macro_expansion_tests/mbe/tt_conversion.rs

@@ -98,7 +98,7 @@ macro_rules! m1 { ($x:ident) => { ($x } }
 macro_rules! m2 { ($x:ident) => {} }
 
 /* error: invalid macro definition: expected subtree */
-/* error: Failed to lower macro args to token tree */
+/* error: invalid token tree */
 "#]],
     )
 }

crates/hir-def/src/macro_expansion_tests/mod.rs

@@ -151,34 +151,33 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream
         if let Some(err) = exp.err {
             format_to!(expn_text, "/* error: {} */", err);
         }
-        if let Some((parse, token_map)) = exp.value {
-            if expect_errors {
-                assert!(!parse.errors().is_empty(), "no parse errors in expansion");
-                for e in parse.errors() {
-                    format_to!(expn_text, "/* parse error: {} */\n", e);
-                }
-            } else {
-                assert!(
-                    parse.errors().is_empty(),
-                    "parse errors in expansion: \n{:#?}",
-                    parse.errors()
-                );
+        let (parse, token_map) = exp.value;
+        if expect_errors {
+            assert!(!parse.errors().is_empty(), "no parse errors in expansion");
+            for e in parse.errors() {
+                format_to!(expn_text, "/* parse error: {} */\n", e);
             }
-            let pp = pretty_print_macro_expansion(
-                parse.syntax_node(),
-                show_token_ids.then_some(&*token_map),
+        } else {
+            assert!(
+                parse.errors().is_empty(),
+                "parse errors in expansion: \n{:#?}",
+                parse.errors()
             );
-            let indent = IndentLevel::from_node(call.syntax());
-            let pp = reindent(indent, pp);
-            format_to!(expn_text, "{}", pp);
+        }
+        let pp = pretty_print_macro_expansion(
+            parse.syntax_node(),
+            show_token_ids.then_some(&*token_map),
+        );
+        let indent = IndentLevel::from_node(call.syntax());
+        let pp = reindent(indent, pp);
+        format_to!(expn_text, "{}", pp);
 
-            if tree {
-                let tree = format!("{:#?}", parse.syntax_node())
-                    .split_inclusive('\n')
-                    .map(|line| format!("// {line}"))
-                    .collect::<String>();
-                format_to!(expn_text, "\n{}", tree)
-            }
+        if tree {
+            let tree = format!("{:#?}", parse.syntax_node())
+                .split_inclusive('\n')
+                .map(|line| format!("// {line}"))
+                .collect::<String>();
+            format_to!(expn_text, "\n{}", tree)
         }
         let range = call.syntax().text_range();
         let range: Range<usize> = range.into();

crates/hir-def/src/nameres/collector.rs

@@ -1371,7 +1371,7 @@ impl DefCollector<'_> {
 
             self.def_map.diagnostics.push(diag);
         }
-        if let Some(errors) = value {
+        if let 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);

crates/hir-def/src/nameres/tests/incremental.rs

@@ -109,7 +109,7 @@ fn typing_inside_a_macro_should_not_invalidate_def_map() {
 }
 
 #[test]
-fn typing_inside_a_function_should_not_invalidate_expansions() {
+fn typing_inside_a_function_should_not_invalidate_item_expansions() {
     let (mut db, pos) = TestDB::with_position(
         r#"
 //- /lib.rs
@@ -161,7 +161,7 @@ m!(Z);
         let n_recalculated_item_trees = events.iter().filter(|it| it.contains("item_tree")).count();
         assert_eq!(n_recalculated_item_trees, 1);
         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, 0);
     }
 }

crates/hir-def/src/src.rs

@@ -20,7 +20,7 @@ impl<N: ItemTreeNode> HasSource for AssocItemLoc<N> {
     fn source(&self, db: &dyn DefDatabase) -> InFile<N::Source> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -33,7 +33,7 @@ impl<N: ItemTreeNode> HasSource for ItemLoc<N> {
     fn source(&self, db: &dyn DefDatabase) -> InFile<N::Source> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -46,7 +46,7 @@ impl HasSource for Macro2Loc {
     fn source(&self, db: &dyn DefDatabase) -> InFile<Self::Value> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -59,7 +59,7 @@ impl HasSource for MacroRulesLoc {
     fn source(&self, db: &dyn DefDatabase) -> InFile<Self::Value> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))
@@ -72,7 +72,7 @@ impl HasSource for ProcMacroLoc {
     fn source(&self, db: &dyn DefDatabase) -> InFile<Self::Value> {
         let tree = self.id.item_tree(db);
         let ast_id_map = db.ast_id_map(self.id.file_id());
-        let root = db.parse_or_expand(self.id.file_id()).unwrap();
+        let root = db.parse_or_expand(self.id.file_id());
         let node = &tree[self.id.value];
 
         InFile::new(self.id.file_id(), ast_id_map.get(node.ast_id()).to_node(&root))