From 0cc5d9f6367acc463e9fbf3b2d39f3c8e65bc3ed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 06:39:33 +0000 Subject: [PATCH 1/3] Initial plan From ae6a813505495b991256d75924a0d413baa0728d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 06:48:31 +0000 Subject: [PATCH 2/3] Consolidate AI bot PRs: apply remaining Bolt optimizations and bump bytes dependency Merges changes from the following open AI bot PRs: - PRs #34-43: Clippy fixes, field shorthand, lifetime annotations, iterator optimizations - PRs #45-55: Value Display, parser list/map/chain expr optimizations - PRs #56-57: Token string allocation optimizations (already in codebase) - PR #33: Bump bytes 1.4.0 -> 1.11.1 (dependabot) - PR #38: Value Display formatting (already in codebase) Changes applied: - parser.rs: Remove unnecessary & on op args, into_iter -> iter in describe(), push_str("x") -> push('x'), borrow instead of clone in map_expr, lifetime annotations - tokenizer.rs: Field shorthand, remove unnecessary return, use is_ascii_*() methods, lifetime annotations, Copy instead of clone for Token - operator.rs: Field shorthand for all managers, *precedence instead of clone() - function.rs: Field shorthand - descriptor.rs: Use ? operator for early return - lib.rs: Explicit lifetime annotation on parse_expression - .jules/bolt.md: Add Display optimization learning - Cargo.lock: Bump bytes 1.4.0 -> 1.11.1 Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/e76f8236-8653-4bc9-bf47-86b983ff48e3 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com> --- .jules/bolt.md | 4 ++++ Cargo.lock | 6 +++--- src/descriptor.rs | 4 +--- src/function.rs | 2 +- src/lib.rs | 2 +- src/operator.rs | 8 +++---- src/parser.rs | 54 +++++++++++++++++++++++------------------------ src/tokenizer.rs | 28 ++++++++++++------------ 8 files changed, 54 insertions(+), 54 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index 1878ef8..d3c2f53 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -9,3 +9,7 @@ ## 2024-05-26 - Optimize decimal conversion in Value **Learning:** `rust_decimal::Decimal` allows efficient and direct conversion to basic types like `i64` and `f64` via `to_i64` and `to_f64` using the `rust_decimal::prelude::ToPrimitive` trait. Converting to string and then parsing `val.to_string().parse()` is an anti-pattern as it incurs heap allocation overhead, which is bad for performance. When doing integer conversions from `Decimal`, it's critical to check `val.scale() == 0` first to maintain behavioral parity with string parsing (which would reject floats). **Action:** Always favor direct conversion traits like `rust_decimal::prelude::ToPrimitive` methods over stringification when converting `Decimal` values to native numeric types. Keep edge cases like `scale()` properties in mind when changing conversion methods. + +## 2024-05-27 - Optimize Display implementations for collections +**Learning:** `fmt::Display` implementations involving collections (like Lists or Maps) can be highly inefficient if they build intermediate `String`s via `format!()` and `push_str()` inside loops. This causes unnecessary heap allocations. +**Action:** Always write directly to the `fmt::Formatter` inside a loop using `write!(f, ...)` to minimize heap allocations and improve formatting performance. diff --git a/Cargo.lock b/Cargo.lock index 24c6de1..83535db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "ahash" @@ -156,9 +156,9 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" [[package]] name = "bytes" -version = "1.4.0" +version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89b2fd2a0dcf38d7971e2194b6b6eebab45ae01067456a7fd93d5547a61b70be" +checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" [[package]] name = "cast" diff --git a/src/descriptor.rs b/src/descriptor.rs index 0623009..a518989 100644 --- a/src/descriptor.rs +++ b/src/descriptor.rs @@ -57,9 +57,7 @@ impl DescriptorManager { fn get(&self, key: DescriptorKey) -> Option { let binding = self.store.lock().unwrap(); let value = binding.get(&key); - if value.is_none() { - return None; - } + value?; Some(value.unwrap().clone()) } diff --git a/src/function.rs b/src/function.rs index 3f6e6a2..d778299 100644 --- a/src/function.rs +++ b/src/function.rs @@ -16,7 +16,7 @@ impl InnerFunctionManager { pub fn new() -> Self { static STORE: OnceCell>>> = OnceCell::new(); let store = STORE.get_or_init(|| Mutex::new(HashMap::new())); - InnerFunctionManager { store: store } + InnerFunctionManager { store } } pub fn init(&mut self) { diff --git a/src/lib.rs b/src/lib.rs index 1cd8b5e..7a56571 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,7 +54,7 @@ pub fn execute(expr: &str, mut ctx: context::Context) -> Result { /// let ast = parse_expression(input); /// assert!(ast.is_ok()); /// ``` -pub fn parse_expression(expr: &str) -> Result { +pub fn parse_expression(expr: &str) -> Result> { init(); parser::Parser::new(expr)?.parse_stmt() } diff --git a/src/operator.rs b/src/operator.rs index 4d60616..d7c3340 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -49,7 +49,7 @@ impl InfixOpManager { pub fn new() -> Self { static STORE: OnceCell>> = OnceCell::new(); let store = STORE.get_or_init(|| Mutex::new(HashMap::new())); - InfixOpManager { store: store } + InfixOpManager { store } } pub fn init(&mut self) { @@ -309,7 +309,7 @@ impl InfixOpManager { let mut ans = vec![]; let binding = self.store.lock().unwrap(); for (op, InfixOpConfig(precedence, _, _, _)) in binding.iter() { - ans.push((op.clone(), precedence.clone())); + ans.push((op.clone(), *precedence)); } ans.sort_by(|a, b| a.1.cmp(&b.1)); ans @@ -325,7 +325,7 @@ impl PrefixOpManager { pub fn new() -> Self { static STORE: OnceCell>>> = OnceCell::new(); let store = STORE.get_or_init(|| Mutex::new(HashMap::new())); - PrefixOpManager { store: store } + PrefixOpManager { store } } pub fn init(&mut self) { @@ -423,7 +423,7 @@ impl PostfixOpManager { pub fn new() -> Self { static STORE: OnceCell>>> = OnceCell::new(); let store = STORE.get_or_init(|| Mutex::new(HashMap::new())); - Self { store: store } + Self { store } } pub fn init(&mut self) { diff --git a/src/parser.rs b/src/parser.rs index 155d116..b297533 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -148,7 +148,7 @@ impl<'a> ExprAST<'a> { } fn exec_unary(&self, op: &'a str, rhs: &ExprAST, ctx: &mut Context) -> Result { - PrefixOpManager::new().get(&op)?(rhs.exec(ctx)?) + PrefixOpManager::new().get(op)?(rhs.exec(ctx)?) } fn exec_binary( @@ -158,15 +158,15 @@ impl<'a> ExprAST<'a> { rhs: &ExprAST<'a>, ctx: &mut Context, ) -> Result { - match InfixOpManager::new().get_op_type(&op)? { + match InfixOpManager::new().get_op_type(op)? { InfixOpType::CALC => { - InfixOpManager::new().get_handler(&op)?(lhs.exec(ctx)?, rhs.exec(ctx)?) + InfixOpManager::new().get_handler(op)?(lhs.exec(ctx)?, rhs.exec(ctx)?) } InfixOpType::SETTER => { let (a, b) = (lhs.exec(ctx)?, rhs.exec(ctx)?); ctx.set_variable( lhs.get_reference_name()?, - InfixOpManager::new().get_handler(&op)?(a, b)?, + InfixOpManager::new().get_handler(op)?(a, b)?, ); Ok(Value::None) } @@ -262,7 +262,7 @@ impl<'a> ExprAST<'a> { "false".into() } } - String(value) => "\"".to_string() + &value + "\"", + String(value) => "\"".to_string() + value + "\"", } } @@ -292,7 +292,7 @@ impl<'a> ExprAST<'a> { let (is, precidence) = lhs.get_precidence(); let mut tmp: String = lhs.expr(); if is && precidence < InfixOpManager::new().get_precidence(op) { - tmp = "(".to_string() + &lhs.expr() + &")".to_string(); + tmp = "(".to_string() + &lhs.expr() + ")"; } tmp }; @@ -300,7 +300,7 @@ impl<'a> ExprAST<'a> { let (is, precidence) = rhs.get_precidence(); let mut tmp = rhs.expr(); if is && precidence < InfixOpManager::new().get_precidence(op) { - tmp = "(".to_string() + &rhs.expr() + &")".to_string(); + tmp = "(".to_string() + &rhs.expr() + ")"; } tmp }; @@ -318,36 +318,36 @@ impl<'a> ExprAST<'a> { fn list_expr(&self, params: Vec) -> String { let mut s = String::from("["); for i in 0..params.len() { - s.push_str(params[i].expr().as_str()); + s.push_str(¶ms[i].expr()); if i < params.len() - 1 { - s.push_str(","); + s.push(','); } } - s.push_str("]"); + s.push(']'); s } fn map_expr(&self, m: Vec<(ExprAST, ExprAST)>) -> String { let mut s = String::from("{"); for i in 0..m.len() { - let (key, value) = m[i].clone(); - s.push_str(key.expr().as_str()); - s.push_str(":"); - s.push_str(value.expr().as_str()); + let (key, value) = &m[i]; + s.push_str(&key.expr()); + s.push(':'); + s.push_str(&value.expr()); if i < m.len() - 1 { - s.push_str(","); + s.push(','); } } - s.push_str("}"); + s.push('}'); s } fn chain_expr(&self, exprs: Vec) -> String { let mut s = String::new(); for i in 0..exprs.len() { - s.push_str(exprs[i].expr().as_str()); + s.push_str(&exprs[i].expr()); if i < exprs.len() - 1 { - s.push_str(";"); + s.push(';'); } } s @@ -373,25 +373,25 @@ impl<'a> ExprAST<'a> { op.clone(), ), Self::List(values) => DescriptorManager::new().get_list_descriptor()( - values.into_iter().map(|v| v.describe()).collect(), + values.iter().map(|v| v.describe()).collect(), ), Self::Map(values) => DescriptorManager::new().get_map_descriptor()( values - .into_iter() + .iter() .map(|value| (value.0.describe(), value.1.describe())) .collect(), ), Self::Function(name, values) => DescriptorManager::new() .get_function_descriptor(name.to_string())( name.to_string(), - values.into_iter().map(|v| v.describe()).collect(), + values.iter().map(|v| v.describe()).collect(), ), Self::Reference(name) => DescriptorManager::new() .get_reference_descriptor(name.to_string())( name.to_string() ), Self::Stmt(values) => DescriptorManager::new().get_chain_descriptor()( - values.into_iter().map(|v| v.describe()).collect(), + values.iter().map(|v| v.describe()).collect(), ), Self::Ternary(condition, lhs, rhs) => { DescriptorManager::new().get_ternary_descriptor()( @@ -410,23 +410,21 @@ pub struct Parser<'a> { } impl<'a> Parser<'a> { - fn cur_tok(&self) -> Token { - self.tokenizer.cur_token.clone() + fn cur_tok(&self) -> Token<'_> { + self.tokenizer.cur_token } pub fn new(input: &'a str) -> Result { let mut tokenizer = Tokenizer::new(input); tokenizer.next()?; - Ok(Self { - tokenizer: tokenizer, - }) + Ok(Self { tokenizer }) } fn is_eof(&self) -> bool { self.cur_tok().is_eof() } - pub fn next(&mut self) -> Result { + pub fn next(&mut self) -> Result> { self.tokenizer.next() } diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 2c5a516..c4c828e 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -15,9 +15,9 @@ pub struct Tokenizer<'a> { } impl<'a> Tokenizer<'a> { - pub fn new(input: &str) -> Tokenizer { + pub fn new(input: &str) -> Tokenizer<'_> { Tokenizer { - input: input, + input, chars: input.char_indices(), cur_char: ' ', cur_token: Token::EOF, @@ -83,7 +83,7 @@ impl<'a> Tokenizer<'a> { } else if atom == "False" || atom == "false" { return self.bool_token(start, false); } - return self.function_or_reference_token(atom, start); + self.function_or_reference_token(atom, start) } fn try_parse_op(&self, start: usize) -> bool { @@ -114,10 +114,10 @@ impl<'a> Tokenizer<'a> { None => break, } } - return Ok(Token::Operator( + Ok(Token::Operator( self.input[start..self.current()].into(), Span(start, self.current()), - )); + )) } fn parse_var(&mut self, start: usize) -> (&'a str, usize) { @@ -136,12 +136,12 @@ impl<'a> Tokenizer<'a> { (self.input[start..self.current()].into(), start) } - pub fn peek(&self) -> Result { + pub fn peek(&self) -> Result> { self.clone().next() } pub fn expect(&mut self, op: &str) -> Result<()> { - let token = self.cur_token.clone(); + let token = self.cur_token; self.next()?; match token { Token::Delim(bracket, _) => { @@ -268,23 +268,23 @@ impl<'a> Tokenizer<'a> { } fn is_digit_char(ch: char) -> bool { - return '0' <= ch && ch <= '9' || ch == '.' || ch == '-' || ch == 'e' || ch == 'E' || ch == '+'; + ch.is_ascii_digit() || ch == '.' || ch == '-' || ch == 'e' || ch == 'E' || ch == '+' } fn is_whitespace_char(ch: char) -> bool { - return ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n'; + ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n' } fn is_delim_char(ch: char) -> bool { - return ch == '(' || ch == ')' || ch == '[' || ch == ']' || ch == '{' || ch == '}'; + ch == '(' || ch == ')' || ch == '[' || ch == ']' || ch == '{' || ch == '}' } fn is_param_char(ch: char) -> bool { - return ('0' <= ch && ch <= '9') - || ('a' <= ch && ch <= 'z') - || ('A' <= ch && ch <= 'Z') + ch.is_ascii_digit() + || ch.is_ascii_lowercase() + || ch.is_ascii_uppercase() || ch == '.' - || ch == '_'; + || ch == '_' } #[cfg(test)] From c66c016390a33447ac0292bfeff0f1d964a11052 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 06:51:45 +0000 Subject: [PATCH 3/3] Simplify descriptor.rs get() to use .cloned() directly Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/e76f8236-8653-4bc9-bf47-86b983ff48e3 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com> --- src/descriptor.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/descriptor.rs b/src/descriptor.rs index a518989..a9577b1 100644 --- a/src/descriptor.rs +++ b/src/descriptor.rs @@ -56,9 +56,7 @@ impl DescriptorManager { fn get(&self, key: DescriptorKey) -> Option { let binding = self.store.lock().unwrap(); - let value = binding.get(&key); - value?; - Some(value.unwrap().clone()) + binding.get(&key).cloned() } pub fn set_unary_descriptor(&mut self, op: String, descriptor: Arc) {