Reorganize proc-macro-srv

5 files changed, 131 insertions, 148 deletions

diff --git a/src/tools/rust-analyzer/crates/proc-macro-srv-cli/src/main_loop.rs b/src/tools/rust-analyzer/crates/proc-macro-srv-cli/src/main_loop.rs
index 46be8a21004..703bc965db2 100644
--- a/src/tools/rust-analyzer/crates/proc-macro-srv-cli/src/main_loop.rs
+++ b/src/tools/rust-analyzer/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,
+                            &macro_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,
+                            &macro_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/src/tools/rust-analyzer/crates/proc-macro-srv/src/dylib.rs b/src/tools/rust-analyzer/crates/proc-macro-srv/src/dylib.rs
index 095e9fa2e98..c8513a10675 100644
--- a/src/tools/rust-analyzer/crates/proc-macro-srv/src/dylib.rs
+++ b/src/tools/rust-analyzer/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<Library, libloading::Error> {
-    // 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<Library, libloading::Error> {
-    // 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<Expander, LoadProcMacroDylibError> {
+        // 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<S: ProcMacroSrvSpan>(
+        &self,
+        macro_name: &str,
+        macro_body: TopSubtree<S>,
+        attributes: Option<TopSubtree<S>>,
+        def_site: S,
+        call_site: S,
+        mixed_site: S,
+    ) -> Result<TopSubtree<S>, PanicMessage>
+    where
+        <S::Server as bridge::server::Types>::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<Item = (&str, ProcMacroKind)> {
+        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<Expander, LoadProcMacroDylibError> {
-        // 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<S: ProcMacroSrvSpan>(
-        &self,
-        macro_name: &str,
-        macro_body: TopSubtree<S>,
-        attributes: Option<TopSubtree<S>>,
-        def_site: S,
-        call_site: S,
-        mixed_site: S,
-    ) -> Result<TopSubtree<S>, String>
-    where
-        <S::Server as bridge::server::Types>::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<Box<dyn std::error::Error + Send + Sync>>) -> io::Error {
     io::Error::new(io::ErrorKind::InvalidData, e)
 }
@@ -214,15 +163,6 @@ fn find_registrar_symbol(obj: &object::File<'_>) -> object::Result<Option<String
         }))
 }
 
-struct RemoveFileOnDrop(Utf8PathBuf);
-impl Drop for RemoveFileOnDrop {
-    fn drop(&mut self) {
-        #[cfg(windows)]
-        std::fs::remove_file(&self.0).unwrap();
-        _ = self.0;
-    }
-}
-
 /// Copy the dylib to temp directory to prevent locking in Windows
 #[cfg(windows)]
 fn ensure_file_with_lock_free_access(
@@ -259,3 +199,54 @@ fn ensure_file_with_lock_free_access(
 ) -> io::Result<Utf8PathBuf> {
     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<Library, libloading::Error> {
+    // 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<Library, libloading::Error> {
+    // 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/src/tools/rust-analyzer/crates/proc-macro-srv/src/proc_macros.rs b/src/tools/rust-analyzer/crates/proc-macro-srv/src/dylib/proc_macros.rs
index 18532706c4a..9b5721e370a 100644
--- a/src/tools/rust-analyzer/crates/proc-macro-srv/src/proc_macros.rs
+++ b/src/tools/rust-analyzer/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<Item = (&str, ProcMacroKind)> {
+        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/src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs b/src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs
index 29fe5aed2b1..cb97882c585 100644
--- a/src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs
+++ b/src/tools/rust-analyzer/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<Utf8Path>,
         env: &[(String, String)],
         current_dir: Option<impl AsRef<Path>>,
-        macro_name: String,
+        macro_name: &str,
         macro_body: tt::TopSubtree<S>,
         attribute: Option<tt::TopSubtree<S>>,
         def_site: S,
         call_site: S,
         mixed_site: S,
-    ) -> Result<Vec<tt::TokenTree<S>>, String> {
+    ) -> Result<Vec<tt::TokenTree<S>>, 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(
-                            &macro_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<Vec<(String, ProcMacroKind)>, 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<Arc<dylib::Expander>, String> {
@@ -186,6 +181,8 @@ impl ProcMacroSrvSpan for Span {
         }
     }
 }
+
+#[derive(Debug, Clone)]
 pub struct PanicMessage {
     message: Option<String>,
 }
@@ -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/src/tools/rust-analyzer/crates/proc-macro-srv/src/server_impl.rs b/src/tools/rust-analyzer/crates/proc-macro-srv/src/server_impl.rs
index 662f6257642..32ad32731ba 100644
--- a/src/tools/rust-analyzer/crates/proc-macro-srv/src/server_impl.rs
+++ b/src/tools/rust-analyzer/crates/proc-macro-srv/src/server_impl.rs
@@ -209,7 +209,7 @@ pub(super) fn from_token_tree<Span: Copy>(
                 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..]);
             }