From 1c235aa6fa87440db61bff75ab95234c060408a3 Mon Sep 17 00:00:00 2001
From: Rute Figueiredo <rfigueiredo.dev@gmail.com>
Date: Thu, 1 Aug 2024 11:01:51 +0100
Subject: [PATCH] fix clippy

---
 src/compiler/cse.rs | 29 ++++++++++++++++-------------
 src/poly/cse.rs     | 27 ++++++++-------------------
 2 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/src/compiler/cse.rs b/src/compiler/cse.rs
index 6701f22d..beea80f0 100644
--- a/src/compiler/cse.rs
+++ b/src/compiler/cse.rs
@@ -25,6 +25,7 @@ use crate::{
 /// Equivalent expressions are found by hashing the expressions with random assignments to the
 /// queriables. Using the Schwartz-Zippel lemma, we can determine if two expressions are equivalent
 /// with high probability.
+#[allow(dead_code)]
 pub(super) fn cse<F: Field + Hash>(
     mut circuit: SBPIR<F, NullTraceGenerator>,
 ) -> SBPIR<F, NullTraceGenerator> {
@@ -36,11 +37,11 @@ pub(super) fn cse<F: Field + Hash>(
             let mut queriables = Vec::<Queriable<F>>::new();
 
             circuit.forward_signals.iter().for_each(|signal| {
-                queriables.push(Queriable::Forward(signal.clone(), false));
-                queriables.push(Queriable::Forward(signal.clone(), true));
+                queriables.push(Queriable::Forward(*signal, false));
+                queriables.push(Queriable::Forward(*signal, true));
             });
             step_type.signals.iter().for_each(|signal| {
-                queriables.push(Queriable::Internal(signal.clone()));
+                queriables.push(Queriable::Internal(*signal));
             });
 
             // Generate random assignments for the queriables
@@ -75,18 +76,14 @@ pub(super) fn cse<F: Field + Hash>(
                 // Add the new signal to the step type and a constraint for it
                 decomp.auto_signals.iter().for_each(|(q, expr)| {
                     if let Queriable::Internal(signal) = q {
-                        step_type_with_hash.add_internal(signal.clone());
+                        step_type_with_hash.add_internal(*signal);
                     }
-                    step_type_with_hash
-                        .auto_signals
-                        .insert(q.clone(), expr.clone());
+                    step_type_with_hash.auto_signals.insert(*q, expr.clone());
                     step_type_with_hash.add_constr(format!("{:?}", q), expr.clone());
                 });
 
                 // Replace the common subexpression in all constraints
-                step_type_with_hash.decomp_constraints(|expr| {
-                    replace_expr(expr, &common_se, &mut signal_factory, decomp.clone())
-                });
+                step_type_with_hash.decomp_constraints(|expr| replace_expr(expr, &common_se));
             } else {
                 // No more common subexpressions found, exit the loop
                 break;
@@ -117,7 +114,7 @@ impl SubexprInfo {
 
 /// Find the optimal subexpression to replace in a list of expressions.
 fn find_optimal_subexpression<F: Field + Hash>(
-    exprs: &Vec<Expr<F, Queriable<F>, HashResult>>,
+    exprs: &[Expr<F, Queriable<F>, HashResult>],
     replaced_hashes: &HashSet<u64>,
 ) -> Option<Expr<F, Queriable<F>, HashResult>> {
     let mut count_map = HashMap::<u64, SubexprInfo>::new();
@@ -314,8 +311,14 @@ mod test {
             .auto_signals
             .values();
 
-        assert!(common_ses_found_and_replaced.clone().find(|expr| format!("{:?}", expr) == "(a * b)").is_some());
-        assert!(common_ses_found_and_replaced.clone().find(|expr| format!("{:?}", expr) == "(e * f * d)").is_some());
+        assert!(common_ses_found_and_replaced
+            .clone()
+            .find(|expr| format!("{:?}", expr) == "(a * b)")
+            .is_some());
+        assert!(common_ses_found_and_replaced
+            .clone()
+            .find(|expr| format!("{:?}", expr) == "(e * f * d)")
+            .is_some());
     }
 
     #[derive(Clone)]
diff --git a/src/poly/cse.rs b/src/poly/cse.rs
index b1f95ab2..ab53474e 100644
--- a/src/poly/cse.rs
+++ b/src/poly/cse.rs
@@ -4,14 +4,11 @@ use super::{ConstrDecomp, Expr, HashResult, SignalFactory};
 use std::{fmt::Debug, hash::Hash};
 
 /// This function replaces a common subexpression in an expression with a new signal.
-pub fn replace_expr<F: Field + Hash, V: Clone + Eq + Hash + Debug, SF: SignalFactory<V>>(
+pub fn replace_expr<F: Field + Hash, V: Clone + Eq + Hash + Debug>(
     expr: &Expr<F, V, HashResult>,
     common_se: &Expr<F, V, HashResult>,
-    signal_factory: &mut SF,
-    decomp: ConstrDecomp<F, V, HashResult>,
 ) -> (Expr<F, V, HashResult>, ConstrDecomp<F, V, HashResult>) {
-    let mut decomp = decomp;
-    let new_expr = replace_subexpr(expr, common_se, signal_factory, &mut decomp);
+    let new_expr = replace_subexpr(expr, common_se);
 
     (new_expr, ConstrDecomp::default())
 }
@@ -33,26 +30,23 @@ pub fn create_common_ses_signal<
 }
 
 /// This function replaces a common subexpression in an expression with a new signal.
-fn replace_subexpr<F: Field + Hash, V: Clone + Eq + Hash + Debug, SF: SignalFactory<V>>(
+fn replace_subexpr<F: Field + Hash, V: Clone + Eq + Hash + Debug>(
     expr: &Expr<F, V, HashResult>,
     common_se: &Expr<F, V, HashResult>,
-    signal_factory: &mut SF,
-    decomp: &mut ConstrDecomp<F, V, HashResult>,
 ) -> Expr<F, V, HashResult> {
     let common_expr_hash = common_se.meta().hash;
 
     if expr.meta().degree < common_se.meta().degree {
         // If the current expression's degree is less than the common subexpression's degree,
         // it can't contain the common subexpression, so we return it as is
-        return expr.clone();
+        expr.clone()
     }
-
     // If the expression is the same as the common subexpression return the signal
-    if expr.meta().hash == common_expr_hash {
-        return common_se.clone();
+    else if expr.meta().hash == common_expr_hash {
+        common_se.clone()
     } else {
         // Recursively apply the function to the subexpressions
-        expr.apply_subexpressions(|se| replace_subexpr(se, common_se, signal_factory, decomp))
+        expr.apply_subexpressions(|se| replace_subexpr(se, common_se))
     }
 }
 
@@ -102,12 +96,7 @@ mod tests {
         let (common_se, decomp) =
             create_common_ses_signal(&common_expr.hash(&assignments), &mut signal_factory);
 
-        let (new_expr, _) = replace_expr(
-            &expr.hash(&assignments),
-            &common_se,
-            &mut signal_factory,
-            decomp.clone(),
-        );
+        let (new_expr, _) = replace_expr(&expr.hash(&assignments), &common_se);
 
         assert!(decomp.auto_signals.len() == 1);
         assert_eq!(format!("{:#?}", new_expr), "((-0x1) + cse-1 + (-c))");