From ae132f478c6cfc6ffb062fecd7efb42ac4e5d804 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Mon, 3 Jan 2022 06:36:44 -0500 Subject: stop cloning exe and env all over the place --- Cargo.lock | 12 ----------- Cargo.toml | 1 - src/builtins/command.rs | 57 +++++++++++++++++++++++++++++++------------------ src/builtins/mod.rs | 56 ++++++++++++++++++++++-------------------------- src/env.rs | 1 - src/parse.rs | 11 ++++------ src/pipeline/command.rs | 41 +++++++++++++++++++++++------------ src/pipeline/mod.rs | 6 +++--- 8 files changed, 96 insertions(+), 89 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27fd957..63a8de5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -108,17 +108,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "async-recursion" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7d78656ba01f1b93024b7c3a0467f1608e4be67d725749fdcd7d2c7678fd7a2" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "async-std" version = "1.10.0" @@ -503,7 +492,6 @@ name = "nbsh" version = "0.1.0" dependencies = [ "anyhow", - "async-recursion", "async-std", "futures-lite", "futures-util", diff --git a/Cargo.toml b/Cargo.toml index ca89db4..e132517 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,6 @@ license = "MIT" [dependencies] anyhow = "1.0.52" -async-recursion = "0.3.2" async-std = { version = "1.10.0", features = ["unstable"] } futures-lite = "1.12.0" futures-util = "0.3.19" diff --git a/src/builtins/command.rs b/src/builtins/command.rs index 6dfa56c..2ea7f18 100644 --- a/src/builtins/command.rs +++ b/src/builtins/command.rs @@ -8,12 +8,16 @@ pub struct Command { } impl Command { - pub fn new(exe: &crate::parse::Exe) -> Option { - super::BUILTINS.get(exe.exe()).map(|f| Self { - exe: exe.clone(), - f, - io: Io::new(), - }) + pub fn new(exe: crate::parse::Exe) -> Result { + if let Some(f) = super::BUILTINS.get(exe.exe()) { + Ok(Self { + exe, + f, + io: Io::new(), + }) + } else { + Err(exe) + } } pub fn stdin(&mut self, fh: std::fs::File) { @@ -37,7 +41,7 @@ impl Command { pub fn spawn(self, env: &crate::env::Env) -> anyhow::Result { let Self { f, exe, io } = self; - (f)(&exe, env, io) + (f)(exe, env, io) } } @@ -186,24 +190,25 @@ impl Drop for Io { } } -pub struct Child { +pub struct Child<'a> { fut: std::pin::Pin< Box< dyn std::future::Future + Sync - + Send, + + Send + + 'a, >, >, - wrapped_child: Option>, + wrapped_child: Option>>, } -impl Child { +impl<'a> Child<'a> { pub fn new_fut(fut: F) -> Self where F: std::future::Future + Sync + Send - + 'static, + + 'a, { Self { fut: Box::pin(fut), @@ -211,7 +216,7 @@ impl Child { } } - pub fn new_wrapped(child: crate::pipeline::Child) -> Self { + pub fn new_wrapped(child: crate::pipeline::Child<'a>) -> Self { Self { fut: Box::pin(async move { unreachable!() }), wrapped_child: Some(Box::new(child)), @@ -222,14 +227,24 @@ impl Child { self.wrapped_child.as_ref().and_then(|cmd| cmd.id()) } - #[async_recursion::async_recursion] - pub async fn status( + // can't use async_recursion because it enforces a 'static lifetime + pub fn status( self, - ) -> anyhow::Result { - if let Some(child) = self.wrapped_child { - child.status().await - } else { - Ok(self.fut.await) - } + ) -> std::pin::Pin< + Box< + dyn std::future::Future< + Output = anyhow::Result, + > + Send + + Sync + + 'a, + >, + > { + Box::pin(async move { + if let Some(child) = self.wrapped_child { + child.status().await + } else { + Ok(self.fut.await) + } + }) } } diff --git a/src/builtins/mod.rs b/src/builtins/mod.rs index 5c7d4b6..f39d3ef 100644 --- a/src/builtins/mod.rs +++ b/src/builtins/mod.rs @@ -3,11 +3,11 @@ use std::os::unix::process::ExitStatusExt as _; pub mod command; pub use command::{Child, Command}; -type Builtin = &'static (dyn Fn( - &crate::parse::Exe, - &crate::env::Env, +type Builtin = &'static (dyn for<'a> Fn( + crate::parse::Exe, + &'a crate::env::Env, command::Io, -) -> anyhow::Result +) -> anyhow::Result> + Sync + Send); @@ -28,12 +28,12 @@ static BUILTINS: once_cell::sync::Lazy< // clippy can't tell that the type is necessary #[allow(clippy::unnecessary_wraps)] fn cd( - exe: &crate::parse::Exe, + exe: crate::parse::Exe, env: &crate::env::Env, io: command::Io, ) -> anyhow::Result { async fn async_cd( - exe: &crate::parse::Exe, + exe: crate::parse::Exe, _env: &crate::env::Env, io: command::Io, ) -> std::process::ExitStatus { @@ -84,10 +84,8 @@ fn cd( async_std::process::ExitStatus::from_raw(code << 8) } - let exe = exe.clone(); - let env = env.clone(); Ok(command::Child::new_fut(async move { - async_cd(&exe, &env, io).await + async_cd(exe, env, io).await })) } @@ -96,12 +94,12 @@ fn cd( // mostly just for testing and ensuring that builtins work, i'll likely remove // this later, since the binary seems totally fine fn echo( - exe: &crate::parse::Exe, + exe: crate::parse::Exe, env: &crate::env::Env, io: command::Io, ) -> anyhow::Result { async fn async_echo( - exe: &crate::parse::Exe, + exe: crate::parse::Exe, _env: &crate::env::Env, io: command::Io, ) -> std::process::ExitStatus { @@ -128,63 +126,61 @@ fn echo( async_std::process::ExitStatus::from_raw(0) } - let exe = exe.clone(); - let env = env.clone(); Ok(command::Child::new_fut(async move { - async_echo(&exe, &env, io).await + async_echo(exe, env, io).await })) } fn and( - exe: &crate::parse::Exe, + mut exe: crate::parse::Exe, env: &crate::env::Env, io: command::Io, ) -> anyhow::Result { - let exe = exe.shift(); + exe.shift(); if env.latest_status().success() { - let mut cmd = crate::pipeline::Command::new(&exe); + let mut cmd = crate::pipeline::Command::new(exe); io.setup_command(&mut cmd); Ok(command::Child::new_wrapped(cmd.spawn(env)?)) } else { - let env = env.clone(); - Ok(command::Child::new_fut(async move { *env.latest_status() })) + let status = *env.latest_status(); + Ok(command::Child::new_fut(async move { status })) } } fn or( - exe: &crate::parse::Exe, + mut exe: crate::parse::Exe, env: &crate::env::Env, io: command::Io, ) -> anyhow::Result { - let exe = exe.shift(); + exe.shift(); if env.latest_status().success() { - let env = env.clone(); - Ok(command::Child::new_fut(async move { *env.latest_status() })) + let status = *env.latest_status(); + Ok(command::Child::new_fut(async move { status })) } else { - let mut cmd = crate::pipeline::Command::new(&exe); + let mut cmd = crate::pipeline::Command::new(exe); io.setup_command(&mut cmd); Ok(command::Child::new_wrapped(cmd.spawn(env)?)) } } fn command( - exe: &crate::parse::Exe, + mut exe: crate::parse::Exe, env: &crate::env::Env, io: command::Io, ) -> anyhow::Result { - let exe = exe.shift(); - let mut cmd = crate::pipeline::Command::new_binary(&exe); + exe.shift(); + let mut cmd = crate::pipeline::Command::new_binary(exe); io.setup_command(&mut cmd); Ok(command::Child::new_wrapped(cmd.spawn(env)?)) } fn builtin( - exe: &crate::parse::Exe, + mut exe: crate::parse::Exe, env: &crate::env::Env, io: command::Io, ) -> anyhow::Result { - let exe = exe.shift(); - let mut cmd = crate::pipeline::Command::new_builtin(&exe); + exe.shift(); + let mut cmd = crate::pipeline::Command::new_builtin(exe); io.setup_command(&mut cmd); Ok(command::Child::new_wrapped(cmd.spawn(env)?)) } diff --git a/src/env.rs b/src/env.rs index e88ec7c..31671a3 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,6 +1,5 @@ use std::os::unix::process::ExitStatusExt as _; -#[derive(Clone)] pub struct Env { latest_status: async_std::process::ExitStatus, } diff --git a/src/parse.rs b/src/parse.rs index 0249d7f..4bdbce2 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -51,11 +51,8 @@ impl Exe { self.args.iter().map(|arg| arg.word.as_ref()) } - pub fn shift(&self) -> Self { - let mut new = self.clone(); - let new_exe = new.args.remove(0); - new.exe = new_exe; - new + pub fn shift(&mut self) { + self.exe = self.args.remove(0); } } @@ -75,8 +72,8 @@ impl Pipeline { )) } - pub fn exes(&self) -> &[Exe] { - &self.exes + pub fn into_exes(self) -> impl Iterator { + self.exes.into_iter() } pub fn input_string(&self) -> &str { diff --git a/src/pipeline/command.rs b/src/pipeline/command.rs index 5a7798c..e4a3537 100644 --- a/src/pipeline/command.rs +++ b/src/pipeline/command.rs @@ -6,20 +6,21 @@ pub enum Command { } impl Command { - pub fn new(exe: &crate::parse::Exe) -> Self { + pub fn new(exe: crate::parse::Exe) -> Self { crate::builtins::Command::new(exe) - .map_or_else(|| Self::new_binary(exe), Self::Builtin) + .map_or_else(Self::new_binary, Self::Builtin) } - pub fn new_binary(exe: &crate::parse::Exe) -> Self { + #[allow(clippy::needless_pass_by_value)] + pub fn new_binary(exe: crate::parse::Exe) -> Self { let mut cmd = async_std::process::Command::new(exe.exe()); cmd.args(exe.args()); Self::Binary(cmd) } - pub fn new_builtin(exe: &crate::parse::Exe) -> Self { + pub fn new_builtin(exe: crate::parse::Exe) -> Self { crate::builtins::Command::new(exe) - .map_or_else(|| todo!(), Self::Builtin) + .map_or_else(|_| todo!(), Self::Builtin) } pub fn stdin(&mut self, fh: std::fs::File) { @@ -77,12 +78,12 @@ impl Command { } } -pub enum Child { +pub enum Child<'a> { Binary(async_std::process::Child), - Builtin(crate::builtins::Child), + Builtin(crate::builtins::Child<'a>), } -impl Child { +impl<'a> Child<'a> { pub fn id(&self) -> Option { match self { Self::Binary(child) => Some(child.id()), @@ -90,11 +91,23 @@ impl Child { } } - #[async_recursion::async_recursion] - pub async fn status(self) -> anyhow::Result { - match self { - Self::Binary(child) => Ok(child.status_no_drop().await?), - Self::Builtin(child) => Ok(child.status().await?), - } + // can't use async_recursion because it enforces a 'static lifetime + pub fn status( + self, + ) -> std::pin::Pin< + Box< + dyn std::future::Future< + Output = anyhow::Result, + > + Send + + Sync + + 'a, + >, + > { + Box::pin(async move { + match self { + Self::Binary(child) => Ok(child.status_no_drop().await?), + Self::Builtin(child) => Ok(child.status().await?), + } + }) } } diff --git a/src/pipeline/mod.rs b/src/pipeline/mod.rs index b1e4c21..1f9a607 100644 --- a/src/pipeline/mod.rs +++ b/src/pipeline/mod.rs @@ -11,7 +11,7 @@ pub use command::{Child, Command}; pub async fn run() -> anyhow::Result { let (code, pipeline) = read_data().await?; let env = crate::env::Env::new(code); - let children = spawn_children(&pipeline, &env)?; + let children = spawn_children(pipeline, &env)?; let count = children.len(); let mut children: futures_util::stream::FuturesUnordered<_> = @@ -59,10 +59,10 @@ async fn read_data() -> anyhow::Result<(i32, crate::parse::Pipeline)> { } fn spawn_children( - pipeline: &crate::parse::Pipeline, + pipeline: crate::parse::Pipeline, env: &crate::env::Env, ) -> anyhow::Result> { - let mut cmds: Vec<_> = pipeline.exes().iter().map(Command::new).collect(); + let mut cmds: Vec<_> = pipeline.into_exes().map(Command::new).collect(); for i in 0..(cmds.len() - 1) { let (r, w) = pipe()?; cmds[i].stdout(w); -- cgit v1.2.3-54-g00ecf