Merge #6683

6683: Emit macro diagnostics when lowering bodies r=matklad a=jonas-schievink

Changes `Expander::enter_expand` to return an `ExpandResult`, and adds any contained errors to the body diagnostic list.

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

crates/hir_def/src/body.rs

@@ -14,8 +14,8 @@ use cfg::CfgOptions;
 use drop_bomb::DropBomb;
 use either::Either;
 use hir_expand::{
-    ast_id_map::AstIdMap, diagnostics::DiagnosticSink, hygiene::Hygiene, AstId, HirFileId, InFile,
-    MacroDefId,
+    ast_id_map::AstIdMap, diagnostics::DiagnosticSink, hygiene::Hygiene, AstId, ExpandResult,
+    HirFileId, InFile, MacroDefId,
 };
 use rustc_hash::FxHashMap;
 use syntax::{ast, AstNode, AstPtr};
@@ -102,11 +102,11 @@ impl Expander {
         db: &dyn DefDatabase,
         local_scope: Option<&ItemScope>,
         macro_call: ast::MacroCall,
-    ) -> Option<(Mark, T)> {
+    ) -> ExpandResult<Option<(Mark, T)>> {
         self.recursion_limit += 1;
         if self.recursion_limit > EXPANSION_RECURSION_LIMIT {
             mark::hit!(your_stack_belongs_to_me);
-            return None;
+            return ExpandResult::str_err("reached recursion limit during macro expansion".into());
         }
 
         let macro_call = InFile::new(self.current_file_id, &macro_call);
@@ -120,11 +120,44 @@ impl Expander {
             self.resolve_path_as_macro(db, &path)
         };
 
-        if let Some(call_id) = macro_call.as_call_id(db, self.crate_def_map.krate, resolver) {
+        let call_id = match macro_call.as_call_id(db, self.crate_def_map.krate, resolver) {
+            Some(it) => it,
+            None => {
+                // FIXME: this can mean other things too, but `as_call_id` doesn't provide enough
+                // info.
+                return ExpandResult::only_err(mbe::ExpandError::Other(
+                    "failed to parse or resolve macro invocation".into(),
+                ));
+            }
+        };
+
+        let err = db.macro_expand_error(call_id);
+
         let file_id = call_id.as_file();
-            if let Some(node) = db.parse_or_expand(file_id) {
-                if let Some(expr) = T::cast(node) {
-                    log::debug!("macro expansion {:#?}", expr.syntax());
+
+        let raw_node = match db.parse_or_expand(file_id) {
+            Some(it) => it,
+            None => {
+                // Only `None` if the macro expansion produced no usable AST.
+                if err.is_none() {
+                    log::warn!("no error despite `parse_or_expand` failing");
+                }
+
+                return ExpandResult::only_err(err.unwrap_or_else(|| {
+                    mbe::ExpandError::Other("failed to parse macro invocation".into())
+                }));
+            }
+        };
+
+        let node = match T::cast(raw_node) {
+            Some(it) => it,
+            None => {
+                // This can happen without being an error, so only forward previous errors.
+                return ExpandResult { value: None, err };
+            }
+        };
+
+        log::debug!("macro expansion {:#?}", node.syntax());
 
         let mark = Mark {
             file_id: self.current_file_id,
@@ -134,14 +167,8 @@ impl Expander {
         self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id);
         self.current_file_id = file_id;
         self.ast_id_map = db.ast_id_map(file_id);
-                    return Some((mark, expr));
-                }
-            }
-        }
 
-        // FIXME: Instead of just dropping the error from expansion
-        // report it
-        None
+        ExpandResult { value: Some((mark, node)), err }
     }
 
     pub(crate) fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {

crates/hir_def/src/body/diagnostics.rs

@@ -2,11 +2,13 @@
 
 use hir_expand::diagnostics::DiagnosticSink;
 
-use crate::diagnostics::InactiveCode;
+use crate::diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro};
 
 #[derive(Debug, Eq, PartialEq)]
 pub(crate) enum BodyDiagnostic {
     InactiveCode(InactiveCode),
+    MacroError(MacroError),
+    UnresolvedProcMacro(UnresolvedProcMacro),
 }
 
 impl BodyDiagnostic {
@@ -15,6 +17,12 @@ impl BodyDiagnostic {
             BodyDiagnostic::InactiveCode(diag) => {
                 sink.push(diag.clone());
             }
+            BodyDiagnostic::MacroError(diag) => {
+                sink.push(diag.clone());
+            }
+            BodyDiagnostic::UnresolvedProcMacro(diag) => {
+                sink.push(diag.clone());
+            }
         }
     }
 }

crates/hir_def/src/body/lower.rs

@@ -8,7 +8,7 @@ use either::Either;
 use hir_expand::{
     hygiene::Hygiene,
     name::{name, AsName, Name},
-    HirFileId, MacroDefId, MacroDefKind,
+    ExpandError, HirFileId, MacroDefId, MacroDefKind,
 };
 use rustc_hash::FxHashMap;
 use syntax::{
@@ -25,7 +25,7 @@ use crate::{
     body::{Body, BodySourceMap, Expander, PatPtr, SyntheticSyntax},
     builtin_type::{BuiltinFloat, BuiltinInt},
     db::DefDatabase,
-    diagnostics::InactiveCode,
+    diagnostics::{InactiveCode, MacroError, UnresolvedProcMacro},
     expr::{
         dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal,
         LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
@@ -561,7 +561,32 @@ impl ExprCollector<'_> {
                     self.alloc_expr(Expr::Missing, syntax_ptr)
                 } else {
                     let macro_call = self.expander.to_source(AstPtr::new(&e));
-                    match self.expander.enter_expand(self.db, Some(&self.body.item_scope), 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: self.expander.current_file_id,
+                                    node: syntax_ptr.clone().into(),
+                                    precise_location: None,
+                                    macro_name: None,
+                                },
+                            ));
+                        }
+                        Some(err) => {
+                            self.source_map.diagnostics.push(BodyDiagnostic::MacroError(
+                                MacroError {
+                                    file: self.expander.current_file_id,
+                                    node: syntax_ptr.clone().into(),
+                                    message: err.to_string(),
+                                },
+                            ));
+                        }
+                        None => {}
+                    }
+
+                    match res.value {
                         Some((mark, expansion)) => {
                             self.source_map
                                 .expansions

crates/hir_def/src/data.rs

@@ -257,7 +257,7 @@ fn collect_items(
                 let root = db.parse_or_expand(file_id).unwrap();
                 let call = ast_id_map.get(call.ast_id).to_node(&root);
 
-                if let Some((mark, mac)) = expander.enter_expand(db, None, call) {
+                if let Some((mark, mac)) = expander.enter_expand(db, None, call).value {
                     let src: InFile<ast::MacroItems> = expander.to_source(mac);
                     let item_tree = db.item_tree(src.file_id);
                     let iter =