From c8ba0f1c202bb133fe7d6fcb6ca4509e17574d12 Mon Sep 17 00:00:00 2001 From: Jay Pratt Date: Mon, 19 Feb 2024 19:25:22 +1100 Subject: [PATCH] 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 --- Cargo.lock | 4 +++ better-std/src/lib.rs | 2 -- llamada/Cargo.toml | 2 ++ llamada/src/macros.rs | 6 ++-- llamada/src/reprs/ref_counted.rs | 6 +++- llamada/src/reprs/sparse.rs | 6 +++- llamada/src/tests.rs | 42 +++++++++++++++++++++++ llamada/src/type_checking.rs | 4 +-- llamada/src/visitors/compact_to_church.rs | 4 +-- llamada/src/visitors/transform_meta.rs | 2 +- takolib/src/codegen/backend/llvm.rs | 1 + takolib/src/interpreter.rs | 8 ++--- takolib/src/lib.rs | 1 - takolib/src/primitives/meta.rs | 1 + takolib/src/primitives/typed_index.rs | 1 + takolib/src/tasks/manager.rs | 12 ++----- takolib/src/ui/client.rs | 2 +- 17 files changed, 77 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f68ce11..eefae263 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1116,6 +1116,10 @@ dependencies = [ [[package]] name = "llamada" version = "0.1.0" +dependencies = [ + "better-std", + "pretty_assertions", +] [[package]] name = "llvm-sys" diff --git a/better-std/src/lib.rs b/better-std/src/lib.rs index 04baf5ee..421148e0 100644 --- 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 index 91e5b277..2a3b4a5f 100644 --- 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 index 317da00a..e4cf44a4 100644 --- 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 index 44a06794..bbb847cb 100644 --- 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 { @@ -93,7 +94,9 @@ impl< term: Term, 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; #[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 index 180d69a1..9ed58c40 100644 --- a/llamada/src/reprs/sparse.rs +++ b/llamada/src/reprs/sparse.rs @@ -95,7 +95,10 @@ impl< term: Term, 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; #[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 index e9d1f4ac..99a0776e 100644 --- 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 index 9b052b98..89d4bbe8 100644 --- 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 index 7450a667..0da463f3 100644 --- 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 index 9944acdf..618e2915 100644 --- 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 index 1e8a31c5..88f57a3b 100644 --- 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 index 1f516c25..1763d69e 100644 --- a/takolib/src/interpreter.rs +++ b/takolib/src/interpreter.rs @@ -60,7 +60,7 @@ pub fn run(path: &Path, ast: &Ast, root: Option) -> Result 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 index f9c1194d..74adc332 100644 --- 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 index 4891fe00..9c149cac 100644 --- a/takolib/src/primitives/meta.rs +++ b/takolib/src/primitives/meta.rs @@ -35,6 +35,7 @@ impl std::hash::Hash for Meta { } impl PartialOrd for Meta { + #[allow(clippy::non_canonical_partial_ord_impl)] fn partial_cmp(&self, _other: &Self) -> Option { Some(std::cmp::Ordering::Equal) } diff --git a/takolib/src/primitives/typed_index.rs b/takolib/src/primitives/typed_index.rs index 1e48f9a0..7a17fcd1 100644 --- a/takolib/src/primitives/typed_index.rs +++ b/takolib/src/primitives/typed_index.rs @@ -115,6 +115,7 @@ impl, Container: IndexMut impl + std::convert::TryFrom> TypedIndex> { + #[allow(clippy::ptr_arg)] pub fn next(container: &Vec) -> Result>::Error> { Ok(Self::from_raw(Idx::try_from(container.len())?)) } diff --git a/takolib/src/tasks/manager.rs b/takolib/src/tasks/manager.rs index 2b0f6672..dc946e7a 100644 --- 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 = vec![(num_cached, "cached"), (num_failed, "failed")] + let items: Vec = [(num_cached, "cached"), (num_failed, "failed")] .iter() .filter(|(n, _label)| **n > 0) .map(|(n, label)| format!("{n} {label}")) @@ -143,10 +143,7 @@ impl TaskManager { 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 TaskManager { ) { // 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 index f2ffb8e9..5b60130a 100644 --- 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);