Auto merge of #17707 - Veykril:proc-macro-err-cleanup, r=Veykril

feat: Use spans for builtin and declarative macro expansion errors

This should generally improve some error reporting for macro expansion errors. Especially for `compile_error!` within proc-macros

Author: bors

src/tools/rust-analyzer/crates/base-db/src/input.rs

@@ -16,9 +16,7 @@ use span::{Edition, EditionedFileId};
 use triomphe::Arc;
 use vfs::{file_set::FileSet, AbsPathBuf, AnchoredPath, FileId, VfsPath};
 
-// Map from crate id to the name of the crate and path of the proc-macro. If the value is `None`,
-// then the crate for the proc-macro hasn't been build yet as the build data is missing.
-pub type ProcMacroPaths = FxHashMap<CrateId, Result<(Option<String>, AbsPathBuf), String>>;
+pub type ProcMacroPaths = FxHashMap<CrateId, Result<(String, AbsPathBuf), String>>;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct SourceRootId(pub u32);

src/tools/rust-analyzer/crates/hir-def/src/body.rs

@@ -10,7 +10,7 @@ use std::ops::{Deref, Index};
 
 use base_db::CrateId;
 use cfg::{CfgExpr, CfgOptions};
-use hir_expand::{name::Name, InFile};
+use hir_expand::{name::Name, ExpandError, InFile};
 use la_arena::{Arena, ArenaMap, Idx, RawIdx};
 use rustc_hash::FxHashMap;
 use smallvec::SmallVec;
@@ -115,8 +115,7 @@ pub struct SyntheticSyntax;
 #[derive(Debug, Eq, PartialEq)]
 pub enum BodyDiagnostic {
     InactiveCode { node: InFile<SyntaxNodePtr>, cfg: CfgExpr, opts: CfgOptions },
-    MacroError { node: InFile<AstPtr<ast::MacroCall>>, message: String },
-    UnresolvedProcMacro { node: InFile<AstPtr<ast::MacroCall>>, krate: CrateId },
+    MacroError { node: InFile<AstPtr<ast::MacroCall>>, err: ExpandError },
     UnresolvedMacroCall { node: InFile<AstPtr<ast::MacroCall>>, path: ModPath },
     UnreachableLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name },
     UndeclaredLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name },

src/tools/rust-analyzer/crates/hir-def/src/body/lower.rs

@@ -7,7 +7,7 @@ use base_db::CrateId;
 use either::Either;
 use hir_expand::{
     name::{AsName, Name},
-    ExpandError, InFile,
+    InFile,
 };
 use intern::{sym, Interned, Symbol};
 use rustc_hash::FxHashMap;
@@ -992,20 +992,11 @@ impl ExprCollector<'_> {
             }
         };
         if record_diagnostics {
-            match &res.err {
-                Some(ExpandError::UnresolvedProcMacro(krate)) => {
-                    self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro {
-                        node: InFile::new(outer_file, syntax_ptr),
-                        krate: *krate,
-                    });
-                }
-                Some(err) => {
-                    self.source_map.diagnostics.push(BodyDiagnostic::MacroError {
-                        node: InFile::new(outer_file, syntax_ptr),
-                        message: err.to_string(),
-                    });
-                }
-                None => {}
+            if let Some(err) = res.err {
+                self.source_map.diagnostics.push(BodyDiagnostic::MacroError {
+                    node: InFile::new(outer_file, syntax_ptr),
+                    err,
+                });
             }
         }
 

src/tools/rust-analyzer/crates/hir-def/src/data.rs

