4 files changed, 89 insertions, 32 deletions

diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs
index 33eb5e78c..92bcc1705 100644
--- a/crates/hir_def/src/body.rs
+++ b/crates/hir_def/src/body.rs
@@ -120,18 +120,24 @@ impl Expander {
             self.resolve_path_as_macro(db, &path)
         };
 
-        let call_id = match macro_call.as_call_id(db, self.crate_def_map.krate, resolver) {
+        let mut err = None;
+        let call_id =
+            macro_call.as_call_id_with_errors(db, self.crate_def_map.krate, resolver, &mut |e| {
+                err.get_or_insert(e);
+            });
+        let call_id = match call_id {
             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(),
-                ));
+                if err.is_none() {
+                    eprintln!("no error despite `as_call_id_with_errors` returning `None`");
+                }
+                return ExpandResult { value: None, err };
             }
         };
 
-        let err = db.macro_expand_error(call_id);
+        if err.is_none() {
+            err = db.macro_expand_error(call_id);
+        }
 
         let file_id = call_id.as_file();
 
diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs
index baf1179f1..6dba9817d 100644
--- a/crates/hir_def/src/body/tests.rs
+++ b/crates/hir_def/src/body/tests.rs
@@ -78,21 +78,41 @@ fn f() {
 fn macro_diag_builtin() {
     check_diagnostics(
         r#"
+#[rustc_builtin_macro]
+macro_rules! env {}
+
+#[rustc_builtin_macro]
+macro_rules! include {}
+
+#[rustc_builtin_macro]
+macro_rules! compile_error {}
+
+#[rustc_builtin_macro]
+macro_rules! format_args {
+    () => {}
+}
+
 fn f() {
     // Test a handful of built-in (eager) macros:
 
     include!(invalid);
-  //^^^^^^^^^^^^^^^^^ failed to parse or resolve macro invocation
+  //^^^^^^^^^^^^^^^^^ could not convert tokens
     include!("does not exist");
-  //^^^^^^^^^^^^^^^^^^^^^^^^^^ failed to parse or resolve macro invocation
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^ could not convert tokens
 
     env!(invalid);
-  //^^^^^^^^^^^^^ failed to parse or resolve macro invocation
+  //^^^^^^^^^^^^^ could not convert tokens
+
+    env!("OUT_DIR");
+  //^^^^^^^^^^^^^^^ `OUT_DIR` not set, enable "load out dirs from check" to fix
+
+    compile_error!("compile_error works");
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ compile_error works
 
     // Lazy:
 
     format_args!();
-  //^^^^^^^^^^^^^^ failed to parse or resolve macro invocation
+  //^^^^^^^^^^^^^^ no rule matches input tokens
 }
         "#,
     );
diff --git a/crates/hir_def/src/docs.rs b/crates/hir_def/src/docs.rs
index e9a02b11b..3e59a8f47 100644
--- a/crates/hir_def/src/docs.rs
+++ b/crates/hir_def/src/docs.rs
@@ -6,7 +6,8 @@
 use std::sync::Arc;
 
 use either::Either;
-use syntax::ast;
+use itertools::Itertools;
+use syntax::{ast, SmolStr};
 
 use crate::{
     db::DefDatabase,
@@ -93,7 +94,7 @@ fn merge_doc_comments_and_attrs(
 ) -> Option<String> {
     match (doc_comment_text, doc_attr_text) {
         (Some(mut comment_text), Some(attr_text)) => {
-            comment_text.push_str("\n\n");
+            comment_text.push_str("\n");
             comment_text.push_str(&attr_text);
             Some(comment_text)
         }
@@ -105,17 +106,16 @@ fn merge_doc_comments_and_attrs(
 
 fn expand_doc_attrs(owner: &dyn ast::AttrsOwner) -> Option<String> {
     let mut docs = String::new();
-    for attr in owner.attrs() {
-        if let Some(("doc", value)) =
-            attr.as_simple_key_value().as_ref().map(|(k, v)| (k.as_str(), v.as_str()))
-        {
-            docs.push_str(value);
-            docs.push_str("\n\n");
-        }
-    }
+    owner
+        .attrs()
+        .filter_map(|attr| attr.as_simple_key_value().filter(|(key, _)| key == "doc"))
+        .map(|(_, value)| value)
+        .intersperse(SmolStr::new_inline("\n"))
+        // No FromIterator<SmolStr> for String
+        .for_each(|s| docs.push_str(s.as_str()));
     if docs.is_empty() {
         None
     } else {
-        Some(docs.trim_end_matches("\n\n").to_owned())
+        Some(docs)
     }
 }
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index 1b22d1eec..b41c5acb2 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -465,21 +465,37 @@ pub trait AsMacroCall {
         db: &dyn db::DefDatabase,
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
+    ) -> Option<MacroCallId> {
+        self.as_call_id_with_errors(db, krate, resolver, &mut |_| ())
+    }
+
+    fn as_call_id_with_errors(
+        &self,
+        db: &dyn db::DefDatabase,
+        krate: CrateId,
+        resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
+        error_sink: &mut dyn FnMut(mbe::ExpandError),
     ) -> Option<MacroCallId>;
 }
 
 impl AsMacroCall for InFile<&ast::MacroCall> {
-    fn as_call_id(
+    fn as_call_id_with_errors(
         &self,
         db: &dyn db::DefDatabase,
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
+        error_sink: &mut dyn FnMut(mbe::ExpandError),
     ) -> Option<MacroCallId> {
         let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
         let h = Hygiene::new(db.upcast(), self.file_id);
-        let path = path::ModPath::from_src(self.value.path()?, &h)?;
+        let path = self.value.path().and_then(|path| path::ModPath::from_src(path, &h));
+
+        if path.is_none() {
+            error_sink(mbe::ExpandError::Other("malformed macro invocation".into()));
+        }
 
-        AstIdWithPath::new(ast_id.file_id, ast_id.value, path).as_call_id(db, krate, resolver)
+        AstIdWithPath::new(ast_id.file_id, ast_id.value, path?)
+            .as_call_id_with_errors(db, krate, resolver, error_sink)
     }
 }
 
@@ -497,22 +513,32 @@ impl<T: ast::AstNode> AstIdWithPath<T> {
 }
 
 impl AsMacroCall for AstIdWithPath<ast::MacroCall> {
-    fn as_call_id(
+    fn as_call_id_with_errors(
         &self,
         db: &dyn db::DefDatabase,
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
+        error_sink: &mut dyn FnMut(mbe::ExpandError),
     ) -> Option<MacroCallId> {
-        let def: MacroDefId = resolver(self.path.clone())?;
+        let def: MacroDefId = resolver(self.path.clone()).or_else(|| {
+            error_sink(mbe::ExpandError::Other(format!("could not resolve macro `{}`", self.path)));
+            None
+        })?;
 
         if let MacroDefKind::BuiltInEager(_) = def.kind {
             let macro_call = InFile::new(self.ast_id.file_id, self.ast_id.to_node(db.upcast()));
             let hygiene = Hygiene::new(db.upcast(), self.ast_id.file_id);
 
             Some(
-                expand_eager_macro(db.upcast(), krate, macro_call, def, &|path: ast::Path| {
-                    resolver(path::ModPath::from_src(path, &hygiene)?)
-                })?
+                expand_eager_macro(
+                    db.upcast(),
+                    krate,
+                    macro_call,
+                    def,
+                    &|path: ast::Path| resolver(path::ModPath::from_src(path, &hygiene)?),
+                    error_sink,
+                )
+                .ok()?
                 .into(),
             )
         } else {
@@ -522,13 +548,18 @@ impl AsMacroCall for AstIdWithPath<ast::MacroCall> {
 }
 
 impl AsMacroCall for AstIdWithPath<ast::Item> {
-    fn as_call_id(
+    fn as_call_id_with_errors(
         &self,
         db: &dyn db::DefDatabase,
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
+        error_sink: &mut dyn FnMut(mbe::ExpandError),
     ) -> Option<MacroCallId> {
-        let def = resolver(self.path.clone())?;
+        let def: MacroDefId = resolver(self.path.clone()).or_else(|| {
+            error_sink(mbe::ExpandError::Other(format!("could not resolve macro `{}`", self.path)));
+            None
+        })?;
+
         Some(
             def.as_lazy_macro(
                 db.upcast(),