From 32128bbba632cbe0c74e63cd1b646289431f7018 Mon Sep 17 00:00:00 2001 From: reuben olinsky Date: Mon, 13 Jan 2025 08:24:09 -0800 Subject: [PATCH] chore: remove unneeded clones for arith eval --- brush-core/src/arithmetic.rs | 49 ++++++++++++++++--------- brush-core/src/expansion.rs | 8 ++--- brush-core/src/extendedtests.rs | 63 +++++++-------------------------- brush-core/src/interp.rs | 5 ++- 4 files changed, 51 insertions(+), 74 deletions(-) diff --git a/brush-core/src/arithmetic.rs b/brush-core/src/arithmetic.rs index 42c315a..ae92d7c 100644 --- a/brush-core/src/arithmetic.rs +++ b/brush-core/src/arithmetic.rs @@ -52,25 +52,40 @@ pub trait ExpandAndEvaluate { impl ExpandAndEvaluate for ast::UnexpandedArithmeticExpr { async fn eval(&self, shell: &mut Shell, trace_if_needed: bool) -> Result { - // Per documentation, first shell-expand it. - let expanded_self = expansion::basic_expand_str_without_tilde(shell, self.value.as_str()) - .await - .map_err(|_e| EvalError::FailedToExpandExpression)?; - - // Now parse. - let expr = brush_parser::arithmetic::parse(&expanded_self) - .map_err(|_e| EvalError::ParseError(expanded_self))?; - - // Trace if applicable. - if trace_if_needed && shell.options.print_commands_and_arguments { - shell - .trace_command(std::format!("(( {expr} ))")) - .map_err(|_err| EvalError::TraceError)?; - } + expand_and_eval(shell, self.value.as_str(), trace_if_needed).await + } +} - // Now evaluate. - expr.eval(shell) +/// Evaluate the given arithmetic expression, returning the resulting numeric value. +/// +/// # Arguments +/// +/// * `shell` - The shell to use for evaluation. +/// * `expr` - The unexpanded arithmetic expression to evaluate. +/// * `trace_if_needed` - Whether to trace the evaluation. +pub(crate) async fn expand_and_eval( + shell: &mut Shell, + expr: &str, + trace_if_needed: bool, +) -> Result { + // Per documentation, first shell-expand it. + let expanded_self = expansion::basic_expand_str_without_tilde(shell, expr) + .await + .map_err(|_e| EvalError::FailedToExpandExpression)?; + + // Now parse. + let expr = brush_parser::arithmetic::parse(&expanded_self) + .map_err(|_e| EvalError::ParseError(expanded_self))?; + + // Trace if applicable. + if trace_if_needed && shell.options.print_commands_and_arguments { + shell + .trace_command(std::format!("(( {expr} ))")) + .map_err(|_err| EvalError::TraceError)?; } + + // Now evaluate. + expr.eval(shell) } /// Trait implemented by evaluatable arithmetic expressions. diff --git a/brush-core/src/expansion.rs b/brush-core/src/expansion.rs index 6be5910..a2ef88f 100644 --- a/brush-core/src/expansion.rs +++ b/brush-core/src/expansion.rs @@ -6,6 +6,7 @@ use brush_parser::word::ParameterTransformOp; use brush_parser::word::SubstringMatchKind; use itertools::Itertools; +use crate::arithmetic; use crate::arithmetic::ExpandAndEvaluate; use crate::commands; use crate::env; @@ -1334,10 +1335,9 @@ impl<'a> WordExpander<'a> { let index_to_use = if for_set_associative_array { self.basic_expand_to_str(index).await? } else { - let index_expr = ast::UnexpandedArithmeticExpr { - value: index.to_owned(), - }; - self.expand_arithmetic_expr(index_expr).await? + arithmetic::expand_and_eval(self.shell, index, false) + .await? + .to_string() }; Ok(index_to_use) diff --git a/brush-core/src/extendedtests.rs b/brush-core/src/extendedtests.rs index 824e9dd..6cc4140 100644 --- a/brush-core/src/extendedtests.rs +++ b/brush-core/src/extendedtests.rs @@ -2,8 +2,7 @@ use brush_parser::ast; use std::path::Path; use crate::{ - arithmetic::ExpandAndEvaluate, - env, error, escape, expansion, namedoptions, patterns, + arithmetic, env, error, escape, expansion, namedoptions, patterns, sys::{ fs::{MetadataExt, PathExt}, users, @@ -269,14 +268,8 @@ async fn apply_binary_predicate( Ok(left > right) } ast::BinaryPredicate::ArithmeticEqualTo => { - let unexpanded_left = ast::UnexpandedArithmeticExpr { - value: left.value.clone(), - }; - let unexpanded_right = ast::UnexpandedArithmeticExpr { - value: right.value.clone(), - }; - let left = unexpanded_left.eval(shell, false).await?; - let right = unexpanded_right.eval(shell, false).await?; + let left = arithmetic::expand_and_eval(shell, left.value.as_str(), false).await?; + let right = arithmetic::expand_and_eval(shell, right.value.as_str(), false).await?; if shell.options.print_commands_and_arguments { shell.trace_command(std::format!("[[ {left} {op} {right} ]]"))?; @@ -285,14 +278,8 @@ async fn apply_binary_predicate( Ok(left == right) } ast::BinaryPredicate::ArithmeticNotEqualTo => { - let unexpanded_left = ast::UnexpandedArithmeticExpr { - value: left.value.clone(), - }; - let unexpanded_right = ast::UnexpandedArithmeticExpr { - value: right.value.clone(), - }; - let left = unexpanded_left.eval(shell, false).await?; - let right = unexpanded_right.eval(shell, false).await?; + let left = arithmetic::expand_and_eval(shell, left.value.as_str(), false).await?; + let right = arithmetic::expand_and_eval(shell, right.value.as_str(), false).await?; if shell.options.print_commands_and_arguments { shell.trace_command(std::format!("[[ {left} {op} {right} ]]"))?; @@ -301,14 +288,8 @@ async fn apply_binary_predicate( Ok(left != right) } ast::BinaryPredicate::ArithmeticLessThan => { - let unexpanded_left = ast::UnexpandedArithmeticExpr { - value: left.value.clone(), - }; - let unexpanded_right = ast::UnexpandedArithmeticExpr { - value: right.value.clone(), - }; - let left = unexpanded_left.eval(shell, false).await?; - let right = unexpanded_right.eval(shell, false).await?; + let left = arithmetic::expand_and_eval(shell, left.value.as_str(), false).await?; + let right = arithmetic::expand_and_eval(shell, right.value.as_str(), false).await?; if shell.options.print_commands_and_arguments { shell.trace_command(std::format!("[[ {left} {op} {right} ]]"))?; @@ -317,14 +298,8 @@ async fn apply_binary_predicate( Ok(left < right) } ast::BinaryPredicate::ArithmeticLessThanOrEqualTo => { - let unexpanded_left = ast::UnexpandedArithmeticExpr { - value: left.value.clone(), - }; - let unexpanded_right = ast::UnexpandedArithmeticExpr { - value: right.value.clone(), - }; - let left = unexpanded_left.eval(shell, false).await?; - let right = unexpanded_right.eval(shell, false).await?; + let left = arithmetic::expand_and_eval(shell, left.value.as_str(), false).await?; + let right = arithmetic::expand_and_eval(shell, right.value.as_str(), false).await?; if shell.options.print_commands_and_arguments { shell.trace_command(std::format!("[[ {left} {op} {right} ]]"))?; @@ -333,14 +308,8 @@ async fn apply_binary_predicate( Ok(left <= right) } ast::BinaryPredicate::ArithmeticGreaterThan => { - let unexpanded_left = ast::UnexpandedArithmeticExpr { - value: left.value.clone(), - }; - let unexpanded_right = ast::UnexpandedArithmeticExpr { - value: right.value.clone(), - }; - let left = unexpanded_left.eval(shell, false).await?; - let right = unexpanded_right.eval(shell, false).await?; + let left = arithmetic::expand_and_eval(shell, left.value.as_str(), false).await?; + let right = arithmetic::expand_and_eval(shell, right.value.as_str(), false).await?; if shell.options.print_commands_and_arguments { shell.trace_command(std::format!("[[ {left} {op} {right} ]]"))?; @@ -349,14 +318,8 @@ async fn apply_binary_predicate( Ok(left > right) } ast::BinaryPredicate::ArithmeticGreaterThanOrEqualTo => { - let unexpanded_left = ast::UnexpandedArithmeticExpr { - value: left.value.clone(), - }; - let unexpanded_right = ast::UnexpandedArithmeticExpr { - value: right.value.clone(), - }; - let left = unexpanded_left.eval(shell, false).await?; - let right = unexpanded_right.eval(shell, false).await?; + let left = arithmetic::expand_and_eval(shell, left.value.as_str(), false).await?; + let right = arithmetic::expand_and_eval(shell, right.value.as_str(), false).await?; if shell.options.print_commands_and_arguments { shell.trace_command(std::format!("[[ {left} {op} {right} ]]"))?; diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs index 5cb1dd5..8fc3d64 100644 --- a/brush-core/src/interp.rs +++ b/brush-core/src/interp.rs @@ -9,7 +9,7 @@ use std::os::unix::process::ExitStatusExt; use std::path::{Path, PathBuf}; use std::sync::Arc; -use crate::arithmetic::ExpandAndEvaluate; +use crate::arithmetic::{self, ExpandAndEvaluate}; use crate::commands::{self, CommandArg, CommandSpawnResult}; use crate::env::{EnvironmentLookup, EnvironmentScope}; use crate::openfiles::{OpenFile, OpenFiles}; @@ -1219,8 +1219,7 @@ async fn apply_assignment( if will_be_indexed_array { array_index = Some( - ast::UnexpandedArithmeticExpr { value: idx.clone() } - .eval(shell, false) + arithmetic::expand_and_eval(shell, idx.as_str(), false) .await? .to_string(), );