From 7d51fc4640e1f59b882ff8a8095f4c4f08b80e9e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 15 Jun 2022 17:33:55 +0200 Subject: [PATCH 1/4] Show proc-macro loading errors in unresolved-proc-macro diagnostics --- crates/base-db/src/fixture.rs | 8 +-- crates/base-db/src/input.rs | 30 ++++----- crates/base-db/src/lib.rs | 2 +- crates/hir-def/src/nameres/collector.rs | 40 ++++++++---- crates/hir-def/src/nameres/diagnostics.rs | 13 +++- crates/hir-expand/src/proc_macro.rs | 10 ++- crates/hir/src/diagnostics.rs | 1 + crates/hir/src/lib.rs | 14 ++++- .../src/handlers/unresolved_proc_macro.rs | 26 ++++---- crates/ide/src/lib.rs | 2 +- crates/proc-macro-api/src/lib.rs | 16 ++--- crates/project-model/src/tests.rs | 2 +- crates/project-model/src/workspace.rs | 37 ++++++----- crates/rust-analyzer/src/reload.rs | 62 ++++++++----------- 14 files changed, 149 insertions(+), 114 deletions(-) diff --git a/crates/base-db/src/fixture.rs b/crates/base-db/src/fixture.rs index 69585b44a1..8e6e6a11ab 100644 --- a/crates/base-db/src/fixture.rs +++ b/crates/base-db/src/fixture.rs @@ -159,7 +159,7 @@ impl ChangeFixture { meta.cfg.clone(), meta.cfg, meta.env, - Default::default(), + Ok(Vec::new()), false, origin, ); @@ -194,7 +194,7 @@ impl ChangeFixture { default_cfg.clone(), default_cfg, Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -231,7 +231,7 @@ impl ChangeFixture { CfgOptions::default(), CfgOptions::default(), Env::default(), - Vec::new(), + Ok(Vec::new()), false, CrateOrigin::Lang(LangCrateOrigin::Core), ); @@ -268,7 +268,7 @@ impl ChangeFixture { CfgOptions::default(), CfgOptions::default(), Env::default(), - proc_macro, + Ok(proc_macro), true, CrateOrigin::CratesIo { repo: None }, ); diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 9fcaa4b06d..4d0a3a3012 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -231,6 +231,8 @@ pub enum ProcMacroExpansionError { System(String), } +pub type ProcMacroLoadResult = Result, String>; + #[derive(Debug, Clone)] pub struct ProcMacro { pub name: SmolStr, @@ -254,7 +256,7 @@ pub struct CrateData { pub potential_cfg_options: CfgOptions, pub env: Env, pub dependencies: Vec, - pub proc_macro: Vec, + pub proc_macro: ProcMacroLoadResult, pub origin: CrateOrigin, pub is_proc_macro: bool, } @@ -300,19 +302,19 @@ impl Dependency { impl CrateGraph { pub fn add_crate_root( &mut self, - file_id: FileId, + root_file_id: FileId, edition: Edition, display_name: Option, version: Option, cfg_options: CfgOptions, potential_cfg_options: CfgOptions, env: Env, - proc_macro: Vec, + proc_macro: ProcMacroLoadResult, is_proc_macro: bool, origin: CrateOrigin, ) -> CrateId { let data = CrateData { - root_file_id: file_id, + root_file_id, edition, version, display_name, @@ -628,7 +630,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -640,7 +642,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -652,7 +654,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -678,7 +680,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -690,7 +692,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -713,7 +715,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -725,7 +727,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -737,7 +739,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -760,7 +762,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -772,7 +774,7 @@ mod tests { CfgOptions::default(), CfgOptions::default(), Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); diff --git a/crates/base-db/src/lib.rs b/crates/base-db/src/lib.rs index 2454698bb6..f5622ecf60 100644 --- a/crates/base-db/src/lib.rs +++ b/crates/base-db/src/lib.rs @@ -13,7 +13,7 @@ pub use crate::{ input::{ CrateData, CrateDisplayName, CrateGraph, CrateId, CrateName, CrateOrigin, Dependency, Edition, Env, LangCrateOrigin, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, - ProcMacroId, ProcMacroKind, SourceRoot, SourceRootId, + ProcMacroId, ProcMacroKind, ProcMacroLoadResult, SourceRoot, SourceRootId, }, }; pub use salsa::{self, Cancelled}; diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 7fea46bee3..1c276e0d6f 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -74,19 +74,27 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T } let cfg_options = &krate.cfg_options; - let proc_macros = krate - .proc_macro - .iter() - .enumerate() - .map(|(idx, it)| { - // FIXME: a hacky way to create a Name from string. - let name = tt::Ident { text: it.name.clone(), id: tt::TokenId::unspecified() }; + let (proc_macros, proc_macro_err) = match &krate.proc_macro { + Ok(proc_macros) => { ( - name.as_name(), - ProcMacroExpander::new(def_map.krate, base_db::ProcMacroId(idx as u32)), + proc_macros + .iter() + .enumerate() + .map(|(idx, it)| { + // FIXME: a hacky way to create a Name from string. + let name = + tt::Ident { text: it.name.clone(), id: tt::TokenId::unspecified() }; + ( + name.as_name(), + ProcMacroExpander::new(def_map.krate, base_db::ProcMacroId(idx as u32)), + ) + }) + .collect(), + None, ) - }) - .collect(); + } + Err(e) => (Vec::new(), Some(e.clone())), + }; let is_proc_macro = krate.is_proc_macro; let mut collector = DefCollector { @@ -100,6 +108,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T mod_dirs: FxHashMap::default(), cfg_options, proc_macros, + proc_macro_err, from_glob_import: Default::default(), skip_attrs: Default::default(), derive_helpers_in_scope: Default::default(), @@ -241,6 +250,7 @@ struct DefCollector<'a> { /// empty when proc. macro support is disabled (in which case we still do name resolution for /// them). proc_macros: Vec<(Name, ProcMacroExpander)>, + proc_macro_err: Option, 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. @@ -1232,6 +1242,7 @@ impl DefCollector<'_> { self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( directive.module_id, loc.kind, + self.proc_macro_err.clone(), )); return recollect_without(self); @@ -1283,7 +1294,11 @@ impl DefCollector<'_> { let diag = match err { hir_expand::ExpandError::UnresolvedProcMacro => { // Missing proc macros are non-fatal, so they are handled specially. - DefDiagnostic::unresolved_proc_macro(module_id, loc.kind.clone()) + DefDiagnostic::unresolved_proc_macro( + module_id, + loc.kind.clone(), + self.proc_macro_err.clone(), + ) } _ => DefDiagnostic::macro_error(module_id, loc.kind.clone(), err.to_string()), }; @@ -2097,6 +2112,7 @@ mod tests { mod_dirs: FxHashMap::default(), cfg_options: &CfgOptions::default(), proc_macros: Default::default(), + proc_macro_err: None, from_glob_import: Default::default(), skip_attrs: Default::default(), derive_helpers_in_scope: Default::default(), diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs index dd3ff92cb3..927237962d 100644 --- a/crates/hir-def/src/nameres/diagnostics.rs +++ b/crates/hir-def/src/nameres/diagnostics.rs @@ -23,7 +23,7 @@ pub enum DefDiagnosticKind { UnconfiguredCode { ast: AstId, cfg: CfgExpr, opts: CfgOptions }, - UnresolvedProcMacro { ast: MacroCallKind }, + UnresolvedProcMacro { ast: MacroCallKind, proc_macro_err: Option }, UnresolvedMacroCall { ast: MacroCallKind, path: ModPath }, @@ -81,8 +81,15 @@ impl DefDiagnostic { Self { in_module: container, kind: DefDiagnosticKind::UnconfiguredCode { ast, cfg, opts } } } - pub(super) fn unresolved_proc_macro(container: LocalModuleId, ast: MacroCallKind) -> Self { - Self { in_module: container, kind: DefDiagnosticKind::UnresolvedProcMacro { ast } } + pub(super) fn unresolved_proc_macro( + container: LocalModuleId, + ast: MacroCallKind, + proc_macro_err: Option, + ) -> Self { + Self { + in_module: container, + kind: DefDiagnosticKind::UnresolvedProcMacro { ast, proc_macro_err }, + } } pub(super) fn macro_error( diff --git a/crates/hir-expand/src/proc_macro.rs b/crates/hir-expand/src/proc_macro.rs index df6c38761c..b2686592a5 100644 --- a/crates/hir-expand/src/proc_macro.rs +++ b/crates/hir-expand/src/proc_macro.rs @@ -34,7 +34,15 @@ impl ProcMacroExpander { match self.proc_macro_id { Some(id) => { let krate_graph = db.crate_graph(); - let proc_macro = match krate_graph[self.krate].proc_macro.get(id.0 as usize) { + let proc_macros = match &krate_graph[self.krate].proc_macro { + Ok(proc_macros) => proc_macros, + Err(e) => { + return ExpandResult::only_err(ExpandError::Other( + e.clone().into_boxed_str(), + )) + } + }; + let proc_macro = match proc_macros.get(id.0 as usize) { Some(proc_macro) => proc_macro, None => { return ExpandResult::only_err(ExpandError::Other( diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 0c88d15b08..4dd23dfa14 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -87,6 +87,7 @@ pub struct UnresolvedProcMacro { pub precise_location: Option, pub macro_name: Option, pub kind: MacroKind, + pub proc_macro_err: Option, } #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 4620d0c03a..9f7a2d63fe 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -627,7 +627,7 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: ); } - DefDiagnosticKind::UnresolvedProcMacro { ast } => { + DefDiagnosticKind::UnresolvedProcMacro { ast, proc_macro_err } => { let (node, precise_location, macro_name, kind) = match ast { MacroCallKind::FnLike { ast_id, .. } => { let node = ast_id.to_node(db.upcast()); @@ -689,7 +689,16 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: ) } }; - acc.push(UnresolvedProcMacro { node, precise_location, macro_name, kind }.into()); + acc.push( + UnresolvedProcMacro { + node, + precise_location, + macro_name, + kind, + proc_macro_err: proc_macro_err.clone(), + } + .into(), + ); } DefDiagnosticKind::UnresolvedMacroCall { ast, path } => { @@ -1163,6 +1172,7 @@ impl DefWithBody { precise_location: None, macro_name: None, kind: MacroKind::ProcMacro, + proc_macro_err: None, } .into(), ), diff --git a/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs b/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs index 37350a7aaf..5f905de4f1 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs @@ -29,19 +29,23 @@ pub(crate) fn unresolved_proc_macro( Some(name) => format!("proc macro `{}` not expanded", name), None => "proc macro not expanded".to_string(), }; - let (message, severity) = if config_enabled { - (message, Severity::Error) - } else { - let message = match d.kind { - hir::MacroKind::Attr if proc_macros_enabled => { - format!("{message}{}", " (attribute macro expansion is disabled)") + let severity = if config_enabled { Severity::Error } else { Severity::WeakWarning }; + let message = format!( + "{message}: {}", + if config_enabled { + match &d.proc_macro_err { + Some(e) => e, + None => "proc macro not found", } - _ => { - format!("{message}{}", " (proc-macro expansion is disabled)") + } else { + match d.kind { + hir::MacroKind::Attr if proc_macros_enabled => { + "attribute macro expansion is disabled" + } + _ => "proc-macro expansion is disabled", } - }; - (message, Severity::WeakWarning) - }; + }, + ); Diagnostic::new("unresolved-proc-macro", message, display_range).severity(severity) } diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index aab5ceda36..07a7fbd783 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -233,7 +233,7 @@ impl Analysis { cfg_options.clone(), cfg_options, Env::default(), - Default::default(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); diff --git a/crates/proc-macro-api/src/lib.rs b/crates/proc-macro-api/src/lib.rs index eb7a28b972..4a30168ca5 100644 --- a/crates/proc-macro-api/src/lib.rs +++ b/crates/proc-macro-api/src/lib.rs @@ -118,16 +118,13 @@ impl ProcMacroServer { Ok(ProcMacroServer { process: Arc::new(Mutex::new(process)) }) } - pub fn load_dylib( - &self, - dylib: MacroDylib, - ) -> Result, String>, ServerError> { + pub fn load_dylib(&self, dylib: MacroDylib) -> Result, ServerError> { let _p = profile::span("ProcMacroClient::by_dylib_path"); let macros = self.process.lock().unwrap_or_else(|e| e.into_inner()).find_proc_macros(&dylib.path)?; - let res = macros.map(|macros| { - macros + match macros { + Ok(macros) => Ok(macros .into_iter() .map(|(name, kind)| ProcMacro { process: self.process.clone(), @@ -135,10 +132,9 @@ impl ProcMacroServer { kind, dylib_path: dylib.path.clone(), }) - .collect() - }); - - Ok(res) + .collect()), + Err(message) => Err(ServerError { message, io: None }), + } } } diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index e72903e55f..8eb8c40a22 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -88,7 +88,7 @@ fn rooted_project_json(data: ProjectJsonData) -> ProjectJson { } fn to_crate_graph(project_workspace: ProjectWorkspace) -> CrateGraph { - project_workspace.to_crate_graph(&mut |_, _| Vec::new(), &mut { + project_workspace.to_crate_graph(&mut |_, _| Ok(Vec::new()), &mut { let mut counter = 0; move |_path| { counter += 1; diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index a94a38a17d..8982a9904e 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -7,7 +7,7 @@ use std::{collections::VecDeque, fmt, fs, process::Command}; use anyhow::{format_err, Context, Result}; use base_db::{ CrateDisplayName, CrateGraph, CrateId, CrateName, CrateOrigin, Dependency, Edition, Env, - FileId, LangCrateOrigin, ProcMacro, + FileId, LangCrateOrigin, ProcMacroLoadResult, }; use cfg::{CfgDiff, CfgOptions}; use paths::{AbsPath, AbsPathBuf}; @@ -389,7 +389,7 @@ impl ProjectWorkspace { pub fn to_crate_graph( &self, - load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> Vec, + load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> ProcMacroLoadResult, load: &mut dyn FnMut(&AbsPath) -> Option, ) -> CrateGraph { let _p = profile::span("ProjectWorkspace::to_crate_graph"); @@ -434,7 +434,7 @@ impl ProjectWorkspace { fn project_json_to_crate_graph( rustc_cfg: Vec, - load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> Vec, + load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> ProcMacroLoadResult, load: &mut dyn FnMut(&AbsPath) -> Option, project: &ProjectJson, sysroot: &Option, @@ -454,12 +454,13 @@ fn project_json_to_crate_graph( }) .map(|(crate_id, krate, file_id)| { let env = krate.env.clone().into_iter().collect(); - let proc_macro = krate.proc_macro_dylib_path.clone().map(|it| { - load_proc_macro( + let proc_macro = match krate.proc_macro_dylib_path.clone() { + Some(it) => load_proc_macro( krate.display_name.as_ref().map(|it| it.canonical_name()).unwrap_or(""), &it, - ) - }); + ), + None => Ok(Vec::new()), + }; let target_cfgs = match krate.target.as_deref() { Some(target) => { @@ -480,7 +481,7 @@ fn project_json_to_crate_graph( cfg_options.clone(), cfg_options, env, - proc_macro.unwrap_or_default(), + proc_macro, krate.is_proc_macro, if krate.display_name.is_some() { CrateOrigin::CratesIo { repo: krate.repository.clone() } @@ -521,7 +522,7 @@ fn project_json_to_crate_graph( fn cargo_to_crate_graph( rustc_cfg: Vec, override_cfg: &CfgOverrides, - load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> Vec, + load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> ProcMacroLoadResult, load: &mut dyn FnMut(&AbsPath) -> Option, cargo: &CargoWorkspace, build_scripts: &WorkspaceBuildScripts, @@ -708,7 +709,7 @@ fn detached_files_to_crate_graph( cfg_options.clone(), cfg_options.clone(), Env::default(), - Vec::new(), + Ok(Vec::new()), false, CrateOrigin::CratesIo { repo: None }, ); @@ -724,7 +725,7 @@ fn handle_rustc_crates( crate_graph: &mut CrateGraph, cfg_options: &CfgOptions, override_cfg: &CfgOverrides, - load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> Vec, + load_proc_macro: &mut dyn FnMut(&str, &AbsPath) -> ProcMacroLoadResult, pkg_to_lib_crate: &mut FxHashMap, CrateId>, public_deps: &SysrootPublicDeps, cargo: &CargoWorkspace, @@ -840,7 +841,7 @@ fn add_target_crate_root( pkg: &PackageData, build_data: Option<&BuildScriptOutput>, cfg_options: &CfgOptions, - load_proc_macro: &mut dyn FnMut(&AbsPath) -> Vec, + load_proc_macro: &mut dyn FnMut(&AbsPath) -> ProcMacroLoadResult, file_id: FileId, cargo_name: &str, is_proc_macro: bool, @@ -866,11 +867,10 @@ fn add_target_crate_root( } } - let proc_macro = build_data - .as_ref() - .and_then(|it| it.proc_macro_dylib_path.as_ref()) - .map(|it| load_proc_macro(it)) - .unwrap_or_default(); + let proc_macro = match build_data.as_ref().and_then(|it| it.proc_macro_dylib_path.as_ref()) { + Some(it) => load_proc_macro(it), + None => Ok(Vec::new()), + }; let display_name = CrateDisplayName::from_canonical_name(cargo_name.to_string()); let mut potential_cfg_options = cfg_options.clone(); @@ -922,7 +922,6 @@ fn sysroot_to_crate_graph( let file_id = load(&sysroot[krate].root)?; let env = Env::default(); - let proc_macro = vec![]; let display_name = CrateDisplayName::from_canonical_name(sysroot[krate].name.clone()); let crate_id = crate_graph.add_crate_root( file_id, @@ -932,7 +931,7 @@ fn sysroot_to_crate_graph( cfg_options.clone(), cfg_options.clone(), env, - proc_macro, + Ok(Vec::new()), false, CrateOrigin::Lang(LangCrateOrigin::from(&*sysroot[krate].name)), ); diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 62a446ce2a..3f129efd95 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -19,7 +19,7 @@ use hir::db::DefDatabase; use ide::Change; use ide_db::base_db::{ CrateGraph, Env, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacroKind, - SourceRoot, VfsPath, + ProcMacroLoadResult, SourceRoot, VfsPath, }; use proc_macro_api::{MacroDylib, ProcMacroServer}; use project_model::{ProjectWorkspace, WorkspaceBuildScripts}; @@ -536,45 +536,37 @@ impl SourceRootConfig { /// Load the proc-macros for the given lib path, replacing all expanders whose names are in `dummy_replace` /// with an identity dummy expander. pub(crate) fn load_proc_macro( - client: Option<&ProcMacroServer>, + server: Option<&ProcMacroServer>, path: &AbsPath, dummy_replace: &[Box], -) -> Vec { - let dylib = match MacroDylib::new(path.to_path_buf()) { - Ok(it) => it, - Err(err) => { - // FIXME: that's not really right -- we store this error in a - // persistent status. - tracing::warn!("failed to load proc macro: {}", err); - return Vec::new(); +) -> ProcMacroLoadResult { + let res: Result<_, String> = (|| { + let dylib = MacroDylib::new(path.to_path_buf()) + .map_err(|io| format!("Proc-macro dylib loading failed: {io}"))?; + Ok(if let Some(it) = server { + let vec = it.load_dylib(dylib).map_err(|e| format!("{e}"))?; + vec.into_iter() + .map(|expander| expander_to_proc_macro(expander, dummy_replace)) + .collect() + } else { + Vec::new() + }) + })(); + return match res { + Ok(proc_macros) => { + tracing::info!( + "Loaded proc-macros for {}: {:?}", + path.display(), + proc_macros.iter().map(|it| it.name.clone()).collect::>() + ); + Ok(proc_macros) + } + Err(e) => { + tracing::warn!("proc-macro loading for {} failed: {e}", path.display()); + Err(e) } }; - let proc_macros: Vec<_> = client - .map(|it| it.load_dylib(dylib)) - .into_iter() - .flat_map(|it| match it { - Ok(Ok(macros)) => macros, - Err(err) => { - tracing::error!("proc macro server crashed: {}", err); - Vec::new() - } - Ok(Err(err)) => { - // FIXME: that's not really right -- we store this error in a - // persistent status. - tracing::warn!("failed to load proc macro: {}", err); - Vec::new() - } - }) - .map(|expander| expander_to_proc_macro(expander, dummy_replace)) - .collect(); - tracing::info!( - "Loaded proc-macros for {}: {:?}", - path.display(), - proc_macros.iter().map(|it| it.name.clone()).collect::>() - ); - return proc_macros; - fn expander_to_proc_macro( expander: proc_macro_api::ProcMacro, dummy_replace: &[Box], From 1d34cdcac07b23dafa7826ecf73ce2008b4be479 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 15 Jun 2022 17:34:13 +0200 Subject: [PATCH 2/4] Diagnose unresolved attribute proc-macros --- crates/hir-def/src/nameres/collector.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 1c276e0d6f..46f752f00a 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1138,7 +1138,19 @@ impl DefCollector<'_> { let def = match resolver(path.clone()) { Some(def) if def.is_attribute() => def, - _ => return true, + _ => { + self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( + directive.module_id, + MacroCallKind::Attr { + ast_id, + attr_args: Default::default(), + invoc_attr_index: attr.id.ast_index, + is_derive: false, + }, + self.proc_macro_err.clone(), + )); + return true; + } }; if matches!( def, From 0e41d15b823423363ff99f4170fa62ad70476290 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 15 Jun 2022 18:04:39 +0200 Subject: [PATCH 3/4] Use the correct crates proc-macro loading error message --- crates/hir-def/src/nameres.rs | 7 +++ crates/hir-def/src/nameres/collector.rs | 44 +++++++++---------- crates/hir-def/src/nameres/diagnostics.rs | 10 ++--- crates/hir/src/diagnostics.rs | 4 +- crates/hir/src/lib.rs | 14 ++---- .../src/handlers/unresolved_proc_macro.rs | 5 ++- 6 files changed, 42 insertions(+), 42 deletions(-) diff --git a/crates/hir-def/src/nameres.rs b/crates/hir-def/src/nameres.rs index e923d883ec..536eecf020 100644 --- a/crates/hir-def/src/nameres.rs +++ b/crates/hir-def/src/nameres.rs @@ -103,6 +103,8 @@ pub struct DefMap { /// Side table for resolving derive helpers. exported_derives: FxHashMap>, fn_proc_macro_mapping: FxHashMap, + /// The error that occurred when failing to load the proc-macro dll. + proc_macro_loading_error: Option>, /// Custom attributes registered with `#![register_attr]`. registered_attrs: Vec, @@ -273,6 +275,7 @@ impl DefMap { extern_prelude: FxHashMap::default(), exported_derives: FxHashMap::default(), fn_proc_macro_mapping: FxHashMap::default(), + proc_macro_loading_error: None, prelude: None, root, modules, @@ -305,6 +308,9 @@ impl DefMap { pub fn fn_as_proc_macro(&self, id: FunctionId) -> Option { self.fn_proc_macro_mapping.get(&id).copied() } + pub fn proc_macro_loading_error(&self) -> Option<&str> { + self.proc_macro_loading_error.as_deref() + } pub(crate) fn krate(&self) -> CrateId { self.krate @@ -460,6 +466,7 @@ impl DefMap { registered_attrs, registered_tools, fn_proc_macro_mapping, + proc_macro_loading_error: _, block: _, edition: _, recursion_limit: _, diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 46f752f00a..6a9f569ea8 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -74,26 +74,25 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T } let cfg_options = &krate.cfg_options; - let (proc_macros, proc_macro_err) = match &krate.proc_macro { + let proc_macros = match &krate.proc_macro { Ok(proc_macros) => { - ( - proc_macros - .iter() - .enumerate() - .map(|(idx, it)| { - // FIXME: a hacky way to create a Name from string. - let name = - tt::Ident { text: it.name.clone(), id: tt::TokenId::unspecified() }; - ( - name.as_name(), - ProcMacroExpander::new(def_map.krate, base_db::ProcMacroId(idx as u32)), - ) - }) - .collect(), - None, - ) + proc_macros + .iter() + .enumerate() + .map(|(idx, it)| { + // FIXME: a hacky way to create a Name from string. + let name = tt::Ident { text: it.name.clone(), id: tt::TokenId::unspecified() }; + ( + name.as_name(), + ProcMacroExpander::new(def_map.krate, base_db::ProcMacroId(idx as u32)), + ) + }) + .collect() + } + Err(e) => { + def_map.proc_macro_loading_error = Some(e.clone().into_boxed_str()); + Vec::new() } - Err(e) => (Vec::new(), Some(e.clone())), }; let is_proc_macro = krate.is_proc_macro; @@ -108,7 +107,6 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: DefMap, tree_id: T mod_dirs: FxHashMap::default(), cfg_options, proc_macros, - proc_macro_err, from_glob_import: Default::default(), skip_attrs: Default::default(), derive_helpers_in_scope: Default::default(), @@ -250,7 +248,6 @@ struct DefCollector<'a> { /// empty when proc. macro support is disabled (in which case we still do name resolution for /// them). proc_macros: Vec<(Name, ProcMacroExpander)>, - proc_macro_err: Option, 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. @@ -1147,7 +1144,7 @@ impl DefCollector<'_> { invoc_attr_index: attr.id.ast_index, is_derive: false, }, - self.proc_macro_err.clone(), + None, )); return true; } @@ -1254,7 +1251,7 @@ impl DefCollector<'_> { self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( directive.module_id, loc.kind, - self.proc_macro_err.clone(), + Some(loc.def.krate), )); return recollect_without(self); @@ -1309,7 +1306,7 @@ impl DefCollector<'_> { DefDiagnostic::unresolved_proc_macro( module_id, loc.kind.clone(), - self.proc_macro_err.clone(), + Some(loc.def.krate), ) } _ => DefDiagnostic::macro_error(module_id, loc.kind.clone(), err.to_string()), @@ -2124,7 +2121,6 @@ mod tests { mod_dirs: FxHashMap::default(), cfg_options: &CfgOptions::default(), proc_macros: Default::default(), - proc_macro_err: None, from_glob_import: Default::default(), skip_attrs: Default::default(), derive_helpers_in_scope: Default::default(), diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs index 927237962d..3ebc5629d7 100644 --- a/crates/hir-def/src/nameres/diagnostics.rs +++ b/crates/hir-def/src/nameres/diagnostics.rs @@ -1,5 +1,6 @@ //! Diagnostics emitted during DefMap construction. +use base_db::CrateId; use cfg::{CfgExpr, CfgOptions}; use hir_expand::MacroCallKind; use la_arena::Idx; @@ -23,7 +24,7 @@ pub enum DefDiagnosticKind { UnconfiguredCode { ast: AstId, cfg: CfgExpr, opts: CfgOptions }, - UnresolvedProcMacro { ast: MacroCallKind, proc_macro_err: Option }, + UnresolvedProcMacro { ast: MacroCallKind, krate: Option }, UnresolvedMacroCall { ast: MacroCallKind, path: ModPath }, @@ -84,12 +85,9 @@ impl DefDiagnostic { pub(super) fn unresolved_proc_macro( container: LocalModuleId, ast: MacroCallKind, - proc_macro_err: Option, + krate: Option, ) -> Self { - Self { - in_module: container, - kind: DefDiagnosticKind::UnresolvedProcMacro { ast, proc_macro_err }, - } + Self { in_module: container, kind: DefDiagnosticKind::UnresolvedProcMacro { ast, krate } } } pub(super) fn macro_error( diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 4dd23dfa14..1f65c05c1e 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -3,6 +3,7 @@ //! //! This probably isn't the best way to do this -- ideally, diagnistics should //! be expressed in terms of hir types themselves. +use base_db::CrateId; use cfg::{CfgExpr, CfgOptions}; use either::Either; use hir_def::path::ModPath; @@ -87,7 +88,8 @@ pub struct UnresolvedProcMacro { pub precise_location: Option, pub macro_name: Option, pub kind: MacroKind, - pub proc_macro_err: Option, + /// The crate id of the proc-macro this macro belongs to, or `None` if the proc-macro can't be found. + pub krate: Option, } #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 9f7a2d63fe..3f0d586bf6 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -627,7 +627,7 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: ); } - DefDiagnosticKind::UnresolvedProcMacro { ast, proc_macro_err } => { + DefDiagnosticKind::UnresolvedProcMacro { ast, krate } => { let (node, precise_location, macro_name, kind) = match ast { MacroCallKind::FnLike { ast_id, .. } => { let node = ast_id.to_node(db.upcast()); @@ -690,14 +690,8 @@ fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec, diag: } }; acc.push( - UnresolvedProcMacro { - node, - precise_location, - macro_name, - kind, - proc_macro_err: proc_macro_err.clone(), - } - .into(), + UnresolvedProcMacro { node, precise_location, macro_name, kind, krate: *krate } + .into(), ); } @@ -1172,7 +1166,7 @@ impl DefWithBody { precise_location: None, macro_name: None, kind: MacroKind::ProcMacro, - proc_macro_err: None, + krate: None, } .into(), ), diff --git a/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs b/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs index 5f905de4f1..5ebfe33dab 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_proc_macro.rs @@ -1,3 +1,5 @@ +use hir::db::DefDatabase; + use crate::{Diagnostic, DiagnosticsContext, Severity}; // Diagnostic: unresolved-proc-macro @@ -30,10 +32,11 @@ pub(crate) fn unresolved_proc_macro( None => "proc macro not expanded".to_string(), }; let severity = if config_enabled { Severity::Error } else { Severity::WeakWarning }; + let def_map = d.krate.map(|krate| ctx.sema.db.crate_def_map(krate)); let message = format!( "{message}: {}", if config_enabled { - match &d.proc_macro_err { + match def_map.as_ref().and_then(|def_map| def_map.proc_macro_loading_error()) { Some(e) => e, None => "proc macro not found", } From 1dd2c502989f90a3494aa9a302869bacca7f9463 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 15 Jun 2022 18:07:37 +0200 Subject: [PATCH 4/4] Update test outputs --- crates/project-model/src/tests.rs | 108 ++++++++++++++++++++++-------- 1 file changed, 81 insertions(+), 27 deletions(-) diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index 8eb8c40a22..1a43935470 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -172,7 +172,9 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -245,7 +247,9 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -308,7 +312,9 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { }, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: Some( "https://github.com/rust-lang/libc", @@ -383,7 +389,9 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -456,7 +464,9 @@ fn cargo_hello_world_project_model_with_wildcard_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -544,7 +554,9 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -619,7 +631,9 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -682,7 +696,9 @@ fn cargo_hello_world_project_model_with_selective_overrides() { }, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: Some( "https://github.com/rust-lang/libc", @@ -759,7 +775,9 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -834,7 +852,9 @@ fn cargo_hello_world_project_model_with_selective_overrides() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -913,7 +933,9 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -988,7 +1010,9 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -1053,7 +1077,9 @@ fn cargo_hello_world_project_model() { }, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: Some( "https://github.com/rust-lang/libc", @@ -1130,7 +1156,9 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -1205,7 +1233,9 @@ fn cargo_hello_world_project_model() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -1260,7 +1290,9 @@ fn rust_project_hello_world_project_model() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Alloc, ), @@ -1292,7 +1324,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Other, ), @@ -1324,7 +1358,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Other, ), @@ -1366,7 +1402,9 @@ fn rust_project_hello_world_project_model() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Other, ), @@ -1398,7 +1436,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Core, ), @@ -1467,7 +1507,9 @@ fn rust_project_hello_world_project_model() { prelude: false, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: CratesIo { repo: None, }, @@ -1499,7 +1541,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Other, ), @@ -1531,7 +1575,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Other, ), @@ -1563,7 +1609,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Other, ), @@ -1595,7 +1643,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Test, ), @@ -1709,7 +1759,9 @@ fn rust_project_hello_world_project_model() { prelude: true, }, ], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Std, ), @@ -1741,7 +1793,9 @@ fn rust_project_hello_world_project_model() { entries: {}, }, dependencies: [], - proc_macro: [], + proc_macro: Ok( + [], + ), origin: Lang( Other, ),