From e54759083a1f7e5178032d78c803a5d8041b1ec5 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 3 Jul 2025 09:28:53 +0200 Subject: [PATCH] Restructure proc-macro loading erros, differentiate hard error property on kind --- crates/base-db/src/input.rs | 45 ++++++++++++++++++- crates/base-db/src/lib.rs | 5 ++- crates/hir-expand/src/lib.rs | 6 +-- .../src/prettify_macro_expansion_.rs | 2 +- crates/hir-expand/src/proc_macro.rs | 14 +++--- crates/hir/src/diagnostics.rs | 12 ++--- .../src/handlers/expand_rest_pattern.rs | 2 +- crates/ide-assists/src/utils.rs | 20 +++++---- crates/load-cargo/src/lib.rs | 40 ++++++++--------- crates/project-model/src/workspace.rs | 10 ++--- crates/rust-analyzer/src/reload.rs | 12 ++--- 11 files changed, 105 insertions(+), 63 deletions(-) diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 2a87b15248..8c9393bcc9 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -6,6 +6,7 @@ //! actual IO. See `vfs` and `project_model` in the `rust-analyzer` crate for how //! actual IO is done and lowered to input. +use std::error::Error; use std::hash::BuildHasherDefault; use std::{fmt, mem, ops}; @@ -22,7 +23,49 @@ use vfs::{AbsPathBuf, AnchoredPath, FileId, VfsPath, file_set::FileSet}; use crate::{CrateWorkspaceData, EditionedFileId, FxIndexSet, RootQueryDb}; -pub type ProcMacroPaths = FxHashMap>; +pub type ProcMacroPaths = + FxHashMap>; + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ProcMacroLoadingError { + Disabled, + FailedToBuild, + MissingDylibPath, + NotYetBuilt, + NoProcMacros, + ProcMacroSrvError(Box), +} +impl ProcMacroLoadingError { + pub fn is_hard_error(&self) -> bool { + match self { + ProcMacroLoadingError::Disabled | ProcMacroLoadingError::NotYetBuilt => false, + ProcMacroLoadingError::FailedToBuild + | ProcMacroLoadingError::MissingDylibPath + | ProcMacroLoadingError::NoProcMacros + | ProcMacroLoadingError::ProcMacroSrvError(_) => true, + } + } +} + +impl Error for ProcMacroLoadingError {} +impl fmt::Display for ProcMacroLoadingError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ProcMacroLoadingError::Disabled => write!(f, "proc-macro expansion is disabled"), + ProcMacroLoadingError::FailedToBuild => write!(f, "proc-macro failed to build"), + ProcMacroLoadingError::MissingDylibPath => { + write!(f, "proc-macro crate build data is missing a dylib path") + } + ProcMacroLoadingError::NotYetBuilt => write!(f, "proc-macro not yet built"), + ProcMacroLoadingError::NoProcMacros => { + write!(f, "proc macro library has no proc macros") + } + ProcMacroLoadingError::ProcMacroSrvError(msg) => { + write!(f, "proc macro server error: {msg}") + } + } + } +} #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct SourceRootId(pub u32); diff --git a/crates/base-db/src/lib.rs b/crates/base-db/src/lib.rs index 478fae67c8..d8afe996c3 100644 --- a/crates/base-db/src/lib.rs +++ b/crates/base-db/src/lib.rs @@ -14,8 +14,9 @@ pub use crate::{ input::{ BuiltCrateData, BuiltDependency, Crate, CrateBuilder, CrateBuilderId, CrateDataBuilder, CrateDisplayName, CrateGraphBuilder, CrateName, CrateOrigin, CratesIdMap, CratesMap, - DependencyBuilder, Env, ExtraCrateData, LangCrateOrigin, ProcMacroPaths, ReleaseChannel, - SourceRoot, SourceRootId, TargetLayoutLoadResult, UniqueCrateData, + DependencyBuilder, Env, ExtraCrateData, LangCrateOrigin, ProcMacroLoadingError, + ProcMacroPaths, ReleaseChannel, SourceRoot, SourceRootId, TargetLayoutLoadResult, + UniqueCrateData, }, }; use dashmap::{DashMap, mapref::entry::Entry}; diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 6ecac1463f..193d6e81d9 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -199,9 +199,9 @@ impl ExpandErrorKind { }, &ExpandErrorKind::MissingProcMacroExpander(def_crate) => { match db.proc_macros_for_crate(def_crate).as_ref().and_then(|it| it.get_error()) { - Some((e, hard_err)) => RenderedExpandError { - message: e.to_owned(), - error: hard_err, + Some(e) => RenderedExpandError { + message: e.to_string(), + error: e.is_hard_error(), kind: RenderedExpandError::GENERAL_KIND, }, None => RenderedExpandError { diff --git a/crates/hir-expand/src/prettify_macro_expansion_.rs b/crates/hir-expand/src/prettify_macro_expansion_.rs index 6134c3a36b..6431d46d39 100644 --- a/crates/hir-expand/src/prettify_macro_expansion_.rs +++ b/crates/hir-expand/src/prettify_macro_expansion_.rs @@ -46,7 +46,7 @@ pub fn prettify_macro_expansion( } else if let Some(crate_name) = ¯o_def_crate.extra_data(db).display_name { make::tokens::ident(crate_name.crate_name().as_str()) } else { - return dollar_crate.clone(); + dollar_crate.clone() } }); if replacement.text() == "$crate" { diff --git a/crates/hir-expand/src/proc_macro.rs b/crates/hir-expand/src/proc_macro.rs index 1c8ebb6f53..f97d721dfa 100644 --- a/crates/hir-expand/src/proc_macro.rs +++ b/crates/hir-expand/src/proc_macro.rs @@ -4,7 +4,7 @@ use core::fmt; use std::any::Any; use std::{panic::RefUnwindSafe, sync}; -use base_db::{Crate, CrateBuilderId, CratesIdMap, Env}; +use base_db::{Crate, CrateBuilderId, CratesIdMap, Env, ProcMacroLoadingError}; use intern::Symbol; use rustc_hash::FxHashMap; use span::Span; @@ -53,8 +53,8 @@ pub enum ProcMacroExpansionError { System(String), } -pub type ProcMacroLoadResult = Result, (String, bool)>; -type StoredProcMacroLoadResult = Result, (Box, bool)>; +pub type ProcMacroLoadResult = Result, ProcMacroLoadingError>; +type StoredProcMacroLoadResult = Result, ProcMacroLoadingError>; #[derive(Default, Debug)] pub struct ProcMacrosBuilder(FxHashMap>); @@ -77,9 +77,7 @@ impl ProcMacrosBuilder { proc_macros_crate, match proc_macro { Ok(it) => Arc::new(CrateProcMacros(Ok(it.into_boxed_slice()))), - Err((e, hard_err)) => { - Arc::new(CrateProcMacros(Err((e.into_boxed_str(), hard_err)))) - } + Err(e) => Arc::new(CrateProcMacros(Err(e))), }, ); } @@ -139,8 +137,8 @@ impl CrateProcMacros { ) } - pub fn get_error(&self) -> Option<(&str, bool)> { - self.0.as_ref().err().map(|(e, hard_err)| (&**e, *hard_err)) + pub fn get_error(&self) -> Option<&ProcMacroLoadingError> { + self.0.as_ref().err() } /// Fetch the [`CustomProcMacroExpander`]s and their corresponding names for the given crate. diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index aba2e032b3..c1e814ec22 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -36,16 +36,16 @@ pub use hir_ty::{ }; macro_rules! diagnostics { - ($($diag:ident $(<$lt:lifetime>)?,)*) => { + ($AnyDiagnostic:ident <$db:lifetime> -> $($diag:ident $(<$lt:lifetime>)?,)*) => { #[derive(Debug)] - pub enum AnyDiagnostic<'db> {$( + pub enum $AnyDiagnostic<$db> {$( $diag(Box<$diag $(<$lt>)?>), )*} $( - impl<'db> From<$diag $(<$lt>)?> for AnyDiagnostic<'db> { - fn from(d: $diag $(<$lt>)?) -> AnyDiagnostic<'db> { - AnyDiagnostic::$diag(Box::new(d)) + impl<$db> From<$diag $(<$lt>)?> for $AnyDiagnostic<$db> { + fn from(d: $diag $(<$lt>)?) -> $AnyDiagnostic<$db> { + $AnyDiagnostic::$diag(Box::new(d)) } } )* @@ -66,7 +66,7 @@ macro_rules! diagnostics { // }, ... // ] -diagnostics![ +diagnostics![AnyDiagnostic<'db> -> AwaitOutsideOfAsync, BreakOutsideOfLoop, CastToUnsized<'db>, diff --git a/crates/ide-assists/src/handlers/expand_rest_pattern.rs b/crates/ide-assists/src/handlers/expand_rest_pattern.rs index b71de5e00c..c80b78fd97 100644 --- a/crates/ide-assists/src/handlers/expand_rest_pattern.rs +++ b/crates/ide-assists/src/handlers/expand_rest_pattern.rs @@ -175,7 +175,7 @@ pub(crate) fn expand_rest_pattern(acc: &mut Assists, ctx: &AssistContext<'_>) -> // ast::TuplePat(it) => (), // FIXME // ast::SlicePat(it) => (), - _ => return None, + _ => None, } } } diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 1a91053f93..7514866f38 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -912,7 +912,7 @@ fn handle_as_ref_str( ) -> Option<(ReferenceConversionType, bool)> { let str_type = hir::BuiltinType::str().ty(db); - ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[str_type.clone()]) + ty.impls_trait(db, famous_defs.core_convert_AsRef()?, std::slice::from_ref(&str_type)) .then_some((ReferenceConversionType::AsRefStr, could_deref_to_target(ty, &str_type, db))) } @@ -924,10 +924,11 @@ fn handle_as_ref_slice( let type_argument = ty.type_arguments().next()?; let slice_type = hir::Type::new_slice(type_argument); - ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[slice_type.clone()]).then_some(( - ReferenceConversionType::AsRefSlice, - could_deref_to_target(ty, &slice_type, db), - )) + ty.impls_trait(db, famous_defs.core_convert_AsRef()?, std::slice::from_ref(&slice_type)) + .then_some(( + ReferenceConversionType::AsRefSlice, + could_deref_to_target(ty, &slice_type, db), + )) } fn handle_dereferenced( @@ -937,10 +938,11 @@ fn handle_dereferenced( ) -> Option<(ReferenceConversionType, bool)> { let type_argument = ty.type_arguments().next()?; - ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[type_argument.clone()]).then_some(( - ReferenceConversionType::Dereferenced, - could_deref_to_target(ty, &type_argument, db), - )) + ty.impls_trait(db, famous_defs.core_convert_AsRef()?, std::slice::from_ref(&type_argument)) + .then_some(( + ReferenceConversionType::Dereferenced, + could_deref_to_target(ty, &type_argument, db), + )) } fn handle_option_as_ref( diff --git a/crates/load-cargo/src/lib.rs b/crates/load-cargo/src/lib.rs index 52f59679b5..577e2bfd2b 100644 --- a/crates/load-cargo/src/lib.rs +++ b/crates/load-cargo/src/lib.rs @@ -11,7 +11,7 @@ use hir_expand::proc_macro::{ }; use ide_db::{ ChangeWithProcMacros, FxHashMap, RootDatabase, - base_db::{CrateGraphBuilder, Env, SourceRoot, SourceRootId}, + base_db::{CrateGraphBuilder, Env, ProcMacroLoadingError, SourceRoot, SourceRootId}, prime_caches, }; use itertools::Itertools; @@ -81,19 +81,16 @@ pub fn load_workspace( ProcMacroServerChoice::Sysroot => ws .find_sysroot_proc_macro_srv() .and_then(|it| ProcMacroClient::spawn(&it, extra_env).map_err(Into::into)) - .map_err(|e| (e, true)), - ProcMacroServerChoice::Explicit(path) => { - ProcMacroClient::spawn(path, extra_env).map_err(Into::into).map_err(|e| (e, true)) - } - ProcMacroServerChoice::None => { - Err((anyhow::format_err!("proc macro server disabled"), false)) - } + .map_err(|e| ProcMacroLoadingError::ProcMacroSrvError(e.to_string().into_boxed_str())), + ProcMacroServerChoice::Explicit(path) => ProcMacroClient::spawn(path, extra_env) + .map_err(|e| ProcMacroLoadingError::ProcMacroSrvError(e.to_string().into_boxed_str())), + ProcMacroServerChoice::None => Err(ProcMacroLoadingError::Disabled), }; match &proc_macro_server { Ok(server) => { tracing::info!(path=%server.server_path(), "Proc-macro server started") } - Err((e, _)) => { + Err(e) => { tracing::info!(%e, "Failed to start proc-macro server") } } @@ -112,21 +109,18 @@ pub fn load_workspace( let proc_macros = { let proc_macro_server = match &proc_macro_server { Ok(it) => Ok(it), - Err((e, hard_err)) => Err((e.to_string(), *hard_err)), + Err(e) => Err(ProcMacroLoadingError::ProcMacroSrvError(e.to_string().into_boxed_str())), }; proc_macros .into_iter() .map(|(crate_id, path)| { ( crate_id, - path.map_or_else( - |e| Err((e, true)), - |(_, path)| { - proc_macro_server.as_ref().map_err(Clone::clone).and_then( - |proc_macro_server| load_proc_macro(proc_macro_server, &path, &[]), - ) - }, - ), + path.map_or_else(Err, |(_, path)| { + proc_macro_server.as_ref().map_err(Clone::clone).and_then( + |proc_macro_server| load_proc_macro(proc_macro_server, &path, &[]), + ) + }), ) }) .collect() @@ -391,11 +385,13 @@ pub fn load_proc_macro( path: &AbsPath, ignored_macros: &[Box], ) -> ProcMacroLoadResult { - let res: Result, String> = (|| { + let res: Result, _> = (|| { let dylib = MacroDylib::new(path.to_path_buf()); - let vec = server.load_dylib(dylib).map_err(|e| format!("{e}"))?; + let vec = server.load_dylib(dylib).map_err(|e| { + ProcMacroLoadingError::ProcMacroSrvError(format!("{e}").into_boxed_str()) + })?; if vec.is_empty() { - return Err("proc macro library returned no proc macros".to_owned()); + return Err(ProcMacroLoadingError::NoProcMacros); } Ok(vec .into_iter() @@ -412,7 +408,7 @@ pub fn load_proc_macro( } Err(e) => { tracing::warn!("proc-macro loading for {path} failed: {e}"); - Err((e, true)) + Err(e) } } } diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 5bc64df535..c824ff11d3 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -7,8 +7,8 @@ use std::{collections::VecDeque, fmt, fs, iter, ops::Deref, sync, thread}; use anyhow::Context; use base_db::{ CrateBuilderId, CrateDisplayName, CrateGraphBuilder, CrateName, CrateOrigin, - CrateWorkspaceData, DependencyBuilder, Env, LangCrateOrigin, ProcMacroPaths, - TargetLayoutLoadResult, + CrateWorkspaceData, DependencyBuilder, Env, LangCrateOrigin, ProcMacroLoadingError, + ProcMacroPaths, TargetLayoutLoadResult, }; use cfg::{CfgAtom, CfgDiff, CfgOptions}; use intern::{Symbol, sym}; @@ -1641,11 +1641,11 @@ fn add_target_crate_root( Some((BuildScriptOutput { proc_macro_dylib_path, .. }, has_errors)) => { match proc_macro_dylib_path { Some(path) => Ok((cargo_name.to_owned(), path.clone())), - None if has_errors => Err("failed to build proc-macro".to_owned()), - None => Err("proc-macro crate build data is missing dylib path".to_owned()), + None if has_errors => Err(ProcMacroLoadingError::FailedToBuild), + None => Err(ProcMacroLoadingError::MissingDylibPath), } } - None => Err("build scripts have not been built".to_owned()), + None => Err(ProcMacroLoadingError::NotYetBuilt), }; proc_macros.insert(crate_id, proc_macro); } diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 133d5a6cf7..0218d842e6 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -18,7 +18,7 @@ use std::{iter, mem}; use hir::{ChangeWithProcMacros, ProcMacrosBuilder, db::DefDatabase}; use ide_db::{ FxHashMap, - base_db::{CrateGraphBuilder, ProcMacroPaths, salsa::Durability}, + base_db::{CrateGraphBuilder, ProcMacroLoadingError, ProcMacroPaths, salsa::Durability}, }; use itertools::Itertools; use load_cargo::{ProjectFolders, load_proc_macro}; @@ -438,9 +438,11 @@ impl GlobalState { load_proc_macro(client, path, ignored_proc_macros) } - Err(e) => Err((e.clone(), true)), + Err(e) => Err(e.clone()), }, - Err(ref e) => Err((e.clone(), true)), + Err(ref e) => Err(ProcMacroLoadingError::ProcMacroSrvError( + e.clone().into_boxed_str(), + )), }; builder.insert(*crate_id, expansion_res) } @@ -753,14 +755,14 @@ impl GlobalState { change.set_proc_macros( crate_graph .iter() - .map(|id| (id, Err(("proc-macro has not been built yet".to_owned(), true)))) + .map(|id| (id, Err(ProcMacroLoadingError::NotYetBuilt))) .collect(), ); } else { change.set_proc_macros( crate_graph .iter() - .map(|id| (id, Err(("proc-macro expansion is disabled".to_owned(), false)))) + .map(|id| (id, Err(ProcMacroLoadingError::Disabled))) .collect(), ); }