@@ -657,22 +657,18 @@ impl<'a> AssocItemCollector<'a> {
                             // crate failed), skip expansion like we would if it was
                             // disabled. This is analogous to the handling in
                             // `DefCollector::collect_macros`.
-                            if exp.is_dummy() {
-                                self.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
+                            if let Some(err) = exp.as_expand_error(self.module_id.krate) {
+                                self.diagnostics.push(DefDiagnostic::macro_error(
                                     self.module_id.local_id,
-                                    loc.kind,
-                                    loc.def.krate,
+                                    ast_id,
+                                    (*attr.path).clone(),
+                                    err,
                                 ));
-
-                                continue 'attrs;
-                            }
-                            if exp.is_disabled() {
                                 continue 'attrs;
                             }
                         }
 
                         self.macro_calls.push((ast_id, call_id));
-
                         let res =
                             self.expander.enter_expand_id::<ast::MacroItems>(self.db, call_id);
                         self.collect_macro_items(res);

src/tools/rust-analyzer/crates/hir-def/src/expander.rs

@@ -6,8 +6,8 @@ use base_db::CrateId;
 use cfg::CfgOptions;
 use drop_bomb::DropBomb;
 use hir_expand::{
-    attrs::RawAttrs, mod_path::ModPath, span_map::SpanMap, ExpandError, ExpandResult, HirFileId,
-    InFile, MacroCallId,
+    attrs::RawAttrs, mod_path::ModPath, span_map::SpanMap, ExpandError, ExpandErrorKind,
+    ExpandResult, HirFileId, InFile, Lookup, MacroCallId,
 };
 use limit::Limit;
 use span::SyntaxContextId;
@@ -160,26 +160,30 @@ impl Expander {
             // so don't return overflow error here to avoid diagnostics duplication.
             cov_mark::hit!(overflow_but_not_me);
             return ExpandResult::ok(None);
-        } else if self.recursion_limit.check(self.recursion_depth as usize + 1).is_err() {
-            self.recursion_depth = u32::MAX;
-            cov_mark::hit!(your_stack_belongs_to_me);
-            return ExpandResult::only_err(ExpandError::RecursionOverflow);
         }
 
         let ExpandResult { value, err } = op(self);
         let Some(call_id) = value else {
             return ExpandResult { value: None, err };
         };
+        if self.recursion_limit.check(self.recursion_depth as usize + 1).is_err() {
+            self.recursion_depth = u32::MAX;
+            cov_mark::hit!(your_stack_belongs_to_me);
+            return ExpandResult::only_err(ExpandError::new(
+                db.macro_arg_considering_derives(call_id, &call_id.lookup(db.upcast()).kind).2,
+                ExpandErrorKind::RecursionOverflow,
+            ));
+        }
 
         let macro_file = call_id.as_macro_file();
         let res = db.parse_macro_expansion(macro_file);
 
         let err = err.or(res.err);
         ExpandResult {
-            value: match err {
+            value: match &err {
                 // If proc-macro is disabled or unresolved, we want to expand to a missing expression
                 // instead of an empty tree which might end up in an empty block.
-                Some(ExpandError::UnresolvedProcMacro(_)) => None,
+                Some(e) if matches!(e.kind(), ExpandErrorKind::MissingProcMacroExpander(_)) => None,
                 _ => (|| {
                     let parse = res.value.0.cast::<T>()?;
 

src/tools/rust-analyzer/crates/hir-def/src/lib.rs

@@ -75,9 +75,7 @@ use base_db::{
     CrateId,
 };
 use hir_expand::{
-    builtin_attr_macro::BuiltinAttrExpander,
-    builtin_derive_macro::BuiltinDeriveExpander,
-    builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander},
+    builtin::{BuiltinAttrExpander, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerExpander},
     db::ExpandDatabase,
     eager::expand_eager_macro_input,
     impl_intern_lookup,
@@ -1436,7 +1434,10 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
         });
 
         let Some((call_site, path)) = path else {
-            return Ok(ExpandResult::only_err(ExpandError::other("malformed macro invocation")));
+            return Ok(ExpandResult::only_err(ExpandError::other(
+                span_map.span_for_range(self.value.syntax().text_range()),
+                "malformed macro invocation",
+            )));
         };
 
         macro_call_as_call_id_with_eager(

src/tools/rust-analyzer/crates/hir-def/src/macro_expansion_tests/mbe/regression.rs

@@ -1084,7 +1084,7 @@ fn main() {
 macro_rules! concat_bytes {}
 
 fn main() {
-    let x = /* error: unexpected token in input */b"";
+    let x = /* error: unexpected token */b"";
 }
 
 "#]],

src/tools/rust-analyzer/crates/hir-def/src/macro_expansion_tests/mod.rs

@@ -122,7 +122,7 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream
 
         let mut expn_text = String::new();
         if let Some(err) = exp.err {
-            format_to!(expn_text, "/* error: {} */", err);
+            format_to!(expn_text, "/* error: {} */", err.render_to_string(&db).0);
         }
         let (parse, token_map) = exp.value;
         if expect_errors {

src/tools/rust-analyzer/crates/hir-def/src/nameres.rs

@@ -145,8 +145,6 @@ struct DefMapCrateData {
     /// Side table for resolving derive helpers.
     exported_derives: FxHashMap<MacroDefId, Box<[Name]>>,
     fn_proc_macro_mapping: FxHashMap<FunctionId, ProcMacroId>,
-    /// The error that occurred when failing to load the proc-macro dll.
-    proc_macro_loading_error: Option<Box<str>>,
 
     /// Custom attributes registered with `#![register_attr]`.
     registered_attrs: Vec<Symbol>,
@@ -169,7 +167,6 @@ impl DefMapCrateData {
             extern_prelude: FxIndexMap::default(),
             exported_derives: FxHashMap::default(),
             fn_proc_macro_mapping: FxHashMap::default(),
-            proc_macro_loading_error: None,
             registered_attrs: Vec::new(),
             registered_tools: PREDEFINED_TOOLS.iter().map(|it| Symbol::intern(it)).collect(),
             unstable_features: FxHashSet::default(),
@@ -189,7 +186,6 @@ impl DefMapCrateData {
             registered_attrs,
             registered_tools,
             unstable_features,
-            proc_macro_loading_error: _,
             rustc_coherence_is_core: _,
             no_core: _,
             no_std: _,
@@ -474,10 +470,6 @@ impl DefMap {
         self.data.fn_proc_macro_mapping.get(&id).copied()
     }
 
-    pub fn proc_macro_loading_error(&self) -> Option<&str> {
-        self.data.proc_macro_loading_error.as_deref()
-    }
-
     pub fn krate(&self) -> CrateId {
         self.krate
     }

src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs

@@ -10,9 +10,7 @@ use cfg::{CfgExpr, CfgOptions};
 use either::Either;
 use hir_expand::{
     attrs::{Attr, AttrId},
-    builtin_attr_macro::find_builtin_attr,
-    builtin_derive_macro::find_builtin_derive,
-    builtin_fn_macro::find_builtin_macro,
+    builtin::{find_builtin_attr, find_builtin_derive, find_builtin_macro},
     name::{AsName, Name},
     proc_macro::CustomProcMacroExpander,
     ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
@@ -76,34 +74,11 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
     }
 
     let proc_macros = if krate.is_proc_macro {
-        match db.proc_macros().get(&def_map.krate) {
-            Some(Ok(proc_macros)) => Ok({
-                let ctx = db.syntax_context(tree_id.file_id());
-                proc_macros
-                    .iter()
-                    .enumerate()
-                    .map(|(idx, it)| {
-                        let name = Name::new_symbol(it.name.clone(), ctx);
-                        (
-                            name,
-                            if !db.expand_proc_attr_macros() {
-                                CustomProcMacroExpander::dummy()
-                            } else if it.disabled {
-                                CustomProcMacroExpander::disabled()
-                            } else {
-                                CustomProcMacroExpander::new(
-                                    hir_expand::proc_macro::ProcMacroId::new(idx as u32),
-                                )
-                            },
-                        )
-                    })
-                    .collect()
-            }),
-            Some(Err(e)) => Err(e.clone().into_boxed_str()),
-            None => Err("No proc-macros present for crate".to_owned().into_boxed_str()),
-        }
+        db.proc_macros()
+            .for_crate(def_map.krate, db.syntax_context(tree_id.file_id()))
+            .unwrap_or_default()
     } else {
-        Ok(vec![])
+        Default::default()
     };
 
     let mut collector = DefCollector {
@@ -252,10 +227,10 @@ struct DefCollector<'a> {
     mod_dirs: FxHashMap<LocalModuleId, ModDir>,
     cfg_options: &'a CfgOptions,
     /// List of procedural macros defined by this crate. This is read from the dynamic library
-    /// built by the build system, and is the list of proc. macros we can actually expand. It is
-    /// empty when proc. macro support is disabled (in which case we still do name resolution for
-    /// them).
-    proc_macros: Result<Vec<(Name, CustomProcMacroExpander)>, Box<str>>,
+    /// built by the build system, and is the list of proc-macros we can actually expand. It is
+    /// empty when proc-macro support is disabled (in which case we still do name resolution for
+    /// them). The bool signals whether the proc-macro has been explicitly disabled for name-resolution.
+    proc_macros: Box<[(Name, CustomProcMacroExpander, bool)]>,
     is_proc_macro: bool,
     from_glob_import: PerNsGlobImports,
     /// If we fail to resolve an attribute on a `ModItem`, we fall back to ignoring the attribute.
@@ -278,10 +253,6 @@ impl DefCollector<'_> {
         let attrs = item_tree.top_level_attrs(self.db, self.def_map.krate);
         let crate_data = Arc::get_mut(&mut self.def_map.data).unwrap();
 
-        if let Err(e) = &self.proc_macros {
-            crate_data.proc_macro_loading_error = Some(e.clone());
-        }
-
         let mut process = true;
 
         // Process other crate-level attributes.
@@ -608,11 +579,17 @@ impl DefCollector<'_> {
         fn_id: FunctionId,
     ) {
         let kind = def.kind.to_basedb_kind();
-        let (expander, kind) =
-            match self.proc_macros.as_ref().map(|it| it.iter().find(|(n, _)| n == &def.name)) {
-                Ok(Some(&(_, expander))) => (expander, kind),
-                _ => (CustomProcMacroExpander::dummy(), kind),
-            };
+        let (expander, kind) = match self.proc_macros.iter().find(|(n, _, _)| n == &def.name) {
+            Some(_)
+                if kind == hir_expand::proc_macro::ProcMacroKind::Attr
+                    && !self.db.expand_proc_attr_macros() =>
+            {
+                (CustomProcMacroExpander::disabled_proc_attr(), kind)
+            }
+            Some(&(_, _, true)) => (CustomProcMacroExpander::disabled(), kind),
+            Some(&(_, expander, false)) => (expander, kind),
+            None => (CustomProcMacroExpander::missing_expander(), kind),
+        };
 
         let proc_macro_id = ProcMacroLoc {
             container: self.def_map.crate_root(),
@@ -1415,25 +1392,23 @@ impl DefCollector<'_> {
                         return recollect_without(self);
                     }
 
-                    let call_id = call_id();
                     if let MacroDefKind::ProcMacro(_, exp, _) = def.kind {
                         // If there's no expander for the proc macro (e.g.
                         // because proc macros are disabled, or building the
                         // proc macro crate failed), report this and skip
                         // expansion like we would if it was disabled
-                        if exp.is_dummy() {
-                            self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
+                        if let Some(err) = exp.as_expand_error(def.krate) {
+                            self.def_map.diagnostics.push(DefDiagnostic::macro_error(
                                 directive.module_id,
-                                self.db.lookup_intern_macro_call(call_id).kind,
-                                def.krate,
+                                ast_id,
+                                (**path).clone(),
+                                err,
                             ));
                             return recollect_without(self);
                         }
-                        if exp.is_disabled() {
-                            return recollect_without(self);
-                        }
                     }
 
+                    let call_id = call_id();
                     self.def_map.modules[directive.module_id]
                         .scope
                         .add_attr_macro_invoc(ast_id, call_id);
@@ -1472,7 +1447,6 @@ impl DefCollector<'_> {
         }
         let file_id = macro_call_id.as_file();
 
-        // 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);
 
         let mod_dir = if macro_call_id.as_macro_file().is_include_macro(self.db.upcast()) {
@@ -2510,7 +2484,7 @@ mod tests {
             unresolved_macros: Vec::new(),
             mod_dirs: FxHashMap::default(),
             cfg_options: &CfgOptions::default(),
-            proc_macros: Ok(vec![]),
+            proc_macros: Default::default(),
             from_glob_import: Default::default(),
             skip_attrs: Default::default(),
             is_proc_macro: false,