From c21860bd6a8ec683f881aa8509fd7c61b074f2eb Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 26 Apr 2023 08:06:15 +0200 Subject: [PATCH] Remove proc-macro server command from the rust-analyzer binary --- Cargo.lock | 1 - crates/proc-macro-api/src/lib.rs | 8 +-- crates/proc-macro-api/src/process.rs | 24 ++------- crates/proc-macro-srv-cli/src/lib.rs | 54 ------------------- crates/proc-macro-srv-cli/src/main.rs | 35 ++++++++++++- crates/project-model/src/workspace.rs | 23 ++++++-- crates/rust-analyzer/Cargo.toml | 3 -- crates/rust-analyzer/src/bin/main.rs | 3 -- crates/rust-analyzer/src/cli/flags.rs | 6 --- crates/rust-analyzer/src/cli/load_cargo.rs | 25 ++++----- crates/rust-analyzer/src/config.rs | 21 +++----- crates/rust-analyzer/src/global_state.rs | 2 +- crates/rust-analyzer/src/reload.rs | 61 +++++++++------------- docs/user/generated_config.adoc | 3 +- editors/code/package.json | 2 +- 15 files changed, 109 insertions(+), 162 deletions(-) delete mode 100644 crates/proc-macro-srv-cli/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 8759782bb2..e0d5878d7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1464,7 +1464,6 @@ dependencies = [ "parking_lot 0.12.1", "parking_lot_core 0.9.6", "proc-macro-api", - "proc-macro-srv-cli", "profile", "project-model", "rayon", diff --git a/crates/proc-macro-api/src/lib.rs b/crates/proc-macro-api/src/lib.rs index 14a7b913bf..ab15f414bb 100644 --- a/crates/proc-macro-api/src/lib.rs +++ b/crates/proc-macro-api/src/lib.rs @@ -13,7 +13,6 @@ mod version; use paths::AbsPathBuf; use std::{ - ffi::OsStr, fmt, io, sync::{Arc, Mutex}, }; @@ -103,11 +102,8 @@ pub struct MacroPanic { impl ProcMacroServer { /// Spawns an external process as the proc macro server and returns a client connected to it. - pub fn spawn( - process_path: AbsPathBuf, - args: impl IntoIterator> + Clone, - ) -> io::Result { - let process = ProcMacroProcessSrv::run(process_path, args)?; + pub fn spawn(process_path: AbsPathBuf) -> io::Result { + let process = ProcMacroProcessSrv::run(process_path)?; Ok(ProcMacroServer { process: Arc::new(Mutex::new(process)) }) } diff --git a/crates/proc-macro-api/src/process.rs b/crates/proc-macro-api/src/process.rs index a526a21831..9a20fa63ed 100644 --- a/crates/proc-macro-api/src/process.rs +++ b/crates/proc-macro-api/src/process.rs @@ -1,7 +1,6 @@ //! Handle process life-time and message passing for proc-macro client use std::{ - ffi::{OsStr, OsString}, io::{self, BufRead, BufReader, Write}, process::{Child, ChildStdin, ChildStdout, Command, Stdio}, }; @@ -23,12 +22,9 @@ pub(crate) struct ProcMacroProcessSrv { } impl ProcMacroProcessSrv { - pub(crate) fn run( - process_path: AbsPathBuf, - args: impl IntoIterator> + Clone, - ) -> io::Result { + pub(crate) fn run(process_path: AbsPathBuf) -> io::Result { let create_srv = |null_stderr| { - let mut process = Process::run(process_path.clone(), args.clone(), null_stderr)?; + let mut process = Process::run(process_path.clone(), null_stderr)?; let (stdin, stdout) = process.stdio().expect("couldn't access child stdio"); io::Result::Ok(ProcMacroProcessSrv { _process: process, stdin, stdout, version: 0 }) @@ -100,13 +96,8 @@ struct Process { } impl Process { - fn run( - path: AbsPathBuf, - args: impl IntoIterator>, - null_stderr: bool, - ) -> io::Result { - let args: Vec = args.into_iter().map(|s| s.as_ref().into()).collect(); - let child = JodChild(mk_child(&path, args, null_stderr)?); + fn run(path: AbsPathBuf, null_stderr: bool) -> io::Result { + let child = JodChild(mk_child(&path, null_stderr)?); Ok(Process { child }) } @@ -119,13 +110,8 @@ impl Process { } } -fn mk_child( - path: &AbsPath, - args: impl IntoIterator>, - null_stderr: bool, -) -> io::Result { +fn mk_child(path: &AbsPath, null_stderr: bool) -> io::Result { Command::new(path.as_os_str()) - .args(args) .env("RUST_ANALYZER_INTERNALS_DO_NOT_USE", "this is unstable") .stdin(Stdio::piped()) .stdout(Stdio::piped()) diff --git a/crates/proc-macro-srv-cli/src/lib.rs b/crates/proc-macro-srv-cli/src/lib.rs deleted file mode 100644 index b307701e19..0000000000 --- a/crates/proc-macro-srv-cli/src/lib.rs +++ /dev/null @@ -1,54 +0,0 @@ -//! Driver for proc macro server -use std::io; - -use proc_macro_api::msg::{self, Message}; - -#[cfg(feature = "sysroot-abi")] -pub fn run() -> io::Result<()> { - let mut srv = proc_macro_srv::ProcMacroSrv::default(); - let mut buf = String::new(); - - while let Some(req) = read_request(&mut buf)? { - let res = match req { - msg::Request::ListMacros { dylib_path } => { - msg::Response::ListMacros(srv.list_macros(&dylib_path)) - } - msg::Request::ExpandMacro(task) => msg::Response::ExpandMacro(srv.expand(task)), - msg::Request::ApiVersionCheck {} => { - msg::Response::ApiVersionCheck(proc_macro_api::msg::CURRENT_API_VERSION) - } - }; - write_response(res)? - } - - Ok(()) -} -#[cfg(not(feature = "sysroot-abi"))] -pub fn run() -> io::Result<()> { - let mut buf = String::new(); - - while let Some(req) = read_request(&mut buf)? { - let res = match req { - msg::Request::ListMacros { .. } => { - msg::Response::ListMacros(Err("server is built without sysroot support".to_owned())) - } - msg::Request::ExpandMacro(..) => msg::Response::ExpandMacro(Err(msg::PanicMessage( - "server is built without sysroot support".to_owned(), - ))), - msg::Request::ApiVersionCheck {} => { - msg::Response::ApiVersionCheck(proc_macro_api::msg::CURRENT_API_VERSION) - } - }; - write_response(res)? - } - - Ok(()) -} - -fn read_request(buf: &mut String) -> io::Result> { - msg::Request::read(&mut io::stdin().lock(), buf) -} - -fn write_response(msg: msg::Response) -> io::Result<()> { - msg.write(&mut io::stdout().lock()) -} diff --git a/crates/proc-macro-srv-cli/src/main.rs b/crates/proc-macro-srv-cli/src/main.rs index cc83c60f15..bece195187 100644 --- a/crates/proc-macro-srv-cli/src/main.rs +++ b/crates/proc-macro-srv-cli/src/main.rs @@ -1,5 +1,6 @@ //! A standalone binary for `proc-macro-srv`. //! Driver for proc macro server +use std::io; fn main() -> std::io::Result<()> { let v = std::env::var("RUST_ANALYZER_INTERNALS_DO_NOT_USE"); @@ -14,5 +15,37 @@ fn main() -> std::io::Result<()> { } } - proc_macro_srv_cli::run() + run() +} + +#[cfg(not(feature = "sysroot-abi"))] +fn run() -> io::Result<()> { + panic!("proc-macro-srv-cli requires the `sysroot-abi` feature to be enabled"); +} + +#[cfg(feature = "sysroot-abi")] +fn run() -> io::Result<()> { + use proc_macro_api::msg::{self, Message}; + + let read_request = |buf: &mut String| msg::Request::read(&mut io::stdin().lock(), buf); + + let write_response = |msg: msg::Response| msg.write(&mut io::stdout().lock()); + + let mut srv = proc_macro_srv::ProcMacroSrv::default(); + let mut buf = String::new(); + + while let Some(req) = read_request(&mut buf)? { + let res = match req { + msg::Request::ListMacros { dylib_path } => { + msg::Response::ListMacros(srv.list_macros(&dylib_path)) + } + msg::Request::ExpandMacro(task) => msg::Response::ExpandMacro(srv.expand(task)), + msg::Request::ApiVersionCheck {} => { + msg::Response::ApiVersionCheck(proc_macro_api::msg::CURRENT_API_VERSION) + } + }; + write_response(res)? + } + + Ok(()) } diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 5518b6bc7f..02fda6e6ea 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -459,18 +459,35 @@ impl ProjectWorkspace { } } - pub fn find_sysroot_proc_macro_srv(&self) -> Option { + pub fn find_sysroot_proc_macro_srv(&self) -> Result { match self { ProjectWorkspace::Cargo { sysroot: Ok(sysroot), .. } - | ProjectWorkspace::Json { sysroot: Ok(sysroot), .. } => { + | ProjectWorkspace::Json { sysroot: Ok(sysroot), .. } + | ProjectWorkspace::DetachedFiles { sysroot: Ok(sysroot), .. } => { let standalone_server_name = format!("rust-analyzer-proc-macro-srv{}", std::env::consts::EXE_SUFFIX); ["libexec", "lib"] .into_iter() .map(|segment| sysroot.root().join(segment).join(&standalone_server_name)) .find(|server_path| std::fs::metadata(server_path).is_ok()) + .ok_or_else(|| { + anyhow::anyhow!( + "cannot find proc-macro server in sysroot `{}`", + sysroot.root().display() + ) + }) } - _ => None, + ProjectWorkspace::DetachedFiles { .. } => { + Err(anyhow::anyhow!("cannot find proc-macro server, no sysroot was found")) + } + ProjectWorkspace::Cargo { cargo, .. } => Err(anyhow::anyhow!( + "cannot find proc-macro-srv, the workspace `{}` is missing a sysroot", + cargo.workspace_root().display() + )), + ProjectWorkspace::Json { project, .. } => Err(anyhow::anyhow!( + "cannot find proc-macro-srv, the workspace `{}` is missing a sysroot", + project.path().display() + )), } } diff --git a/crates/rust-analyzer/Cargo.toml b/crates/rust-analyzer/Cargo.toml index e2a815fb7c..ea4f155bfd 100644 --- a/crates/rust-analyzer/Cargo.toml +++ b/crates/rust-analyzer/Cargo.toml @@ -67,7 +67,6 @@ ide-db.workspace = true ide-ssr.workspace = true ide.workspace = true proc-macro-api.workspace = true -proc-macro-srv-cli.workspace = true profile.workspace = true project-model.workspace = true stdx.workspace = true @@ -95,9 +94,7 @@ mbe.workspace = true [features] jemalloc = ["jemallocator", "profile/jemalloc"] force-always-assert = ["always-assert/force"] -sysroot-abi = ["proc-macro-srv-cli/sysroot-abi"] in-rust-tree = [ - "sysroot-abi", "ide/in-rust-tree", "syntax/in-rust-tree", ] diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 3506bed2c7..9b416d1ef0 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -76,9 +76,6 @@ fn try_main(flags: flags::RustAnalyzer) -> Result<()> { } with_extra_thread("LspServer", run_server)?; } - flags::RustAnalyzerCmd::ProcMacro(flags::ProcMacro) => { - with_extra_thread("MacroExpander", || proc_macro_srv_cli::run().map_err(Into::into))?; - } flags::RustAnalyzerCmd::Parse(cmd) => cmd.run()?, flags::RustAnalyzerCmd::Symbols(cmd) => cmd.run()?, flags::RustAnalyzerCmd::Highlight(cmd) => cmd.run()?, diff --git a/crates/rust-analyzer/src/cli/flags.rs b/crates/rust-analyzer/src/cli/flags.rs index b085a0a892..891d99c5ad 100644 --- a/crates/rust-analyzer/src/cli/flags.rs +++ b/crates/rust-analyzer/src/cli/flags.rs @@ -106,8 +106,6 @@ xflags::xflags! { optional --debug snippet: String } - cmd proc-macro {} - cmd lsif { required path: PathBuf } @@ -141,7 +139,6 @@ pub enum RustAnalyzerCmd { Diagnostics(Diagnostics), Ssr(Ssr), Search(Search), - ProcMacro(ProcMacro), Lsif(Lsif), Scip(Scip), } @@ -203,9 +200,6 @@ pub struct Search { pub debug: Option, } -#[derive(Debug)] -pub struct ProcMacro; - #[derive(Debug)] pub struct Lsif { pub path: PathBuf, diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index 33c1b36f22..625f1f2b19 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs @@ -1,8 +1,8 @@ //! Loads a Cargo project into a static instance of analysis, without support //! for incorporating changes. -use std::{convert::identity, path::Path, sync::Arc}; +use std::{path::Path, sync::Arc}; -use anyhow::Result; +use anyhow::{anyhow, Result}; use crossbeam_channel::{unbounded, Receiver}; use ide::{AnalysisHost, Change}; use ide_db::{ @@ -26,7 +26,7 @@ pub struct LoadCargoConfig { #[derive(Debug, Clone, PartialEq, Eq)] pub enum ProcMacroServerChoice { Sysroot, - Explicit(AbsPathBuf, Vec), + Explicit(AbsPathBuf), None, } @@ -71,14 +71,11 @@ pub fn load_workspace( let proc_macro_server = match &load_config.with_proc_macro_server { ProcMacroServerChoice::Sysroot => ws .find_sysroot_proc_macro_srv() - .ok_or_else(|| "failed to find sysroot proc-macro server".to_owned()) - .and_then(|it| { - ProcMacroServer::spawn(it, identity::<&[&str]>(&[])).map_err(|e| e.to_string()) - }), - ProcMacroServerChoice::Explicit(path, args) => { - ProcMacroServer::spawn(path.clone(), args).map_err(|e| e.to_string()) + .and_then(|it| ProcMacroServer::spawn(it).map_err(Into::into)), + ProcMacroServerChoice::Explicit(path) => { + ProcMacroServer::spawn(path.clone()).map_err(Into::into) } - ProcMacroServerChoice::None => Err("proc macro server disabled".to_owned()), + ProcMacroServerChoice::None => Err(anyhow!("proc macro server disabled")), }; let (crate_graph, proc_macros) = ws.to_crate_graph( @@ -93,7 +90,7 @@ pub fn load_workspace( let proc_macros = { let proc_macro_server = match &proc_macro_server { Ok(it) => Ok(it), - Err(e) => Err(e.as_str()), + Err(e) => Err(e.to_string()), }; proc_macros .into_iter() @@ -102,7 +99,11 @@ pub fn load_workspace( crate_id, path.map_or_else( |_| Err("proc macro crate is missing dylib".to_owned()), - |(_, path)| load_proc_macro(proc_macro_server, &path, &[]), + |(_, path)| { + proc_macro_server.as_ref().map_err(Clone::clone).and_then( + |proc_macro_server| load_proc_macro(proc_macro_server, &path, &[]), + ) + }, ), ) }) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 37a8f35363..89ca8e6356 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -437,8 +437,7 @@ config_data! { /// /// This config takes a map of crate names with the exported proc-macro names to ignore as values. procMacro_ignored: FxHashMap, Box<[Box]>> = "{}", - /// Internal config, path to proc-macro server executable (typically, - /// this is rust-analyzer itself, but we override this in tests). + /// Internal config, path to proc-macro server executable. procMacro_server: Option = "null", /// Exclude imports from find-all-references. @@ -1102,17 +1101,13 @@ impl Config { self.data.lru_query_capacities.is_empty().not().then(|| &self.data.lru_query_capacities) } - pub fn proc_macro_srv(&self) -> Option<(AbsPathBuf, /* is path explicitly set */ bool)> { - if !self.data.procMacro_enable { - return None; - } - Some(match &self.data.procMacro_server { - Some(it) => ( - AbsPathBuf::try_from(it.clone()).unwrap_or_else(|path| self.root_path.join(path)), - true, - ), - None => (AbsPathBuf::assert(std::env::current_exe().ok()?), false), - }) + pub fn proc_macro_srv(&self) -> Option { + self.data + .procMacro_server + .clone() + .map(AbsPathBuf::try_from)? + .ok() + .map(|path| self.root_path.join(path)) } pub fn dummy_replacements(&self) -> &FxHashMap, Box<[Box]>> { diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index cd449bad16..07778d2b59 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -63,7 +63,7 @@ pub(crate) struct GlobalState { pub(crate) source_root_config: SourceRootConfig, pub(crate) proc_macro_changed: bool, - pub(crate) proc_macro_clients: Arc<[Result]>, + pub(crate) proc_macro_clients: Arc<[anyhow::Result]>, pub(crate) flycheck: Arc<[FlycheckHandle]>, pub(crate) flycheck_sender: Sender, diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index ef319d9b77..4d840e11df 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -283,8 +283,8 @@ impl GlobalState { let mut res = FxHashMap::default(); let chain = proc_macro_clients .iter() - .map(|res| res.as_ref().map_err(|e| &**e)) - .chain(iter::repeat_with(|| Err("Proc macros servers are not running"))); + .map(|res| res.as_ref().map_err(|e| e.to_string())) + .chain(iter::repeat_with(|| Err("Proc macros servers are not running".into()))); for (client, paths) in chain.zip(paths) { res.extend(paths.into_iter().map(move |(crate_id, res)| { ( @@ -293,16 +293,18 @@ impl GlobalState { |_| Err("proc macro crate is missing dylib".to_owned()), |(crate_name, path)| { progress(path.display().to_string()); - load_proc_macro( - client, - &path, - crate_name - .as_deref() - .and_then(|crate_name| { - dummy_replacements.get(crate_name).map(|v| &**v) - }) - .unwrap_or_default(), - ) + client.as_ref().map_err(Clone::clone).and_then(|client| { + load_proc_macro( + client, + &path, + crate_name + .as_deref() + .and_then(|crate_name| { + dummy_replacements.get(crate_name).map(|v| &**v) + }) + .unwrap_or_default(), + ) + }) }, ), ) @@ -410,39 +412,25 @@ impl GlobalState { let project_folders = ProjectFolders::new(&self.workspaces, &files_config.exclude); if self.proc_macro_clients.is_empty() || !same_workspaces { - if let Some((path, path_manually_set)) = self.config.proc_macro_srv() { + if self.config.expand_proc_macros() { tracing::info!("Spawning proc-macro servers"); + self.proc_macro_clients = self .workspaces .iter() .map(|ws| { - let path = if path_manually_set { - tracing::debug!( - "Pro-macro server path explicitly set: {}", - path.display() - ); - path.clone() - } else { - match ws.find_sysroot_proc_macro_srv() { - Some(server_path) => server_path, - None => path.clone(), - } - }; - let args: &[_] = if path.file_stem() == Some("rust-analyzer".as_ref()) { - &["proc-macro"] - } else { - &[] + let path = match self.config.proc_macro_srv() { + Some(path) => path, + None => ws.find_sysroot_proc_macro_srv()?, }; - tracing::info!(?args, "Using proc-macro server at {}", path.display(),); - ProcMacroServer::spawn(path.clone(), args).map_err(|err| { - let error = format!( + tracing::info!("Using proc-macro server at {}", path.display(),); + ProcMacroServer::spawn(path.clone()).map_err(|err| { + anyhow::anyhow!( "Failed to run proc-macro server from path {}, error: {:?}", path.display(), err - ); - tracing::error!(error); - error + ) }) }) .collect() @@ -740,11 +728,10 @@ 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( - server: Result<&ProcMacroServer, &str>, + server: &ProcMacroServer, path: &AbsPath, dummy_replace: &[Box], ) -> ProcMacroLoadResult { - let server = server.map_err(ToOwned::to_owned)?; let res: Result, String> = (|| { let dylib = MacroDylib::new(path.to_path_buf()); let vec = server.load_dylib(dylib).map_err(|e| format!("{e}"))?; diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 0c3432e903..e9b3d45a37 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -679,8 +679,7 @@ This config takes a map of crate names with the exported proc-macro names to ign [[rust-analyzer.procMacro.server]]rust-analyzer.procMacro.server (default: `null`):: + -- -Internal config, path to proc-macro server executable (typically, -this is rust-analyzer itself, but we override this in tests). +Internal config, path to proc-macro server executable. -- [[rust-analyzer.references.excludeImports]]rust-analyzer.references.excludeImports (default: `false`):: + diff --git a/editors/code/package.json b/editors/code/package.json index f6ad2f7908..c164595aff 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -1305,7 +1305,7 @@ "type": "object" }, "rust-analyzer.procMacro.server": { - "markdownDescription": "Internal config, path to proc-macro server executable (typically,\nthis is rust-analyzer itself, but we override this in tests).", + "markdownDescription": "Internal config, path to proc-macro server executable.", "default": null, "type": [ "null",