From d3019164dcbb46f8c369ed4efff79de5a42a95a8 Mon Sep 17 00:00:00 2001 From: veetaha Date: Mon, 20 Apr 2020 21:26:10 +0300 Subject: ra_proc_macro: cleanups here and there --- crates/ra_proc_macro/src/lib.rs | 17 ++--- crates/ra_proc_macro/src/msg.rs | 37 ++++----- crates/ra_proc_macro/src/process.rs | 27 +++---- crates/ra_proc_macro/src/rpc.rs | 16 ++-- crates/ra_proc_macro_srv/src/cli.rs | 58 ++++++--------- crates/ra_proc_macro_srv/src/dylib.rs | 107 ++++++++++++--------------- crates/ra_proc_macro_srv/src/lib.rs | 36 ++++----- crates/ra_proc_macro_srv/src/rustc_server.rs | 2 +- crates/ra_proc_macro_srv/src/tests/utils.rs | 2 +- crates/rust-analyzer/src/bin/main.rs | 4 +- crates/rust-analyzer/src/cli/load_cargo.rs | 2 +- crates/rust-analyzer/src/world.rs | 23 +++--- 12 files changed, 141 insertions(+), 190 deletions(-) (limited to 'crates') diff --git a/crates/ra_proc_macro/src/lib.rs b/crates/ra_proc_macro/src/lib.rs index b200fd126..004943b9e 100644 --- a/crates/ra_proc_macro/src/lib.rs +++ b/crates/ra_proc_macro/src/lib.rs @@ -2,7 +2,7 @@ //! //! We separate proc-macro expanding logic to an extern program to allow //! different implementations (e.g. wasm or dylib loading). And this crate -//! is used to provide basic infrastructure for communication between two +//! is used to provide basic infrastructure for communication between two //! processes: Client (RA itself), Server (the external program) mod rpc; @@ -13,6 +13,7 @@ use process::{ProcMacroProcessSrv, ProcMacroProcessThread}; use ra_tt::{SmolStr, Subtree}; use std::{ ffi::OsStr, + io, path::{Path, PathBuf}, sync::Arc, }; @@ -57,14 +58,10 @@ pub struct ProcMacroClient { } impl ProcMacroClient { - pub fn extern_process( - process_path: &Path, - args: I, - ) -> Result - where - I: IntoIterator, - S: AsRef, - { + pub fn extern_process( + process_path: PathBuf, + args: impl IntoIterator>, + ) -> io::Result { let (thread, process) = ProcMacroProcessSrv::run(process_path, args)?; Ok(ProcMacroClient { kind: ProcMacroClientKind::Process { process: Arc::new(process), thread }, @@ -84,7 +81,7 @@ impl ProcMacroClient { ProcMacroClientKind::Process { process, .. } => { let macros = match process.find_proc_macros(dylib_path) { Err(err) => { - eprintln!("Fail to find proc macro. Error: {:#?}", err); + eprintln!("Failed to find proc macros. Error: {:#?}", err); return vec![]; } Ok(macros) => macros, diff --git a/crates/ra_proc_macro/src/msg.rs b/crates/ra_proc_macro/src/msg.rs index aa95bcc8f..95d9b8804 100644 --- a/crates/ra_proc_macro/src/msg.rs +++ b/crates/ra_proc_macro/src/msg.rs @@ -1,4 +1,4 @@ -//! Defines messages for cross-process message based on `ndjson` wire protocol +//! Defines messages for cross-process message passing based on `ndjson` wire protocol use std::{ convert::TryFrom, @@ -31,7 +31,7 @@ macro_rules! impl_try_from_response { fn try_from(value: Response) -> Result { match value { Response::$tag(res) => Ok(res), - _ => Err("Fail to convert from response"), + _ => Err(concat!("Failed to convert response to ", stringify!($tag))), } } } @@ -53,18 +53,16 @@ pub enum ErrorCode { ExpansionError, } -pub trait Message: Sized + Serialize + DeserializeOwned { - fn read(r: &mut impl BufRead) -> io::Result> { - let text = match read_json(r)? { - None => return Ok(None), - Some(text) => text, - }; - let msg = serde_json::from_str(&text)?; - Ok(Some(msg)) +pub trait Message: Serialize + DeserializeOwned { + fn read(inp: &mut impl BufRead) -> io::Result> { + Ok(match read_json(inp)? { + None => None, + Some(text) => Some(serde_json::from_str(&text)?), + }) } - fn write(self, w: &mut impl Write) -> io::Result<()> { + fn write(self, out: &mut impl Write) -> io::Result<()> { let text = serde_json::to_string(&self)?; - write_json(w, &text) + write_json(out, &text) } } @@ -73,15 +71,12 @@ impl Message for Response {} fn read_json(inp: &mut impl BufRead) -> io::Result> { let mut buf = String::new(); - if inp.read_line(&mut buf)? == 0 { - return Ok(None); - } - // Remove ending '\n' - let buf = &buf[..buf.len() - 1]; - if buf.is_empty() { - return Ok(None); - } - Ok(Some(buf.to_string())) + inp.read_line(&mut buf)?; + buf.pop(); // Remove traling '\n' + Ok(match buf.len() { + 0 => None, + _ => Some(buf), + }) } fn write_json(out: &mut impl Write, msg: &str) -> io::Result<()> { diff --git a/crates/ra_proc_macro/src/process.rs b/crates/ra_proc_macro/src/process.rs index f851570bc..b1ebf46a1 100644 --- a/crates/ra_proc_macro/src/process.rs +++ b/crates/ra_proc_macro/src/process.rs @@ -45,24 +45,23 @@ impl Drop for Process { } impl Process { - fn run(process_path: &Path, args: I) -> Result - where - I: IntoIterator, - S: AsRef, - { - let child = Command::new(process_path.clone()) + fn run( + process_path: PathBuf, + args: impl IntoIterator>, + ) -> Result { + let child = Command::new(&process_path) .args(args) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::null()) .spawn()?; - Ok(Process { path: process_path.into(), child }) + Ok(Process { path: process_path, child }) } fn restart(&mut self) -> Result<(), io::Error> { let _ = self.child.kill(); - self.child = Command::new(self.path.clone()) + self.child = Command::new(&self.path) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -80,14 +79,10 @@ impl Process { } impl ProcMacroProcessSrv { - pub fn run( - process_path: &Path, - args: I, - ) -> Result<(ProcMacroProcessThread, ProcMacroProcessSrv), io::Error> - where - I: IntoIterator, - S: AsRef, - { + pub fn run( + process_path: PathBuf, + args: impl IntoIterator>, + ) -> io::Result<(ProcMacroProcessThread, ProcMacroProcessSrv)> { let process = Process::run(process_path, args)?; let (task_tx, task_rx) = bounded(0); diff --git a/crates/ra_proc_macro/src/rpc.rs b/crates/ra_proc_macro/src/rpc.rs index 66b3f55db..e97b90860 100644 --- a/crates/ra_proc_macro/src/rpc.rs +++ b/crates/ra_proc_macro/src/rpc.rs @@ -1,9 +1,9 @@ -//! Data struture serialization related stuffs for RPC +//! Data struture serialization related stuff for RPC //! -//! Define all necessary rpc serialization data structure, -//! which include ra_tt related data and some task messages. -//! Although adding Serialize and Deserialize trait to ra_tt directly seem to be much easier, -//! we deliberately duplicate the ra_tt struct with #[serde(with = "XXDef")] +//! Defines all necessary rpc serialization data structures, +//! which includes `ra_tt` related data and some task messages. +//! Although adding `Serialize` and `Deserialize` traits to `ra_tt` directly seems +//! to be much easier, we deliberately duplicate `ra_tt` structs with `#[serde(with = "XXDef")]` //! for separation of code responsibility. use ra_tt::{ @@ -34,15 +34,15 @@ pub struct ListMacrosResult { pub struct ExpansionTask { /// Argument of macro call. /// - /// In custom derive that would be a struct or enum; in attribute-like macro - underlying + /// In custom derive this will be a struct or enum; in attribute-like macro - underlying /// item; in function-like macro - the macro body. #[serde(with = "SubtreeDef")] pub macro_body: Subtree, - /// Names of macros to expand. + /// Names of macros to expand. // TODO: are they comma-separated? /// /// In custom derive those are names of derived traits (`Serialize`, `Getters`, etc.). In - /// attribute-like and functiona-like macros - single name of macro itself (`show_streams`). + /// attribute-like and function-like macros - single name of macro itself (`show_streams`). pub macro_name: String, /// Possible attributes for the attribute-like macros. diff --git a/crates/ra_proc_macro_srv/src/cli.rs b/crates/ra_proc_macro_srv/src/cli.rs index c771f2b38..5f1f3ba3c 100644 --- a/crates/ra_proc_macro_srv/src/cli.rs +++ b/crates/ra_proc_macro_srv/src/cli.rs @@ -2,55 +2,43 @@ use crate::{expand_task, list_macros}; use ra_proc_macro::msg::{self, Message}; - use std::io; -fn read_request() -> Result, io::Error> { - let stdin = io::stdin(); - let mut stdin = stdin.lock(); - msg::Request::read(&mut stdin) -} - -fn write_response(res: Result) -> Result<(), io::Error> { - let msg: msg::Response = match res { - Ok(res) => res, - Err(err) => msg::Response::Error(msg::ResponseError { - code: msg::ErrorCode::ExpansionError, - message: err, - }), - }; - - let stdout = io::stdout(); - let mut stdout = stdout.lock(); - msg.write(&mut stdout) -} - pub fn run() { loop { let req = match read_request() { Err(err) => { - eprintln!("Read message error on ra_proc_macro_srv: {}", err.to_string()); + eprintln!("Read message error on ra_proc_macro_srv: {}", err); continue; } Ok(None) => continue, Ok(Some(req)) => req, }; - match req { - msg::Request::ListMacro(task) => { - if let Err(err) = - write_response(list_macros(&task).map(|it| msg::Response::ListMacro(it))) - { - eprintln!("Write message error on list macro: {}", err); - } - } + let res = match req { + msg::Request::ListMacro(task) => Ok(msg::Response::ListMacro(list_macros(&task))), msg::Request::ExpansionMacro(task) => { - if let Err(err) = - write_response(expand_task(&task).map(|it| msg::Response::ExpansionMacro(it))) - { - eprintln!("Write message error on expansion macro: {}", err); - } + expand_task(&task).map(msg::Response::ExpansionMacro) } + }; + + let msg = res.unwrap_or_else(|err| { + msg::Response::Error(msg::ResponseError { + code: msg::ErrorCode::ExpansionError, + message: err, + }) + }); + + if let Err(err) = write_response(msg) { + eprintln!("Write message error: {}", err); } } } + +fn read_request() -> io::Result> { + msg::Request::read(&mut io::stdin().lock()) +} + +fn write_response(msg: msg::Response) -> io::Result<()> { + msg.write(&mut io::stdout().lock()) +} diff --git a/crates/ra_proc_macro_srv/src/dylib.rs b/crates/ra_proc_macro_srv/src/dylib.rs index 16bd7466e..f7a86a532 100644 --- a/crates/ra_proc_macro_srv/src/dylib.rs +++ b/crates/ra_proc_macro_srv/src/dylib.rs @@ -9,43 +9,37 @@ use libloading::Library; use memmap::Mmap; use ra_proc_macro::ProcMacroKind; -use std::io::Error as IoError; -use std::io::ErrorKind as IoErrorKind; +use std::io; const NEW_REGISTRAR_SYMBOL: &str = "_rustc_proc_macro_decls_"; -fn invalid_data_err(e: impl Into>) -> IoError { - IoError::new(IoErrorKind::InvalidData, e) +fn invalid_data_err(e: impl Into>) -> io::Error { + io::Error::new(io::ErrorKind::InvalidData, e) } -fn is_derive_registrar_symbol(symbol: &str) -> bool { +fn is_derive_registrar_symbol(symbol: &&str) -> bool { symbol.contains(NEW_REGISTRAR_SYMBOL) } -fn find_registrar_symbol(file: &Path) -> Result, IoError> { +fn find_registrar_symbol(file: &Path) -> io::Result> { let file = File::open(file)?; let buffer = unsafe { Mmap::map(&file)? }; let object = Object::parse(&buffer).map_err(invalid_data_err)?; - match object { + let name = match object { Object::Elf(elf) => { let symbols = elf.dynstrtab.to_vec().map_err(invalid_data_err)?; - let name = - symbols.iter().find(|s| is_derive_registrar_symbol(s)).map(|s| s.to_string()); - Ok(name) - } - Object::PE(pe) => { - let name = pe - .exports - .iter() - .flat_map(|s| s.name) - .find(|s| is_derive_registrar_symbol(s)) - .map(|s| s.to_string()); - Ok(name) + symbols.into_iter().find(is_derive_registrar_symbol).map(&str::to_owned) } + Object::PE(pe) => pe + .exports + .iter() + .flat_map(|s| s.name) + .find(is_derive_registrar_symbol) + .map(&str::to_owned), Object::Mach(Mach::Binary(binary)) => { let exports = binary.exports().map_err(invalid_data_err)?; - let name = exports + exports .iter() .map(|s| { // In macos doc: @@ -58,12 +52,12 @@ fn find_registrar_symbol(file: &Path) -> Result, IoError> { &s.name } }) - .find(|s| is_derive_registrar_symbol(s)) - .map(|s| s.to_string()); - Ok(name) + .find(is_derive_registrar_symbol) + .map(&str::to_owned) } - _ => Ok(None), - } + _ => return Ok(None), + }; + Ok(name) } /// Loads dynamic library in platform dependent manner. @@ -93,15 +87,16 @@ fn load_library(file: &Path) -> Result { } struct ProcMacroLibraryLibloading { - // Hold the dylib to prevent it for unloadeding + // Hold the dylib to prevent it from unloading _lib: Library, exported_macros: Vec, } impl ProcMacroLibraryLibloading { - fn open(file: &Path) -> Result { - let symbol_name = find_registrar_symbol(file)? - .ok_or(invalid_data_err(format!("Cannot find registrar symbol in file {:?}", file)))?; + fn open(file: &Path) -> io::Result { + let symbol_name = find_registrar_symbol(file)?.ok_or_else(|| { + invalid_data_err(format!("Cannot find registrar symbol in file {:?}", file)) + })?; let lib = load_library(file).map_err(invalid_data_err)?; let exported_macros = { @@ -121,18 +116,16 @@ pub struct Expander { } impl Expander { - pub fn new>(lib: &P) -> Result { - let mut libs = vec![]; - /* Some libraries for dynamic loading require canonicalized path (even when it is - already absolute - */ - let lib = - lib.as_ref().canonicalize().expect(&format!("Cannot canonicalize {:?}", lib.as_ref())); + pub fn new(lib: &Path) -> Result { + // Some libraries for dynamic loading require canonicalized path even when it is + // already absolute + let lib = lib + .canonicalize() + .unwrap_or_else(|err| panic!("Cannot canonicalize {:?}: {:?}", lib, err)); let library = ProcMacroLibraryImpl::open(&lib).map_err(|e| e.to_string())?; - libs.push(library); - Ok(Expander { libs }) + Ok(Expander { libs: vec![library] }) } pub fn expand( @@ -176,7 +169,6 @@ impl Expander { parsed_attributes, parsed_body, ); - return res.map(|it| it.subtree); } _ => continue, @@ -187,26 +179,21 @@ impl Expander { Err(bridge::PanicMessage::String("Nothing to expand".to_string())) } - pub fn list_macros(&self) -> Result, bridge::PanicMessage> { - let mut result = vec![]; - - for lib in &self.libs { - for proc_macro in &lib.exported_macros { - let res = match proc_macro { - bridge::client::ProcMacro::CustomDerive { trait_name, .. } => { - (trait_name.to_string(), ProcMacroKind::CustomDerive) - } - bridge::client::ProcMacro::Bang { name, .. } => { - (name.to_string(), ProcMacroKind::FuncLike) - } - bridge::client::ProcMacro::Attr { name, .. } => { - (name.to_string(), ProcMacroKind::Attr) - } - }; - result.push(res); - } - } - - Ok(result) + pub fn list_macros(&self) -> Vec<(String, ProcMacroKind)> { + self.libs + .iter() + .flat_map(|it| &it.exported_macros) + .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::FuncLike) + } + bridge::client::ProcMacro::Attr { name, .. } => { + (name.to_string(), ProcMacroKind::Attr) + } + }) + .collect() } } diff --git a/crates/ra_proc_macro_srv/src/lib.rs b/crates/ra_proc_macro_srv/src/lib.rs index c62b0ed89..7faf36834 100644 --- a/crates/ra_proc_macro_srv/src/lib.rs +++ b/crates/ra_proc_macro_srv/src/lib.rs @@ -3,10 +3,10 @@ //! This library is able to call compiled Rust custom derive dynamic libraries on arbitrary code. //! The general idea here is based on https://github.com/fedochet/rust-proc-macro-expander. //! -//! But we change some several design for fitting RA needs: +//! But we adapt it to better fit RA needs: //! -//! * We use `ra_tt` for proc-macro `TokenStream` server, it is easy to manipute and interact with -//! RA then proc-macro2 token stream. +//! * We use `ra_tt` for proc-macro `TokenStream` server, it is easy to manipulate and interact with +//! RA than `proc-macro2` token stream. //! * By **copying** the whole rustc `lib_proc_macro` code, we are able to build this with `stable` //! rustc rather than `unstable`. (Although in gerenal ABI compatibility is still an issue) @@ -21,36 +21,28 @@ mod dylib; use proc_macro::bridge::client::TokenStream; use ra_proc_macro::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask}; +use std::path::Path; pub(crate) fn expand_task(task: &ExpansionTask) -> Result { - let expander = dylib::Expander::new(&task.lib) - .expect(&format!("Cannot expand with provided libraries: ${:?}", &task.lib)); + let expander = create_expander(&task.lib); match expander.expand(&task.macro_name, &task.macro_body, task.attributes.as_ref()) { Ok(expansion) => Ok(ExpansionResult { expansion }), Err(msg) => { - let reason = format!( - "Cannot perform expansion for {}: error {:?}!", - &task.macro_name, - msg.as_str() - ); - Err(reason) + Err(format!("Cannot perform expansion for {}: error {:?}", &task.macro_name, msg)) } } } -pub(crate) fn list_macros(task: &ListMacrosTask) -> Result { - let expander = dylib::Expander::new(&task.lib) - .expect(&format!("Cannot expand with provided libraries: ${:?}", &task.lib)); +pub(crate) fn list_macros(task: &ListMacrosTask) -> ListMacrosResult { + let expander = create_expander(&task.lib); - match expander.list_macros() { - Ok(macros) => Ok(ListMacrosResult { macros }), - Err(msg) => { - let reason = - format!("Cannot perform expansion for {:?}: error {:?}!", &task.lib, msg.as_str()); - Err(reason) - } - } + ListMacrosResult { macros: expander.list_macros() } +} + +fn create_expander(lib: &Path) -> dylib::Expander { + dylib::Expander::new(lib) + .unwrap_or_else(|err| panic!("Cannot create expander for {:?}: {:?}", lib, err)) } pub mod cli; diff --git a/crates/ra_proc_macro_srv/src/rustc_server.rs b/crates/ra_proc_macro_srv/src/rustc_server.rs index 9fcfdc450..f481d70b2 100644 --- a/crates/ra_proc_macro_srv/src/rustc_server.rs +++ b/crates/ra_proc_macro_srv/src/rustc_server.rs @@ -6,7 +6,7 @@ //! The original idea from fedochet is using proc-macro2 as backend, //! we use ra_tt instead for better intergation with RA. //! -//! FIXME: No span and source file informatin is implemented yet +//! FIXME: No span and source file information is implemented yet use crate::proc_macro::bridge::{self, server}; use ra_tt as tt; diff --git a/crates/ra_proc_macro_srv/src/tests/utils.rs b/crates/ra_proc_macro_srv/src/tests/utils.rs index 1ee409449..2139ec7a4 100644 --- a/crates/ra_proc_macro_srv/src/tests/utils.rs +++ b/crates/ra_proc_macro_srv/src/tests/utils.rs @@ -60,6 +60,6 @@ pub fn list(crate_name: &str, version: &str) -> Vec { let path = fixtures::dylib_path(crate_name, version); let task = ListMacrosTask { lib: path }; - let res = list_macros(&task).unwrap(); + let res = list_macros(&task); res.macros.into_iter().map(|(name, kind)| format!("{} [{:?}]", name, kind)).collect() } diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 28b67cfe2..e8d5dad65 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -51,7 +51,7 @@ fn main() -> Result<()> { cli::diagnostics(path.as_ref(), load_output_dirs, with_proc_macro, all)? } - args::Command::ProcMacro => run_proc_macro_sv()?, + args::Command::ProcMacro => run_proc_macro_srv()?, args::Command::RunServer => run_server()?, args::Command::Version => println!("rust-analyzer {}", env!("REV")), } @@ -65,7 +65,7 @@ fn setup_logging() -> Result<()> { Ok(()) } -fn run_proc_macro_sv() -> Result<()> { +fn run_proc_macro_srv() -> Result<()> { ra_proc_macro_srv::cli::run(); Ok(()) } diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index 762f776fe..d0a71120a 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs @@ -76,7 +76,7 @@ pub(crate) fn load_cargo( ProcMacroClient::dummy() } else { let path = std::env::current_exe()?; - ProcMacroClient::extern_process(&path, &["proc-macro"]).unwrap() + ProcMacroClient::extern_process(path, &["proc-macro"]).unwrap() }; let host = load(&source_roots, ws, &mut vfs, receiver, extern_dirs, &proc_macro_client); Ok((host, source_roots)) diff --git a/crates/rust-analyzer/src/world.rs b/crates/rust-analyzer/src/world.rs index f2ad453fa..d50f3f3d4 100644 --- a/crates/rust-analyzer/src/world.rs +++ b/crates/rust-analyzer/src/world.rs @@ -148,20 +148,17 @@ impl WorldState { let proc_macro_client = match &config.proc_macro_srv { None => ProcMacroClient::dummy(), - Some((path, args)) => { - let path = std::path::Path::new(path); - match ProcMacroClient::extern_process(path, args) { - Ok(it) => it, - Err(err) => { - log::error!( - "Fail to run ra_proc_macro_srv from path {}, error : {}", - path.to_string_lossy(), - err - ); - ProcMacroClient::dummy() - } + Some((path, args)) => match ProcMacroClient::extern_process(path.into(), args) { + Ok(it) => it, + Err(err) => { + log::error!( + "Fail to run ra_proc_macro_srv from path {}, error: {:?}", + path, + err + ); + ProcMacroClient::dummy() } - } + }, }; workspaces -- cgit v1.2.3