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 ++++++++-------- 4 files changed, 42 insertions(+), 55 deletions(-) (limited to 'crates/ra_proc_macro') 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. -- cgit v1.2.3 From d8ca817456ed60b0163d3f81c58f6db9e6372e5f Mon Sep 17 00:00:00 2001 From: veetaha Date: Mon, 20 Apr 2020 22:24:10 +0300 Subject: Fix doc comment --- crates/ra_proc_macro/src/rpc.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'crates/ra_proc_macro') diff --git a/crates/ra_proc_macro/src/rpc.rs b/crates/ra_proc_macro/src/rpc.rs index e97b90860..4ce485926 100644 --- a/crates/ra_proc_macro/src/rpc.rs +++ b/crates/ra_proc_macro/src/rpc.rs @@ -39,10 +39,10 @@ pub struct ExpansionTask { #[serde(with = "SubtreeDef")] pub macro_body: Subtree, - /// Names of macros to expand. // TODO: are they comma-separated? + /// Name of macro to expand. /// - /// In custom derive those are names of derived traits (`Serialize`, `Getters`, etc.). In - /// attribute-like and function-like macros - single name of macro itself (`show_streams`). + /// In custom derive this is the name of the derived trait (`Serialize`, `Getters`, etc.). + /// In attribute-like and function-like macros - single name of macro itself (`show_streams`). pub macro_name: String, /// Possible attributes for the attribute-like macros. -- cgit v1.2.3 From fc460b1e423c2c510a10539f8289af548685676c Mon Sep 17 00:00:00 2001 From: veetaha Date: Mon, 20 Apr 2020 22:42:36 +0300 Subject: Migrate to Result -> io::Result --- crates/ra_proc_macro/src/process.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'crates/ra_proc_macro') diff --git a/crates/ra_proc_macro/src/process.rs b/crates/ra_proc_macro/src/process.rs index b1ebf46a1..e24944af4 100644 --- a/crates/ra_proc_macro/src/process.rs +++ b/crates/ra_proc_macro/src/process.rs @@ -48,7 +48,7 @@ impl Process { fn run( process_path: PathBuf, args: impl IntoIterator>, - ) -> Result { + ) -> io::Result { let child = Command::new(&process_path) .args(args) .stdin(Stdio::piped()) @@ -59,7 +59,7 @@ impl Process { Ok(Process { path: process_path, child }) } - fn restart(&mut self) -> Result<(), io::Error> { + fn restart(&mut self) -> io::Result<()> { let _ = self.child.kill(); self.child = Command::new(&self.path) .stdin(Stdio::piped()) @@ -196,7 +196,7 @@ fn send_request( mut writer: &mut impl Write, mut reader: &mut impl BufRead, req: Request, -) -> Result, io::Error> { +) -> io::Result> { req.write(&mut writer)?; Ok(Response::read(&mut reader)?) } -- cgit v1.2.3 From bd350108b0d1be67e86b93a94c324317a00b57cd Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Tue, 21 Apr 2020 04:57:55 +0800 Subject: Fix restart missing arguments in proc-macro-srv --- crates/ra_proc_macro/src/process.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'crates/ra_proc_macro') diff --git a/crates/ra_proc_macro/src/process.rs b/crates/ra_proc_macro/src/process.rs index e24944af4..97ba196b8 100644 --- a/crates/ra_proc_macro/src/process.rs +++ b/crates/ra_proc_macro/src/process.rs @@ -9,7 +9,7 @@ use crate::rpc::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTas use io::{BufRead, BufReader}; use std::{ convert::{TryFrom, TryInto}, - ffi::OsStr, + ffi::{OsStr, OsString}, io::{self, Write}, path::{Path, PathBuf}, process::{Child, Command, Stdio}, @@ -35,6 +35,7 @@ struct Task { struct Process { path: PathBuf, + args: Vec, child: Child, } @@ -46,22 +47,25 @@ impl Drop for Process { impl Process { fn run( - process_path: PathBuf, + path: PathBuf, args: impl IntoIterator>, ) -> io::Result { - let child = Command::new(&process_path) - .args(args) + let args = args.into_iter().map(|s| s.as_ref().into()).collect(); + + let child = Command::new(&path) + .args(&args) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::null()) .spawn()?; - Ok(Process { path: process_path, child }) + Ok(Process { path, args, child }) } fn restart(&mut self) -> io::Result<()> { let _ = self.child.kill(); self.child = Command::new(&self.path) + .args(&self.args) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::null()) -- cgit v1.2.3 From ce382e6a79edf9c00d4e7d7c3834cde7577e6517 Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Tue, 21 Apr 2020 05:22:17 +0800 Subject: Refactor a bit --- crates/ra_proc_macro/src/process.rs | 105 ++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 54 deletions(-) (limited to 'crates/ra_proc_macro') diff --git a/crates/ra_proc_macro/src/process.rs b/crates/ra_proc_macro/src/process.rs index 97ba196b8..673f80a7a 100644 --- a/crates/ra_proc_macro/src/process.rs +++ b/crates/ra_proc_macro/src/process.rs @@ -28,60 +28,6 @@ pub(crate) struct ProcMacroProcessThread { handle: jod_thread::JoinHandle<()>, } -struct Task { - req: Request, - result_tx: Sender>, -} - -struct Process { - path: PathBuf, - args: Vec, - child: Child, -} - -impl Drop for Process { - fn drop(&mut self) { - let _ = self.child.kill(); - } -} - -impl Process { - fn run( - path: PathBuf, - args: impl IntoIterator>, - ) -> io::Result { - let args = args.into_iter().map(|s| s.as_ref().into()).collect(); - - let child = Command::new(&path) - .args(&args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn()?; - - Ok(Process { path, args, child }) - } - - fn restart(&mut self) -> io::Result<()> { - let _ = self.child.kill(); - self.child = Command::new(&self.path) - .args(&self.args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn()?; - Ok(()) - } - - fn stdio(&mut self) -> Option<(impl Write, impl BufRead)> { - let stdin = self.child.stdin.take()?; - let stdout = self.child.stdout.take()?; - let read = BufReader::new(stdout); - - Some((stdin, read)) - } -} - impl ProcMacroProcessSrv { pub fn run( process_path: PathBuf, @@ -196,6 +142,57 @@ fn client_loop(task_rx: Receiver, mut process: Process) { } } +struct Task { + req: Request, + result_tx: Sender>, +} + +struct Process { + path: PathBuf, + args: Vec, + child: Child, +} + +impl Drop for Process { + fn drop(&mut self) { + let _ = self.child.kill(); + } +} + +impl Process { + fn run( + path: PathBuf, + args: impl IntoIterator>, + ) -> io::Result { + let args = args.into_iter().map(|s| s.as_ref().into()).collect(); + let child = mk_child(&path, &args)?; + Ok(Process { path, args, child }) + } + + fn restart(&mut self) -> io::Result<()> { + let _ = self.child.kill(); + self.child = mk_child(&self.path, &self.args)?; + Ok(()) + } + + fn stdio(&mut self) -> Option<(impl Write, impl BufRead)> { + let stdin = self.child.stdin.take()?; + let stdout = self.child.stdout.take()?; + let read = BufReader::new(stdout); + + Some((stdin, read)) + } +} + +fn mk_child(path: &Path, args: impl IntoIterator>) -> io::Result { + Command::new(&path) + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() +} + fn send_request( mut writer: &mut impl Write, mut reader: &mut impl BufRead, -- cgit v1.2.3