4 files changed, 158 insertions, 52 deletions

diff --git a/crates/hir_expand/src/builtin_macro.rs b/crates/hir_expand/src/builtin_macro.rs
index 7f4db106d..44a5556b6 100644
--- a/crates/hir_expand/src/builtin_macro.rs
+++ b/crates/hir_expand/src/builtin_macro.rs
@@ -86,7 +86,6 @@ pub fn find_builtin_macro(
 register_builtin! {
     LAZY:
     (column, Column) => column_expand,
-    (compile_error, CompileError) => compile_error_expand,
     (file, File) => file_expand,
     (line, Line) => line_expand,
     (assert, Assert) => assert_expand,
@@ -97,6 +96,7 @@ register_builtin! {
     (format_args_nl, FormatArgsNl) => format_args_expand,
 
     EAGER:
+    (compile_error, CompileError) => compile_error_expand,
     (concat, Concat) => concat_expand,
     (include, Include) => include_expand,
     (include_bytes, IncludeBytes) => include_bytes_expand,
@@ -213,25 +213,6 @@ fn file_expand(
     ExpandResult::ok(expanded)
 }
 
-fn compile_error_expand(
-    _db: &dyn AstDatabase,
-    _id: LazyMacroId,
-    tt: &tt::Subtree,
-) -> ExpandResult<tt::Subtree> {
-    if tt.count() == 1 {
-        if let tt::TokenTree::Leaf(tt::Leaf::Literal(it)) = &tt.token_trees[0] {
-            let s = it.text.as_str();
-            if s.contains('"') {
-                return ExpandResult::ok(quote! { loop { #it }});
-            }
-        };
-    }
-
-    ExpandResult::only_err(mbe::ExpandError::BindingError(
-        "`compile_error!` argument be a string".into(),
-    ))
-}
-
 fn format_args_expand(
     _db: &dyn AstDatabase,
     _id: LazyMacroId,
@@ -280,6 +261,27 @@ fn unquote_str(lit: &tt::Literal) -> Option<String> {
     token.value().map(|it| it.into_owned())
 }
 
+fn compile_error_expand(
+    _db: &dyn AstDatabase,
+    _id: EagerMacroId,
+    tt: &tt::Subtree,
+) -> ExpandResult<Option<(tt::Subtree, FragmentKind)>> {
+    let err = match &*tt.token_trees {
+        [tt::TokenTree::Leaf(tt::Leaf::Literal(it))] => {
+            let text = it.text.as_str();
+            if text.starts_with('"') && text.ends_with('"') {
+                // FIXME: does not handle raw strings
+                mbe::ExpandError::Other(text[1..text.len() - 1].to_string())
+            } else {
+                mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into())
+            }
+        }
+        _ => mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into()),
+    };
+
+    ExpandResult { value: Some((quote! {}, FragmentKind::Items)), err: Some(err) }
+}
+
 fn concat_expand(
     _db: &dyn AstDatabase,
     _arg_id: EagerMacroId,
@@ -417,17 +419,25 @@ fn env_expand(
         Err(e) => return ExpandResult::only_err(e),
     };
 
-    // FIXME:
-    // If the environment variable is not defined int rustc, then a compilation error will be emitted.
-    // We might do the same if we fully support all other stuffs.
-    // But for now on, we should return some dummy string for better type infer purpose.
-    // However, we cannot use an empty string here, because for
-    // `include!(concat!(env!("OUT_DIR"), "/foo.rs"))` will become
-    // `include!("foo.rs"), which might go to infinite loop
-    let s = get_env_inner(db, arg_id, &key).unwrap_or_else(|| "__RA_UNIMPLEMENTED__".to_string());
+    let mut err = None;
+    let s = get_env_inner(db, arg_id, &key).unwrap_or_else(|| {
+        // The only variable rust-analyzer ever sets is `OUT_DIR`, so only diagnose that to avoid
+        // unnecessary diagnostics for eg. `CARGO_PKG_NAME`.
+        if key == "OUT_DIR" {
+            err = Some(mbe::ExpandError::Other(
+                r#"`OUT_DIR` not set, enable "load out dirs from check" to fix"#.into(),
+            ));
+        }
+
+        // If the variable is unset, still return a dummy string to help type inference along.
+        // We cannot use an empty string here, because for
+        // `include!(concat!(env!("OUT_DIR"), "/foo.rs"))` will become
+        // `include!("foo.rs"), which might go to infinite loop
+        "__RA_UNIMPLEMENTED__".to_string()
+    });
     let expanded = quote! { #s };
 
-    ExpandResult::ok(Some((expanded, FragmentKind::Expr)))
+    ExpandResult { value: Some((expanded, FragmentKind::Expr)), err }
 }
 
 fn option_env_expand(
@@ -638,7 +648,8 @@ mod tests {
             "#,
         );
 
-        assert_eq!(expanded, r#"loop{"error!"}"#);
+        // This expands to nothing (since it's in item position), but emits an error.
+        assert_eq!(expanded, "");
     }
 
     #[test]
diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
index 4fd0ba290..842a177db 100644
--- a/crates/hir_expand/src/db.rs
+++ b/crates/hir_expand/src/db.rs
@@ -207,6 +207,7 @@ fn macro_expand_with_arg(
             } else {
                 return ExpandResult {
                     value: Some(db.lookup_intern_eager_expansion(id).subtree),
+                    // FIXME: There could be errors here!
                     err: None,
                 };
             }
diff --git a/crates/hir_expand/src/eager.rs b/crates/hir_expand/src/eager.rs
index ab6b4477c..0229a836e 100644
--- a/crates/hir_expand/src/eager.rs
+++ b/crates/hir_expand/src/eager.rs
@@ -26,19 +26,89 @@ use crate::{
 };
 
 use base_db::CrateId;
+use mbe::ExpandResult;
 use parser::FragmentKind;
 use std::sync::Arc;
 use syntax::{algo::SyntaxRewriter, SyntaxNode};
 
+pub struct ErrorEmitted {
+    _private: (),
+}
+
+trait ErrorSink {
+    fn emit(&mut self, err: mbe::ExpandError);
+
+    fn option<T>(
+        &mut self,
+        opt: Option<T>,
+        error: impl FnOnce() -> mbe::ExpandError,
+    ) -> Result<T, ErrorEmitted> {
+        match opt {
+            Some(it) => Ok(it),
+            None => {
+                self.emit(error());
+                Err(ErrorEmitted { _private: () })
+            }
+        }
+    }
+
+    fn option_with<T>(
+        &mut self,
+        opt: impl FnOnce() -> Option<T>,
+        error: impl FnOnce() -> mbe::ExpandError,
+    ) -> Result<T, ErrorEmitted> {
+        self.option(opt(), error)
+    }
+
+    fn result<T>(&mut self, res: Result<T, mbe::ExpandError>) -> Result<T, ErrorEmitted> {
+        match res {
+            Ok(it) => Ok(it),
+            Err(e) => {
+                self.emit(e);
+                Err(ErrorEmitted { _private: () })
+            }
+        }
+    }
+
+    fn expand_result_option<T>(&mut self, res: ExpandResult<Option<T>>) -> Result<T, ErrorEmitted> {
+        match (res.value, res.err) {
+            (None, Some(err)) => {
+                self.emit(err);
+                Err(ErrorEmitted { _private: () })
+            }
+            (Some(value), opt_err) => {
+                if let Some(err) = opt_err {
+                    self.emit(err);
+                }
+                Ok(value)
+            }
+            (None, None) => unreachable!("`ExpandResult` without value or error"),
+        }
+    }
+}
+
+impl ErrorSink for &'_ mut dyn FnMut(mbe::ExpandError) {
+    fn emit(&mut self, err: mbe::ExpandError) {
+        self(err);
+    }
+}
+
+fn err(msg: impl Into<String>) -> mbe::ExpandError {
+    mbe::ExpandError::Other(msg.into())
+}
+
 pub fn expand_eager_macro(
     db: &dyn AstDatabase,
     krate: CrateId,
     macro_call: InFile<ast::MacroCall>,
     def: MacroDefId,
     resolver: &dyn Fn(ast::Path) -> Option<MacroDefId>,
-) -> Option<EagerMacroId> {
-    let args = macro_call.value.token_tree()?;
-    let parsed_args = mbe::ast_to_token_tree(&args)?.0;
+    mut diagnostic_sink: &mut dyn FnMut(mbe::ExpandError),
+) -> Result<EagerMacroId, ErrorEmitted> {
+    let parsed_args = diagnostic_sink.option_with(
+        || Some(mbe::ast_to_token_tree(&macro_call.value.token_tree()?)?.0),
+        || err("malformed macro invocation"),
+    )?;
 
     // Note:
     // When `lazy_expand` is called, its *parent* file must be already exists.
@@ -55,17 +125,22 @@ pub fn expand_eager_macro(
     });
     let arg_file_id: MacroCallId = arg_id.into();
 
-    let parsed_args = mbe::token_tree_to_syntax_node(&parsed_args, FragmentKind::Expr).ok()?.0;
+    let parsed_args =
+        diagnostic_sink.result(mbe::token_tree_to_syntax_node(&parsed_args, FragmentKind::Expr))?.0;
     let result = eager_macro_recur(
         db,
         InFile::new(arg_file_id.as_file(), parsed_args.syntax_node()),
         krate,
         resolver,
+        diagnostic_sink,
     )?;
-    let subtree = to_subtree(&result)?;
+    let subtree =
+        diagnostic_sink.option(to_subtree(&result), || err("failed to parse macro result"))?;
 
     if let MacroDefKind::BuiltInEager(eager) = def.kind {
-        let (subtree, fragment) = eager.expand(db, arg_id, &subtree).value?;
+        let res = eager.expand(db, arg_id, &subtree);
+
+        let (subtree, fragment) = diagnostic_sink.expand_result_option(res)?;
         let eager = EagerCallLoc {
             def,
             fragment,
@@ -74,9 +149,9 @@ pub fn expand_eager_macro(
             file_id: macro_call.file_id,
         };
 
-        Some(db.intern_eager_expansion(eager))
+        Ok(db.intern_eager_expansion(eager))
     } else {
-        None
+        panic!("called `expand_eager_macro` on non-eager macro def {:?}", def);
     }
 }
 
@@ -91,13 +166,16 @@ fn lazy_expand(
     def: &MacroDefId,
     macro_call: InFile<ast::MacroCall>,
     krate: CrateId,
-) -> Option<InFile<SyntaxNode>> {
+) -> ExpandResult<Option<InFile<SyntaxNode>>> {
     let ast_id = db.ast_id_map(macro_call.file_id).ast_id(&macro_call.value);
 
     let id: MacroCallId =
         def.as_lazy_macro(db, krate, MacroCallKind::FnLike(macro_call.with_value(ast_id))).into();
 
-    db.parse_or_expand(id.as_file()).map(|node| InFile::new(id.as_file(), node))
+    let err = db.macro_expand_error(id);
+    let value = db.parse_or_expand(id.as_file()).map(|node| InFile::new(id.as_file(), node));
+
+    ExpandResult { value, err }
 }
 
 fn eager_macro_recur(
@@ -105,7 +183,8 @@ fn eager_macro_recur(
     curr: InFile<SyntaxNode>,
     krate: CrateId,
     macro_resolver: &dyn Fn(ast::Path) -> Option<MacroDefId>,
-) -> Option<SyntaxNode> {
+    mut diagnostic_sink: &mut dyn FnMut(mbe::ExpandError),
+) -> Result<SyntaxNode, ErrorEmitted> {
     let original = curr.value.clone();
 
     let children = curr.value.descendants().filter_map(ast::MacroCall::cast);
@@ -113,7 +192,8 @@ fn eager_macro_recur(
 
     // Collect replacement
     for child in children {
-        let def: MacroDefId = macro_resolver(child.path()?)?;
+        let def = diagnostic_sink
+            .option_with(|| macro_resolver(child.path()?), || err("failed to resolve macro"))?;
         let insert = match def.kind {
             MacroDefKind::BuiltInEager(_) => {
                 let id: MacroCallId = expand_eager_macro(
@@ -122,17 +202,21 @@ fn eager_macro_recur(
                     curr.with_value(child.clone()),
                     def,
                     macro_resolver,
+                    diagnostic_sink,
                 )?
                 .into();
-                db.parse_or_expand(id.as_file())?
+                db.parse_or_expand(id.as_file())
+                    .expect("successful macro expansion should be parseable")
             }
             MacroDefKind::Declarative
             | MacroDefKind::BuiltIn(_)
             | MacroDefKind::BuiltInDerive(_)
             | MacroDefKind::ProcMacro(_) => {
-                let expanded = lazy_expand(db, &def, curr.with_value(child.clone()), krate)?;
+                let res = lazy_expand(db, &def, curr.with_value(child.clone()), krate);
+                let val = diagnostic_sink.expand_result_option(res)?;
+
                 // replace macro inside
-                eager_macro_recur(db, expanded, krate, macro_resolver)?
+                eager_macro_recur(db, val, krate, macro_resolver, diagnostic_sink)?
             }
         };
 
@@ -140,5 +224,5 @@ fn eager_macro_recur(
     }
 
     let res = rewriter.rewrite(&original);
-    Some(res)
+    Ok(res)
 }
diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs
index 6dad2507b..2633fd8f7 100644
--- a/crates/hir_expand/src/lib.rs
+++ b/crates/hir_expand/src/lib.rs
@@ -143,7 +143,11 @@ impl HirFileId {
                 let loc: MacroCallLoc = db.lookup_intern_macro(lazy_id);
 
                 let arg_tt = loc.kind.arg(db)?;
-                let def_tt = loc.def.ast_id?.to_node(db).token_tree()?;
+
+                let def = loc.def.ast_id.and_then(|id| {
+                    let def_tt = id.to_node(db).token_tree()?;
+                    Some(InFile::new(id.file_id, def_tt))
+                });
 
                 let macro_def = db.macro_def(loc.def)?;
                 let (parse, exp_map) = db.parse_macro_expansion(macro_file).value?;
@@ -152,7 +156,7 @@ impl HirFileId {
                 Some(ExpansionInfo {
                     expanded: InFile::new(self, parse.syntax_node()),
                     arg: InFile::new(loc.kind.file_id(), arg_tt),
-                    def: InFile::new(loc.def.ast_id?.file_id, def_tt),
+                    def,
                     macro_arg,
                     macro_def,
                     exp_map,
@@ -311,7 +315,8 @@ pub struct EagerCallLoc {
 pub struct ExpansionInfo {
     expanded: InFile<SyntaxNode>,
     arg: InFile<SyntaxNode>,
-    def: InFile<ast::TokenTree>,
+    /// The `macro_rules!` arguments.
+    def: Option<InFile<ast::TokenTree>>,
 
     macro_def: Arc<(db::TokenExpander, mbe::TokenMap)>,
     macro_arg: Arc<(tt::Subtree, mbe::TokenMap)>,
@@ -348,9 +353,14 @@ impl ExpansionInfo {
         let (token_id, origin) = self.macro_def.0.map_id_up(token_id);
         let (token_map, tt) = match origin {
             mbe::Origin::Call => (&self.macro_arg.1, self.arg.clone()),
-            mbe::Origin::Def => {
-                (&self.macro_def.1, self.def.as_ref().map(|tt| tt.syntax().clone()))
-            }
+            mbe::Origin::Def => (
+                &self.macro_def.1,
+                self.def
+                    .as_ref()
+                    .expect("`Origin::Def` used with non-`macro_rules!` macro")
+                    .as_ref()
+                    .map(|tt| tt.syntax().clone()),
+            ),
         };
 
         let range = token_map.range_by_token(token_id)?.by_kind(token.value.kind())?;