Clean up and testing of expr macros (#392)

* Cleanup useless use of vec! * Check for failing behaviours in sparse and ref counted lambda reprs * Clean up unused imports * Clippy fix * Add allow for intentional non-canonical implementation of ord
Cypher1 · Feb 19, 2024 · c8ba0f1 · c8ba0f1
1 parent 034ba55
commit c8ba0f1
Show file tree

Hide file tree

Showing 17 changed files with 77 additions and 27 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/better-std/src/lib.rs b/better-std/src/lib.rs
@@ -1,10 +1,8 @@
 #[macro_use]
 pub mod map_macros;
-pub use map_macros::*;
 
 #[macro_use]
 pub mod todo;
-pub use todo::*;
 
 #[macro_use]
 pub mod more_pretty_assertions;

diff --git a/llamada/Cargo.toml b/llamada/Cargo.toml
@@ -13,3 +13,5 @@ include = ["src/**/*", "LICENSE.md", "README.md"]
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+pretty_assertions = "1.0"
+better-std = { path = "../better-std", features = [ ] }
diff --git a/llamada/src/macros.rs b/llamada/src/macros.rs
@@ -3,7 +3,7 @@ macro_rules! expr(
     { $ctx: expr, $final: ident, $( $name: ident = $ex: expr ),* $(,)? } => {
         {
             #[allow(unused_imports)]
-            use $crate::Term::*;
+            use $crate::{expr, Term::*};
             $( let $name = $ctx.add($ex); )*
             $final
         }
@@ -19,8 +19,8 @@ macro_rules! new_expr(
             let mut e = <$ty>::new($first_ex, Empty {});
             let $first = e.get_last_id();
             let f = expr!(e, $final, $( $name = $ex, )*);
-            *e.root_mut() = f;
-            e
+            *e.root_mut() = f.clone();
+            (e, f)
         }
     }
 );

diff --git a/llamada/src/reprs/ref_counted.rs b/llamada/src/reprs/ref_counted.rs
@@ -65,6 +65,7 @@ impl<
         }
     }
     fn get_last_id(&self) -> Self::Index {
+        // This clone is always cheap thanks to reference counting.
         self.terms.last().unwrap().clone()
     }
     fn get<'a>(&'a self, id: &'a Self::Index) -> &'a Term<Self::Value, Self::Index> {
@@ -93,7 +94,9 @@ impl<
         term: Term<Self::Value, Self::Index>,
         meta: Self::Meta,
     ) -> Self::Index {
-        Rc::new(Ptr::new(term, meta))
+        let node = Rc::new(Ptr::new(term, meta));
+        self.terms.push(node.clone());
+        node
     }
     fn print_meta(&self) -> bool {
         self.print_meta
@@ -107,5 +110,6 @@ pub type LambdaCalc = RcRepr<Never, Empty>;
 #[cfg(test)]
 mod tests {
     use super::*;
+    use better_std::assert_eq;
     tests!(LambdaCalc);
 }
diff --git a/llamada/src/reprs/sparse.rs b/llamada/src/reprs/sparse.rs
@@ -95,7 +95,10 @@ impl<
         term: Term<Self::Value, Self::Index>,
         meta: Self::Meta,
     ) -> Self::Index {
-        Ptr::new(term, meta)
+        let node = Ptr::new(term, meta);
+        // TODO: Consider a way to avoid this copy.
+        self.terms.push(node.clone());
+        node
     }
     fn print_meta(&self) -> bool {
         self.print_meta
@@ -109,5 +112,6 @@ pub type LambdaCalc = SparseRepr<Never, Empty>;
 #[cfg(test)]
 mod tests {
     use super::*;
+    use better_std::assert_eq;
     tests!(LambdaCalc);
 }
diff --git a/llamada/src/tests.rs b/llamada/src/tests.rs
@@ -171,6 +171,48 @@ macro_rules! tests {
             assert_eq!(expr.as_church(expr.root()), Some(2));
         }
 
+        #[test]
+        fn plus_expr_using_macros() {
+            let (mut expr, plus) = $crate::new_expr!(
+                $ty,
+                plus,
+                x = Var(1),
+                f = Var(2),
+                m = Var(3),
+                n = Var(4),
+                nf = App(n.clone(), f.clone()),
+                mf = App(m.clone(), f.clone()),
+                mfx = App(mf, x.clone()),
+                nfmfx = App(nf.clone(), mfx.clone()),
+                abs1_nfmfx = Term::abs(nfmfx),
+                abs2_nfmfx = Term::abs(abs1_nfmfx),
+                abs3_nfmfx = Term::abs(abs2_nfmfx),
+                plus = Term::abs(abs3_nfmfx)
+            );
+
+            assert_eq!(
+                format!("{}", &expr),
+                "(a => (b => (c => (d => ((a c) ((b c) d))))))"
+            );
+            for n in 0..10 {
+                for m in 0..10 {
+                    let church_n = expr.to_church(n);
+                    let church_m = expr.to_church(m);
+
+                    let plus_n_m = $crate::expr!(
+                        &mut expr,
+                        plus_n_m,
+                        plus_m = App(plus.clone(), church_m),
+                        plus_n_m = App(plus_m, church_n)
+                    );
+                    expr.reduce();
+                    let result = expr.as_church(&plus_n_m);
+                    eprintln!("{n:?} + {m:?} = {result:?}");
+                    assert_eq!(result, Some(n + m));
+                }
+            }
+        }
+
         #[test]
         fn plus_expr() {
             let mut expr = <$ty>::new(Term::Var(1), Empty);

diff --git a/llamada/src/type_checking.rs b/llamada/src/type_checking.rs
@@ -327,7 +327,7 @@ mod test {
 
     #[test]
     fn simple_type_system_with_no_collapsing() {
-        let expr = new_expr!(
+        let (expr, _) = new_expr!(
             LambdaCalc,
             p_a_b,
             a = ext(3),
@@ -350,7 +350,7 @@ mod test {
 
     #[test]
     fn simply_typed() {
-        let expr = new_expr!(
+        let (expr, _) = new_expr!(
             LambdaCalc,
             p_a_b,
             a = ext(3),

diff --git a/llamada/src/visitors/compact_to_church.rs b/llamada/src/visitors/compact_to_church.rs
@@ -192,7 +192,7 @@ mod test {
             output: LambdaCalc::new(Term::Var(1), Empty {}),
         };
 
-        let expr = new_expr!(
+        let (expr, _) = new_expr!(
             LambdaCalc,
             p_a_b,
             a = Ext(3.into()),
@@ -216,7 +216,7 @@ mod test {
 
     #[test]
     fn arity_checker() {
-        let expr = new_expr!(
+        let (expr, _) = new_expr!(
             LambdaCalc,
             p_a_b,
             a = ext(3),

diff --git a/llamada/src/visitors/transform_meta.rs b/llamada/src/visitors/transform_meta.rs
@@ -240,7 +240,7 @@ mod test {
 
     #[test]
     fn arity_checker() {
-        let expr = new_expr!(
+        let (expr, _) = new_expr!(
             LambdaCalc,
             p_a_b,
             a = ext(3),

diff --git a/takolib/src/codegen/backend/llvm.rs b/takolib/src/codegen/backend/llvm.rs
@@ -66,6 +66,7 @@ impl<'ctx> Backend<'ctx> for Llvm<'ctx> {
             reloc: RelocMode::Default,
             model: CodeModel::Default,
             opt: OptimizationLevel::Default,
+            #[allow(clippy::arc_with_non_send_sync)]
             target_triple: Arc::new(TargetMachine::get_default_triple()),
             target_machine: None,
         };

diff --git a/takolib/src/interpreter.rs b/takolib/src/interpreter.rs
@@ -60,7 +60,7 @@ pub fn run(path: &Path, ast: &Ast, root: Option<NodeId>) -> Result<Prim, TError>
 
 impl<'a> Ctx<'a> {
     pub fn eval2(&mut self, args: &[NodeId]) -> Result<[Prim; 2], TError> {
-        let l = args.get(0).expect("requires a left argument");
+        let l = args.first().expect("requires a left argument");
         let r = args.get(1).expect("requires a right argument");
         let l = self.eval(*l)?;
         let r = self.eval(*r)?;
@@ -146,7 +146,7 @@ impl<'a> Ctx<'a> {
                 } else {
                     let arg = self.eval(
                         *op.args
-                            .get(0)
+                            .first()
                             .expect("Sub should have at least one operand"),
                     )?;
                     match arg {
@@ -226,7 +226,7 @@ impl<'a> Ctx<'a> {
             Symbol::HasType => todo!(),
             Symbol::Arrow | Symbol::DoubleArrow => {
                 // TODO(clarity): Type arrow vs value arrow?
-                let Some(_l) = op.args.get(0) else {
+                let Some(_l) = op.args.first() else {
                     panic!("-> expects a left and a right. Left not found");
                 };
                 let Some(r) = op.args.get(1) else {
@@ -245,7 +245,7 @@ impl<'a> Ctx<'a> {
             Symbol::Spread => todo!(),
             Symbol::Comma => todo!(),
             Symbol::Sequence => {
-                let Some(l) = op.args.get(0) else {
+                let Some(l) = op.args.first() else {
                     panic!("; expects a left and a right. Neither found");
                 };
                 let Some(r) = op.args.get(1) else {

diff --git a/takolib/src/lib.rs b/takolib/src/lib.rs
@@ -60,7 +60,6 @@ pub fn ensure_initialized() {
     use std::fs::OpenOptions;
     build_logger(|env| {
         let log_file = OpenOptions::new()
-            .write(true)
             .append(true)
             .create(true)
             .open(".tako.log")

diff --git a/takolib/src/primitives/meta.rs b/takolib/src/primitives/meta.rs
@@ -35,6 +35,7 @@ impl<T> std::hash::Hash for Meta<T> {
 }
 
 impl<T> PartialOrd for Meta<T> {
+    #[allow(clippy::non_canonical_partial_ord_impl)]
     fn partial_cmp(&self, _other: &Self) -> Option<std::cmp::Ordering> {
         Some(std::cmp::Ordering::Equal)
     }

diff --git a/takolib/src/primitives/typed_index.rs b/takolib/src/primitives/typed_index.rs
@@ -115,6 +115,7 @@ impl<T, Idx: std::fmt::Debug + std::convert::TryInto<usize>, Container: IndexMut
 impl<T, Idx: std::fmt::Debug + std::convert::TryInto<usize> + std::convert::TryFrom<usize>>
     TypedIndex<T, Idx, Vec<T>>
 {
+    #[allow(clippy::ptr_arg)]
     pub fn next(container: &Vec<T>) -> Result<Self, <Idx as std::convert::TryFrom<usize>>::Error> {
         Ok(Self::from_raw(Idx::try_from(container.len())?))
     }

diff --git a/takolib/src/tasks/manager.rs b/takolib/src/tasks/manager.rs
@@ -45,7 +45,7 @@ impl std::fmt::Display for TaskStats {
         let num_real = num_requests - num_already_running;
         let num_done = num_succeeded + num_cached;
         write!(f, "{num_done}/{num_real}")?;
-        let items: Vec<String> = vec![(num_cached, "cached"), (num_failed, "failed")]
+        let items: Vec<String> = [(num_cached, "cached"), (num_failed, "failed")]
             .iter()
             .filter(|(n, _label)| **n > 0)
             .map(|(n, label)| format!("{n} {label}"))
@@ -143,10 +143,7 @@ impl<T: Debug + Task + 'static> TaskManager<T> {
             Self::name()
         );
         let task_id = task.get_hash();
-        let current_results = self
-            .result_store
-            .entry(task_id)
-            .or_insert_with(TaskStatus::new);
+        let current_results = self.result_store.entry(task_id).or_default();
         let mut is_complete = false;
         let mut error = None;
         let results_so_far = &mut current_results.results;
@@ -197,10 +194,7 @@ impl<T: Debug + Task + 'static> TaskManager<T> {
     ) {
         // Get a new job from 'upstream'.
         self.stats.num_requests += 1;
-        let status = self
-            .result_store
-            .entry(task.get_hash())
-            .or_insert_with(TaskStatus::new);
+        let status = self.result_store.entry(task.get_hash()).or_default();
         if task.invalidate() {
             *status = TaskStatus::new(); // Forget the previous value!
         }

diff --git a/takolib/src/ui/client.rs b/takolib/src/ui/client.rs
@@ -93,7 +93,7 @@ impl Client {
                 trace!("TaskManager status: {kind:?} => {stats}\nerrors: {errors:#?}");
                 for (_id, err) in errors {
                     let file = err.location.as_ref().map(|loc| loc.filename.clone());
-                    let errs = self.errors_for_file.entry(file).or_insert_with(BTreeSet::new);
+                    let errs = self.errors_for_file.entry(file).or_default();
                     errs.insert(err);
                 }
                 self.manager_status.insert(kind, stats);