Internal: Cleanup proc-macro error handling

Author: Veykril

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,17 @@ 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,
+                                    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

@@ -179,7 +179,7 @@ impl Expander {
             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(ExpandError::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,

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(),
@@ -1338,25 +1315,22 @@ 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,
+                                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);
@@ -1395,7 +1369,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()) {
@@ -2433,7 +2406,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,

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

@@ -2,9 +2,8 @@
 
 use std::ops::Not;
 
-use base_db::CrateId;
 use cfg::{CfgExpr, CfgOptions};
-use hir_expand::{attrs::AttrId, MacroCallKind};
+use hir_expand::{attrs::AttrId, ExpandError, MacroCallKind};
 use la_arena::Idx;
 use syntax::ast;
 
@@ -17,48 +16,16 @@ use crate::{
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum DefDiagnosticKind {
-    UnresolvedModule {
-        ast: AstId<ast::Module>,
-        candidates: Box<[String]>,
-    },
-    UnresolvedExternCrate {
-        ast: AstId<ast::ExternCrate>,
-    },
-    UnresolvedImport {
-        id: ItemTreeId<item_tree::Use>,
-        index: Idx<ast::UseTree>,
-    },
-    UnconfiguredCode {
-        tree: TreeId,
-        item: AttrOwner,
-        cfg: CfgExpr,
-        opts: CfgOptions,
-    },
-    /// A proc-macro that is lacking an expander, this might be due to build scripts not yet having
-    /// run or proc-macro expansion being disabled.
-    UnresolvedProcMacro {
-        ast: MacroCallKind,
-        krate: CrateId,
-    },
-    UnresolvedMacroCall {
-        ast: MacroCallKind,
-        path: ModPath,
-    },
-    UnimplementedBuiltinMacro {
-        ast: AstId<ast::Macro>,
-    },
-    InvalidDeriveTarget {
-        ast: AstId<ast::Item>,
-        id: usize,
-    },
-    MalformedDerive {
-        ast: AstId<ast::Adt>,
-        id: usize,
-    },
-    MacroDefError {
-        ast: AstId<ast::Macro>,
-        message: String,
-    },
+    UnresolvedModule { ast: AstId<ast::Module>, candidates: Box<[String]> },
+    UnresolvedExternCrate { ast: AstId<ast::ExternCrate> },
+    UnresolvedImport { id: ItemTreeId<item_tree::Use>, index: Idx<ast::UseTree> },
+    UnconfiguredCode { tree: TreeId, item: AttrOwner, cfg: CfgExpr, opts: CfgOptions },
+    UnresolvedMacroCall { ast: MacroCallKind, path: ModPath },
+    UnimplementedBuiltinMacro { ast: AstId<ast::Macro> },
+    InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize },
+    MalformedDerive { ast: AstId<ast::Adt>, id: usize },
+    MacroDefError { ast: AstId<ast::Macro>, message: String },
+    MacroError { ast: AstId<ast::Item>, err: ExpandError },
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -115,6 +82,10 @@ impl DefDiagnostic {
         Self { in_module: container, kind: DefDiagnosticKind::UnresolvedImport { id, index } }
     }
 
+    pub fn macro_error(container: LocalModuleId, ast: AstId<ast::Item>, err: ExpandError) -> Self {
+        Self { in_module: container, kind: DefDiagnosticKind::MacroError { ast, err } }
+    }
+
     pub fn unconfigured_code(
         container: LocalModuleId,
         tree: TreeId,
@@ -128,14 +99,6 @@ impl DefDiagnostic {
         }
     }
 
-    pub fn unresolved_proc_macro(
-        container: LocalModuleId,
-        ast: MacroCallKind,
-        krate: CrateId,
-    ) -> Self {
-        Self { in_module: container, kind: DefDiagnosticKind::UnresolvedProcMacro { ast, krate } }
-    }
-
     // FIXME: Whats the difference between this and unresolved_proc_macro
     pub(crate) fn unresolved_macro_call(
         container: LocalModuleId,