Merge pull request #18792 from Veykril/push-wonkvzozmmwz

Decouple proc-macro server protocol from the server implementation

Author: Veykril

src/tools/rust-analyzer/Cargo.lock

@@ -1375,7 +1375,6 @@ dependencies = [
  "memmap2",
  "object 0.33.0",
  "paths",
- "proc-macro-api",
  "proc-macro-test",
  "ra-ap-rustc_lexer",
  "span",

src/tools/rust-analyzer/crates/load-cargo/src/lib.rs

@@ -14,7 +14,7 @@ use ide_db::{
     prime_caches, ChangeWithProcMacros, FxHashMap, RootDatabase,
 };
 use itertools::Itertools;
-use proc_macro_api::{MacroDylib, ProcMacroServer};
+use proc_macro_api::{MacroDylib, ProcMacroClient};
 use project_model::{CargoConfig, PackageRoot, ProjectManifest, ProjectWorkspace};
 use span::Span;
 use vfs::{
@@ -42,7 +42,7 @@ pub fn load_workspace_at(
     cargo_config: &CargoConfig,
     load_config: &LoadCargoConfig,
     progress: &dyn Fn(String),
-) -> anyhow::Result<(RootDatabase, vfs::Vfs, Option<ProcMacroServer>)> {
+) -> anyhow::Result<(RootDatabase, vfs::Vfs, Option<ProcMacroClient>)> {
     let root = AbsPathBuf::assert_utf8(std::env::current_dir()?.join(root));
     let root = ProjectManifest::discover_single(&root)?;
     let mut workspace = ProjectWorkspace::load(root, cargo_config, progress)?;
@@ -59,7 +59,7 @@ pub fn load_workspace(
     ws: ProjectWorkspace,
     extra_env: &FxHashMap<String, String>,
     load_config: &LoadCargoConfig,
-) -> anyhow::Result<(RootDatabase, vfs::Vfs, Option<ProcMacroServer>)> {
+) -> anyhow::Result<(RootDatabase, vfs::Vfs, Option<ProcMacroClient>)> {
     let (sender, receiver) = unbounded();
     let mut vfs = vfs::Vfs::default();
     let mut loader = {
@@ -71,18 +71,18 @@ pub fn load_workspace(
     let proc_macro_server = match &load_config.with_proc_macro_server {
         ProcMacroServerChoice::Sysroot => ws
             .find_sysroot_proc_macro_srv()
-            .and_then(|it| ProcMacroServer::spawn(&it, extra_env).map_err(Into::into))
+            .and_then(|it| ProcMacroClient::spawn(&it, extra_env).map_err(Into::into))
             .map_err(|e| (e, true)),
         ProcMacroServerChoice::Explicit(path) => {
-            ProcMacroServer::spawn(path, extra_env).map_err(Into::into).map_err(|e| (e, true))
+            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))
         }
     };
     match &proc_macro_server {
         Ok(server) => {
-            tracing::info!(path=%server.path(), "Proc-macro server started")
+            tracing::info!(path=%server.server_path(), "Proc-macro server started")
         }
         Err((e, _)) => {
             tracing::info!(%e, "Failed to start proc-macro server")
@@ -362,7 +362,7 @@ impl SourceRootConfig {
 
 /// Load the proc-macros for the given lib path, disabling all expanders whose names are in `ignored_macros`.
 pub fn load_proc_macro(
-    server: &ProcMacroServer,
+    server: &ProcMacroClient,
     path: &AbsPath,
     ignored_macros: &[Box<str>],
 ) -> ProcMacroLoadResult {

src/tools/rust-analyzer/crates/proc-macro-api/src/json.rs renamed to src/tools/rust-analyzer/crates/proc-macro-api/src/legacy_protocol/json.rs

@@ -9,10 +9,10 @@ use serde_derive::{Deserialize, Serialize};
 
 use crate::ProcMacroKind;
 
-pub use crate::msg::flat::{
+pub use self::flat::{
     deserialize_span_data_index_map, serialize_span_data_index_map, FlatTree, SpanDataIndexMap,
-    TokenId,
 };
+pub use span::TokenId;
 
 // The versions of the server protocol
 pub const NO_VERSION_CHECK_VERSION: u32 = 0;

src/tools/rust-analyzer/crates/proc-macro-api/src/msg.rs renamed to src/tools/rust-analyzer/crates/proc-macro-api/src/legacy_protocol/msg.rs

@@ -40,9 +40,11 @@ use std::collections::VecDeque;
 use intern::Symbol;
 use rustc_hash::FxHashMap;
 use serde_derive::{Deserialize, Serialize};
-use span::{EditionedFileId, ErasedFileAstId, Span, SpanAnchor, SyntaxContextId, TextRange};
+use span::{
+    EditionedFileId, ErasedFileAstId, Span, SpanAnchor, SyntaxContextId, TextRange, TokenId,
+};
 
-use crate::msg::{ENCODE_CLOSE_SPAN_VERSION, EXTENDED_LEAF_DATA};
+use crate::legacy_protocol::msg::{ENCODE_CLOSE_SPAN_VERSION, EXTENDED_LEAF_DATA};
 
 pub type SpanDataIndexMap =
     indexmap::IndexSet<Span, std::hash::BuildHasherDefault<rustc_hash::FxHasher>>;
@@ -78,15 +80,6 @@ pub fn deserialize_span_data_index_map(map: &[u32]) -> SpanDataIndexMap {
         .collect()
 }
 
-#[derive(Clone, Copy, PartialEq, Eq, Hash)]
-pub struct TokenId(pub u32);
-
-impl std::fmt::Debug for TokenId {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.0.fmt(f)
-    }
-}
-
 #[derive(Serialize, Deserialize, Debug)]
 pub struct FlatTree {
     subtree: Vec<u32>,

src/tools/rust-analyzer/crates/proc-macro-api/src/msg/flat.rs renamed to src/tools/rust-analyzer/crates/proc-macro-api/src/legacy_protocol/msg/flat.rs

@@ -5,26 +5,26 @@
 //! is used to provide basic infrastructure for communication between two
 //! processes: Client (RA itself), Server (the external program)
 
-pub mod json;
-pub mod msg;
+pub mod legacy_protocol {
+    pub mod json;
+    pub mod msg;
+}
 mod process;
 
 use paths::{AbsPath, AbsPathBuf};
 use span::Span;
 use std::{fmt, io, sync::Arc};
 
-use serde::{Deserialize, Serialize};
-
 use crate::{
-    msg::{
+    legacy_protocol::msg::{
         deserialize_span_data_index_map, flat::serialize_span_data_index_map, ExpandMacro,
-        ExpnGlobals, FlatTree, PanicMessage, SpanDataIndexMap, HAS_GLOBAL_SPANS,
-        RUST_ANALYZER_SPAN_SUPPORT,
+        ExpandMacroData, ExpnGlobals, FlatTree, PanicMessage, Request, Response, SpanDataIndexMap,
+        HAS_GLOBAL_SPANS, RUST_ANALYZER_SPAN_SUPPORT,
     },
-    process::ProcMacroProcessSrv,
+    process::ProcMacroServerProcess,
 };
 
-#[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
+#[derive(Copy, Clone, Eq, PartialEq, Debug, serde_derive::Serialize, serde_derive::Deserialize)]
 pub enum ProcMacroKind {
     CustomDerive,
     Attr,
@@ -37,12 +37,12 @@ pub enum ProcMacroKind {
 /// A handle to an external process which load dylibs with macros (.so or .dll)
 /// and runs actual macro expansion functions.
 #[derive(Debug)]
-pub struct ProcMacroServer {
+pub struct ProcMacroClient {
     /// Currently, the proc macro process expands all procedural macros sequentially.
     ///
     /// That means that concurrent salsa requests may block each other when expanding proc macros,
     /// which is unfortunate, but simple and good enough for the time being.
-    process: Arc<ProcMacroProcessSrv>,
+    process: Arc<ProcMacroServerProcess>,
     path: AbsPathBuf,
 }
 
@@ -56,13 +56,13 @@ impl MacroDylib {
     }
 }
 
-/// A handle to a specific macro (a `#[proc_macro]` annotated function).
+/// A handle to a specific proc-macro (a `#[proc_macro]` annotated function).
 ///
-/// It exists within a context of a specific [`ProcMacroProcess`] -- currently
-/// we share a single expander process for all macros.
+/// It exists within the context of a specific proc-macro server -- currently
+/// we share a single expander process for all macros within a workspace.
 #[derive(Debug, Clone)]
 pub struct ProcMacro {
-    process: Arc<ProcMacroProcessSrv>,
+    process: Arc<ProcMacroServerProcess>,
     dylib_path: Arc<AbsPathBuf>,
     name: Box<str>,
     kind: ProcMacroKind,
@@ -95,21 +95,22 @@ impl fmt::Display for ServerError {
     }
 }
 
-impl ProcMacroServer {
+impl ProcMacroClient {
     /// Spawns an external process as the proc macro server and returns a client connected to it.
     pub fn spawn(
         process_path: &AbsPath,
         env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>
             + Clone,
-    ) -> io::Result<ProcMacroServer> {
-        let process = ProcMacroProcessSrv::run(process_path, env)?;
-        Ok(ProcMacroServer { process: Arc::new(process), path: process_path.to_owned() })
+    ) -> io::Result<ProcMacroClient> {
+        let process = ProcMacroServerProcess::run(process_path, env)?;
+        Ok(ProcMacroClient { process: Arc::new(process), path: process_path.to_owned() })
     }
 
-    pub fn path(&self) -> &AbsPath {
+    pub fn server_path(&self) -> &AbsPath {
         &self.path
     }
 
+    /// Loads a proc-macro dylib into the server process returning a list of `ProcMacro`s loaded.
     pub fn load_dylib(&self, dylib: MacroDylib) -> Result<Vec<ProcMacro>, ServerError> {
         let _p = tracing::info_span!("ProcMacroServer::load_dylib").entered();
         let macros = self.process.find_proc_macros(&dylib.path)?;
@@ -160,7 +161,7 @@ impl ProcMacro {
         let call_site = span_data_table.insert_full(call_site).0;
         let mixed_site = span_data_table.insert_full(mixed_site).0;
         let task = ExpandMacro {
-            data: msg::ExpandMacroData {
+            data: ExpandMacroData {
                 macro_body: FlatTree::new(subtree, version, &mut span_data_table),
                 macro_name: self.name.to_string(),
                 attributes: attr
@@ -182,13 +183,13 @@ impl ProcMacro {
             current_dir,
         };
 
-        let response = self.process.send_task(msg::Request::ExpandMacro(Box::new(task)))?;
+        let response = self.process.send_task(Request::ExpandMacro(Box::new(task)))?;
 
         match response {
-            msg::Response::ExpandMacro(it) => {
+            Response::ExpandMacro(it) => {
                 Ok(it.map(|tree| FlatTree::to_subtree_resolved(tree, version, &span_data_table)))
             }
-            msg::Response::ExpandMacroExtended(it) => Ok(it.map(|resp| {
+            Response::ExpandMacroExtended(it) => Ok(it.map(|resp| {
                 FlatTree::to_subtree_resolved(
                     resp.tree,
                     version,

src/tools/rust-analyzer/crates/proc-macro-api/src/lib.rs

@@ -11,13 +11,18 @@ use paths::AbsPath;
 use stdx::JodChild;
 
 use crate::{
-    json::{read_json, write_json},
-    msg::{Message, Request, Response, SpanMode, CURRENT_API_VERSION, RUST_ANALYZER_SPAN_SUPPORT},
+    legacy_protocol::{
+        json::{read_json, write_json},
+        msg::{
+            Message, Request, Response, ServerConfig, SpanMode, CURRENT_API_VERSION,
+            RUST_ANALYZER_SPAN_SUPPORT,
+        },
+    },
     ProcMacroKind, ServerError,
 };
 
 #[derive(Debug)]
-pub(crate) struct ProcMacroProcessSrv {
+pub(crate) struct ProcMacroServerProcess {
     /// The state of the proc-macro server process, the protocol is currently strictly sequential
     /// hence the lock on the state.
     state: Mutex<ProcessSrvState>,
@@ -34,24 +39,24 @@ struct ProcessSrvState {
     stdout: BufReader<ChildStdout>,
 }
 
-impl ProcMacroProcessSrv {
+impl ProcMacroServerProcess {
     pub(crate) fn run(
         process_path: &AbsPath,
         env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>
             + Clone,
-    ) -> io::Result<ProcMacroProcessSrv> {
-        let create_srv = |null_stderr| {
-            let mut process = Process::run(process_path, env.clone(), null_stderr)?;
+    ) -> io::Result<ProcMacroServerProcess> {
+        let create_srv = || {
+            let mut process = Process::run(process_path, env.clone())?;
             let (stdin, stdout) = process.stdio().expect("couldn't access child stdio");
 
-            io::Result::Ok(ProcMacroProcessSrv {
+            io::Result::Ok(ProcMacroServerProcess {
                 state: Mutex::new(ProcessSrvState { process, stdin, stdout }),
                 version: 0,
                 mode: SpanMode::Id,
                 exited: OnceLock::new(),
             })
         };
-        let mut srv = create_srv(true)?;
+        let mut srv = create_srv()?;
         tracing::info!("sending proc-macro server version check");
         match srv.version_check() {
             Ok(v) if v > CURRENT_API_VERSION => Err(io::Error::new(
@@ -62,7 +67,6 @@ impl ProcMacroProcessSrv {
             )),
             Ok(v) => {
                 tracing::info!("Proc-macro server version: {v}");
-                srv = create_srv(false)?;
                 srv.version = v;
                 if srv.version >= RUST_ANALYZER_SPAN_SUPPORT {
                     if let Ok(mode) = srv.enable_rust_analyzer_spans() {
@@ -73,8 +77,10 @@ impl ProcMacroProcessSrv {
                 Ok(srv)
             }
             Err(e) => {
-                tracing::info!(%e, "proc-macro version check failed, restarting and assuming version 0");
-                create_srv(false)
+                tracing::info!(%e, "proc-macro version check failed");
+                Err(
+                    io::Error::new(io::ErrorKind::Other, format!("proc-macro server version check failed: {e}")),
+                )
             }
         }
     }
@@ -98,13 +104,11 @@ impl ProcMacroProcessSrv {
     }
 
     fn enable_rust_analyzer_spans(&self) -> Result<SpanMode, ServerError> {
-        let request = Request::SetConfig(crate::msg::ServerConfig {
-            span_mode: crate::msg::SpanMode::RustAnalyzer,
-        });
+        let request = Request::SetConfig(ServerConfig { span_mode: SpanMode::RustAnalyzer });
         let response = self.send_task(request)?;
 
         match response {
-            Response::SetConfig(crate::msg::ServerConfig { span_mode }) => Ok(span_mode),
+            Response::SetConfig(ServerConfig { span_mode }) => Ok(span_mode),
             _ => Err(ServerError { message: "unexpected response".to_owned(), io: None }),
         }
     }
@@ -182,9 +186,8 @@ impl Process {
     fn run(
         path: &AbsPath,
         env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>,
-        null_stderr: bool,
     ) -> io::Result<Process> {
-        let child = JodChild(mk_child(path, env, null_stderr)?);
+        let child = JodChild(mk_child(path, env)?);
         Ok(Process { child })
     }
 
@@ -200,15 +203,14 @@ impl Process {
 fn mk_child(
     path: &AbsPath,
     env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>,
-    null_stderr: bool,
 ) -> io::Result<Child> {
     #[allow(clippy::disallowed_methods)]
     let mut cmd = Command::new(path);
     cmd.envs(env)
         .env("RUST_ANALYZER_INTERNALS_DO_NOT_USE", "this is unstable")
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
-        .stderr(if null_stderr { Stdio::null() } else { Stdio::inherit() });
+        .stderr(Stdio::inherit());
     if cfg!(windows) {
         let mut path_var = std::ffi::OsString::new();
         path_var.push(path.parent().unwrap().parent().unwrap());