From 218e00d0bbcfe5bc3d535fbe70b098520e1b1c37 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 31 Jul 2025 09:53:26 +0200 Subject: [PATCH] Reorganize proc-macro-srv --- crates/proc-macro-srv-cli/src/main_loop.rs | 10 +- crates/proc-macro-srv/src/dylib.rs | 203 +++++++++--------- .../src/{ => dylib}/proc_macros.rs | 23 +- crates/proc-macro-srv/src/lib.rs | 41 ++-- crates/proc-macro-srv/src/server_impl.rs | 2 +- 5 files changed, 131 insertions(+), 148 deletions(-) rename crates/proc-macro-srv/src/{ => dylib}/proc_macros.rs (81%) diff --git a/crates/proc-macro-srv-cli/src/main_loop.rs b/crates/proc-macro-srv-cli/src/main_loop.rs index 46be8a2100..703bc965db 100644 --- a/crates/proc-macro-srv-cli/src/main_loop.rs +++ b/crates/proc-macro-srv-cli/src/main_loop.rs @@ -24,13 +24,13 @@ impl SpanTransformer for SpanTrans { _: &mut Self::Table, span: Self::Span, ) -> proc_macro_api::legacy_protocol::SpanId { - proc_macro_api::legacy_protocol::SpanId(span.0 as u32) + proc_macro_api::legacy_protocol::SpanId(span.0) } fn span_for_token_id( _: &Self::Table, id: proc_macro_api::legacy_protocol::SpanId, ) -> Self::Span { - SpanId(id.0 as u32) + SpanId(id.0) } } @@ -99,7 +99,7 @@ fn run_json() -> io::Result<()> { lib, &env, current_dir, - macro_name, + ¯o_name, macro_body, attributes, def_site, @@ -112,6 +112,7 @@ fn run_json() -> io::Result<()> { CURRENT_API_VERSION, ) }) + .map_err(|e| e.into_string().unwrap_or_default()) .map_err(msg::PanicMessage) }), SpanMode::RustAnalyzer => msg::Response::ExpandMacroExtended({ @@ -130,7 +131,7 @@ fn run_json() -> io::Result<()> { lib, &env, current_dir, - macro_name, + ¯o_name, macro_body, attributes, def_site, @@ -151,6 +152,7 @@ fn run_json() -> io::Result<()> { tree, span_data_table, }) + .map_err(|e| e.into_string().unwrap_or_default()) .map_err(msg::PanicMessage) }), } diff --git a/crates/proc-macro-srv/src/dylib.rs b/crates/proc-macro-srv/src/dylib.rs index 095e9fa2e9..c8513a1067 100644 --- a/crates/proc-macro-srv/src/dylib.rs +++ b/crates/proc-macro-srv/src/dylib.rs @@ -1,5 +1,6 @@ //! Handles dynamic library loading for proc macro +mod proc_macros; mod version; use proc_macro::bridge; @@ -10,57 +11,56 @@ use libloading::Library; use object::Object; use paths::{Utf8Path, Utf8PathBuf}; -use crate::{ProcMacroKind, ProcMacroSrvSpan, proc_macros::ProcMacros, server_impl::TopSubtree}; +use crate::{ + PanicMessage, ProcMacroKind, ProcMacroSrvSpan, dylib::proc_macros::ProcMacros, + server_impl::TopSubtree, +}; -/// Loads dynamic library in platform dependent manner. -/// -/// For unix, you have to use RTLD_DEEPBIND flag to escape problems described -/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample) -/// and [here](https://github.com/rust-lang/rust/issues/60593). -/// -/// Usage of RTLD_DEEPBIND -/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample/issues/1) -/// -/// It seems that on Windows that behaviour is default, so we do nothing in that case. -/// -/// # Safety -/// -/// The caller is responsible for ensuring that the path is valid proc-macro library -#[cfg(windows)] -unsafe fn load_library(file: &Utf8Path) -> Result { - // SAFETY: The caller is responsible for ensuring that the path is valid proc-macro library - unsafe { Library::new(file) } +pub(crate) struct Expander { + inner: ProcMacroLibrary, + modified_time: SystemTime, } -/// Loads dynamic library in platform dependent manner. -/// -/// For unix, you have to use RTLD_DEEPBIND flag to escape problems described -/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample) -/// and [here](https://github.com/rust-lang/rust/issues/60593). -/// -/// Usage of RTLD_DEEPBIND -/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample/issues/1) -/// -/// It seems that on Windows that behaviour is default, so we do nothing in that case. -/// -/// # Safety -/// -/// The caller is responsible for ensuring that the path is valid proc-macro library -#[cfg(unix)] -unsafe fn load_library(file: &Utf8Path) -> Result { - // not defined by POSIX, different values on mips vs other targets - #[cfg(target_env = "gnu")] - use libc::RTLD_DEEPBIND; - use libloading::os::unix::Library as UnixLibrary; - // defined by POSIX - use libloading::os::unix::RTLD_NOW; +impl Expander { + pub(crate) fn new( + temp_dir: &TempDir, + lib: &Utf8Path, + ) -> Result { + // Some libraries for dynamic loading require canonicalized path even when it is + // already absolute + let lib = lib.canonicalize_utf8()?; + let modified_time = fs::metadata(&lib).and_then(|it| it.modified())?; - // MUSL and bionic don't have it.. - #[cfg(not(target_env = "gnu"))] - const RTLD_DEEPBIND: std::os::raw::c_int = 0x0; + let path = ensure_file_with_lock_free_access(temp_dir, &lib)?; + let library = ProcMacroLibrary::open(path.as_ref())?; - // SAFETY: The caller is responsible for ensuring that the path is valid proc-macro library - unsafe { UnixLibrary::open(Some(file), RTLD_NOW | RTLD_DEEPBIND).map(|lib| lib.into()) } + Ok(Expander { inner: library, modified_time }) + } + + pub(crate) fn expand( + &self, + macro_name: &str, + macro_body: TopSubtree, + attributes: Option>, + def_site: S, + call_site: S, + mixed_site: S, + ) -> Result, PanicMessage> + where + ::TokenStream: Default, + { + self.inner + .proc_macros + .expand(macro_name, macro_body, attributes, def_site, call_site, mixed_site) + } + + pub(crate) fn list_macros(&self) -> impl Iterator { + self.inner.proc_macros.list_macros() + } + + pub(crate) fn modified_time(&self) -> SystemTime { + self.modified_time + } } #[derive(Debug)] @@ -134,57 +134,6 @@ impl ProcMacroLibrary { } } -// Drop order matters as we can't remove the dylib before the library is unloaded -pub(crate) struct Expander { - inner: ProcMacroLibrary, - _remove_on_drop: RemoveFileOnDrop, - modified_time: SystemTime, -} - -impl Expander { - pub(crate) fn new( - temp_dir: &TempDir, - lib: &Utf8Path, - ) -> Result { - // Some libraries for dynamic loading require canonicalized path even when it is - // already absolute - let lib = lib.canonicalize_utf8()?; - let modified_time = fs::metadata(&lib).and_then(|it| it.modified())?; - - let path = ensure_file_with_lock_free_access(temp_dir, &lib)?; - let library = ProcMacroLibrary::open(path.as_ref())?; - - Ok(Expander { inner: library, _remove_on_drop: RemoveFileOnDrop(path), modified_time }) - } - - pub(crate) fn expand( - &self, - macro_name: &str, - macro_body: TopSubtree, - attributes: Option>, - def_site: S, - call_site: S, - mixed_site: S, - ) -> Result, String> - where - ::TokenStream: Default, - { - let result = self - .inner - .proc_macros - .expand(macro_name, macro_body, attributes, def_site, call_site, mixed_site); - result.map_err(|e| e.into_string().unwrap_or_default()) - } - - pub(crate) fn list_macros(&self) -> Vec<(String, ProcMacroKind)> { - self.inner.proc_macros.list_macros() - } - - pub(crate) fn modified_time(&self) -> SystemTime { - self.modified_time - } -} - fn invalid_data_err(e: impl Into>) -> io::Error { io::Error::new(io::ErrorKind::InvalidData, e) } @@ -214,15 +163,6 @@ fn find_registrar_symbol(obj: &object::File<'_>) -> object::Result io::Result { Ok(path.to_owned()) } + +/// Loads dynamic library in platform dependent manner. +/// +/// For unix, you have to use RTLD_DEEPBIND flag to escape problems described +/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample) +/// and [here](https://github.com/rust-lang/rust/issues/60593). +/// +/// Usage of RTLD_DEEPBIND +/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample/issues/1) +/// +/// It seems that on Windows that behaviour is default, so we do nothing in that case. +/// +/// # Safety +/// +/// The caller is responsible for ensuring that the path is valid proc-macro library +#[cfg(windows)] +unsafe fn load_library(file: &Utf8Path) -> Result { + // SAFETY: The caller is responsible for ensuring that the path is valid proc-macro library + unsafe { Library::new(file) } +} + +/// Loads dynamic library in platform dependent manner. +/// +/// For unix, you have to use RTLD_DEEPBIND flag to escape problems described +/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample) +/// and [here](https://github.com/rust-lang/rust/issues/60593). +/// +/// Usage of RTLD_DEEPBIND +/// [here](https://github.com/fedochet/rust-proc-macro-panic-inside-panic-expample/issues/1) +/// +/// It seems that on Windows that behaviour is default, so we do nothing in that case. +/// +/// # Safety +/// +/// The caller is responsible for ensuring that the path is valid proc-macro library +#[cfg(unix)] +unsafe fn load_library(file: &Utf8Path) -> Result { + // not defined by POSIX, different values on mips vs other targets + #[cfg(target_env = "gnu")] + use libc::RTLD_DEEPBIND; + use libloading::os::unix::Library as UnixLibrary; + // defined by POSIX + use libloading::os::unix::RTLD_NOW; + + // MUSL and bionic don't have it.. + #[cfg(not(target_env = "gnu"))] + const RTLD_DEEPBIND: std::os::raw::c_int = 0x0; + + // SAFETY: The caller is responsible for ensuring that the path is valid proc-macro library + unsafe { UnixLibrary::open(Some(file), RTLD_NOW | RTLD_DEEPBIND).map(|lib| lib.into()) } +} diff --git a/crates/proc-macro-srv/src/proc_macros.rs b/crates/proc-macro-srv/src/dylib/proc_macros.rs similarity index 81% rename from crates/proc-macro-srv/src/proc_macros.rs rename to crates/proc-macro-srv/src/dylib/proc_macros.rs index 18532706c4..9b5721e370 100644 --- a/crates/proc-macro-srv/src/proc_macros.rs +++ b/crates/proc-macro-srv/src/dylib/proc_macros.rs @@ -75,20 +75,13 @@ impl ProcMacros { Err(bridge::PanicMessage::String(format!("proc-macro `{macro_name}` is missing")).into()) } - pub(crate) fn list_macros(&self) -> Vec<(String, ProcMacroKind)> { - self.0 - .iter() - .map(|proc_macro| match proc_macro { - bridge::client::ProcMacro::CustomDerive { trait_name, .. } => { - (trait_name.to_string(), ProcMacroKind::CustomDerive) - } - bridge::client::ProcMacro::Bang { name, .. } => { - (name.to_string(), ProcMacroKind::Bang) - } - bridge::client::ProcMacro::Attr { name, .. } => { - (name.to_string(), ProcMacroKind::Attr) - } - }) - .collect() + pub(crate) fn list_macros(&self) -> impl Iterator { + self.0.iter().map(|proc_macro| match *proc_macro { + bridge::client::ProcMacro::CustomDerive { trait_name, .. } => { + (trait_name, ProcMacroKind::CustomDerive) + } + bridge::client::ProcMacro::Bang { name, .. } => (name, ProcMacroKind::Bang), + bridge::client::ProcMacro::Attr { name, .. } => (name, ProcMacroKind::Attr), + }) } } diff --git a/crates/proc-macro-srv/src/lib.rs b/crates/proc-macro-srv/src/lib.rs index 29fe5aed2b..cb97882c58 100644 --- a/crates/proc-macro-srv/src/lib.rs +++ b/crates/proc-macro-srv/src/lib.rs @@ -27,7 +27,6 @@ extern crate ra_ap_rustc_lexer as rustc_lexer; extern crate rustc_lexer; mod dylib; -mod proc_macros; mod server_impl; use std::{ @@ -81,16 +80,17 @@ impl ProcMacroSrv<'_> { lib: impl AsRef, env: &[(String, String)], current_dir: Option>, - macro_name: String, + macro_name: &str, macro_body: tt::TopSubtree, attribute: Option>, def_site: S, call_site: S, mixed_site: S, - ) -> Result>, String> { + ) -> Result>, PanicMessage> { let snapped_env = self.env; - let expander = - self.expander(lib.as_ref()).map_err(|err| format!("failed to load macro: {err}"))?; + let expander = self.expander(lib.as_ref()).map_err(|err| PanicMessage { + message: Some(format!("failed to load macro: {err}")), + })?; let prev_env = EnvChange::apply(snapped_env, env, current_dir.as_ref().map(<_>::as_ref)); @@ -99,11 +99,11 @@ impl ProcMacroSrv<'_> { let result = thread::scope(|s| { let thread = thread::Builder::new() .stack_size(EXPANDER_STACK_SIZE) - .name(macro_name.clone()) + .name(macro_name.to_owned()) .spawn_scoped(s, move || { expander .expand( - ¯o_name, + macro_name, server_impl::TopSubtree(macro_body.0.into_vec()), attribute.map(|it| server_impl::TopSubtree(it.0.into_vec())), def_site, @@ -112,12 +112,7 @@ impl ProcMacroSrv<'_> { ) .map(|tt| tt.0) }); - let res = match thread { - Ok(handle) => handle.join(), - Err(e) => return Err(e.to_string()), - }; - - match res { + match thread.unwrap().join() { Ok(res) => res, Err(e) => std::panic::resume_unwind(e), } @@ -132,7 +127,7 @@ impl ProcMacroSrv<'_> { dylib_path: &Utf8Path, ) -> Result, String> { let expander = self.expander(dylib_path)?; - Ok(expander.list_macros()) + Ok(expander.list_macros().map(|(k, v)| (k.to_owned(), v)).collect()) } fn expander(&self, path: &Utf8Path) -> Result, String> { @@ -186,6 +181,8 @@ impl ProcMacroSrvSpan for Span { } } } + +#[derive(Debug, Clone)] pub struct PanicMessage { message: Option, } @@ -265,14 +262,14 @@ impl Drop for EnvChange<'_> { } } - if let Some(dir) = &self.prev_working_dir { - if let Err(err) = std::env::set_current_dir(dir) { - eprintln!( - "Failed to set the current working dir to {}. Error: {:?}", - dir.display(), - err - ) - } + if let Some(dir) = &self.prev_working_dir + && let Err(err) = std::env::set_current_dir(dir) + { + eprintln!( + "Failed to set the current working dir to {}. Error: {:?}", + dir.display(), + err + ) } } } diff --git a/crates/proc-macro-srv/src/server_impl.rs b/crates/proc-macro-srv/src/server_impl.rs index 662f625764..32ad32731b 100644 --- a/crates/proc-macro-srv/src/server_impl.rs +++ b/crates/proc-macro-srv/src/server_impl.rs @@ -209,7 +209,7 @@ pub(super) fn from_token_tree( token_trees.push(tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { spacing: tt::Spacing::Alone, span: literal.span, - char: '-' as char, + char: '-', }))); symbol = Symbol::intern(&symbol.as_str()[1..]); }