From 48416291339778540bdde93dafb444dc90abe928 Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Wed, 11 Dec 2024 16:37:02 -0500 Subject: [PATCH 1/2] Fix clippy warnings The notable functional change here is that the "Public API" comment is turned from a doc comment into a regular comment. Previously it (along with all the forward slashes) was displaying awkwardly in the docs. Clippy was right in this case that the empty line indicated that it was not intended to attach to the item below. --- src/lib.rs | 2 ++ src/snapshot_vec.rs | 6 +++--- src/undo_log.rs | 2 +- src/unify/mod.rs | 26 +++++++++++++------------- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index aed3524..e8c6eed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,8 @@ //! details. #![cfg_attr(feature = "bench", feature(test))] +// This would add is_empty() functions to public traits +#![allow(clippy::len_without_is_empty)] #[macro_use] extern crate log; diff --git a/src/snapshot_vec.rs b/src/snapshot_vec.rs index efde49a..90f461f 100644 --- a/src/snapshot_vec.rs +++ b/src/snapshot_vec.rs @@ -165,10 +165,10 @@ impl + Default, L: Default> SnapshotVec SnapshotVecStorage { /// Creates a `SnapshotVec` using the `undo_log`, allowing mutating methods to be called - pub fn with_log<'a, L>( - &'a mut self, + pub fn with_log( + &mut self, undo_log: L, - ) -> SnapshotVec::Value>, L> + ) -> SnapshotVec::Value>, L> where L: UndoLogs>, { diff --git a/src/undo_log.rs b/src/undo_log.rs index 0d6d2f6..839b767 100644 --- a/src/undo_log.rs +++ b/src/undo_log.rs @@ -40,7 +40,7 @@ pub trait UndoLogs { } } -impl<'a, T, U> UndoLogs for &'a mut U +impl UndoLogs for &mut U where U: UndoLogs, { diff --git a/src/unify/mod.rs b/src/unify/mod.rs index 41c9d46..44fc668 100644 --- a/src/unify/mod.rs +++ b/src/unify/mod.rs @@ -211,9 +211,9 @@ impl VarValue { fn new(parent: K, value: K::Value, rank: u32) -> VarValue { VarValue { - parent: parent, // this is a root - value: value, - rank: rank, + parent, // this is a root + value, + rank, } } @@ -233,10 +233,10 @@ where { /// Creates a `UnificationTable` using an external `undo_log`, allowing mutating methods to be /// called if `L` does not implement `UndoLogs` - pub fn with_log<'a, L>( - &'a mut self, + pub fn with_log( + &mut self, undo_log: L, - ) -> UnificationTable, L>> + ) -> UnificationTable, L>> where L: UndoLogs>>, { @@ -324,7 +324,7 @@ impl UnificationTable { /// the closure. pub fn reset_unifications(&mut self, mut value: impl FnMut(S::Key) -> S::Value) { self.values.reset_unifications(|i| { - let key = UnifyKey::from_index(i as u32); + let key = UnifyKey::from_index(i); let value = value(key); VarValue::new_var(key, value) }); @@ -443,8 +443,7 @@ impl UnificationTable { } } -/// //////////////////////////////////////////////////////////////////////// -/// Public API +// Public API impl UnificationTable where @@ -535,7 +534,8 @@ where let combined = V::unify_values(&self.value(root_a).value, &self.value(root_b).value)?; - Ok(self.unify_roots(root_a, root_b, combined)) + self.unify_roots(root_a, root_b, combined); + Ok(()) } /// Sets the value of the key `a_id` to `b`, attempting to merge @@ -587,9 +587,9 @@ impl UnifyValue for Option { fn unify_values(a: &Option, b: &Option) -> Result { match (a, b) { - (&None, &None) => Ok(None), - (&Some(ref v), &None) | (&None, &Some(ref v)) => Ok(Some(v.clone())), - (&Some(ref a), &Some(ref b)) => match V::unify_values(a, b) { + (None, None) => Ok(None), + (Some(v), None) | (None, Some(v)) => Ok(Some(v.clone())), + (Some(a), Some(b)) => match V::unify_values(a, b) { Ok(v) => Ok(Some(v)), Err(err) => Err(err), }, From a3b756c18ff54b08d633d4b7adfa30506858e5ab Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Thu, 12 Dec 2024 10:54:19 -0500 Subject: [PATCH 2/2] Fix clippy lints on tests --- src/unify/tests.rs | 20 ++++++++++---------- tests/external_undo_log.rs | 16 ++++------------ 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/unify/tests.rs b/src/unify/tests.rs index 5665aba..04c1cc9 100644 --- a/src/unify/tests.rs +++ b/src/unify/tests.rs @@ -40,6 +40,8 @@ impl UnifyKey for UnitKey { macro_rules! all_modes { ($name:ident for $t:ty => $body:tt) => { + // The $name field is only needed by some callers + #[allow(clippy::extra_unused_type_parameters)] fn test_body< $name: Clone + Default + UnificationStore::Value>, >() { @@ -60,9 +62,9 @@ fn basic() { let mut ut: UnificationTable = UnificationTable::new(); let k1 = ut.new_key(()); let k2 = ut.new_key(()); - assert_eq!(ut.unioned(k1, k2), false); + assert!(!ut.unioned(k1, k2)); ut.union(k1, k2); - assert_eq!(ut.unioned(k1, k2), true); + assert!(ut.unioned(k1, k2)); } } } @@ -386,12 +388,10 @@ impl UnifyKey for OrderedKey { b_rank: &OrderedRank, ) -> Option<(OrderedKey, OrderedKey)> { println!("{:?} vs {:?}", a_rank, b_rank); - if a_rank > b_rank { - Some((a, b)) - } else if b_rank > a_rank { - Some((b, a)) - } else { - None + match a_rank.cmp(b_rank) { + cmp::Ordering::Greater => Some((a, b)), + cmp::Ordering::Less => Some((b, a)), + cmp::Ordering::Equal => None, } } } @@ -424,7 +424,7 @@ fn ordered_key() { ut.union(k0_5, k0_6); // rank of new root now 1 ut.union(k0_1, k0_5); // new root rank 2, should not be k0_5 or k0_6 - assert!(vec![k0_1, k0_2, k0_3, k0_4].contains(&ut.find(k0_1))); + assert!([k0_1, k0_2, k0_3, k0_4].contains(&ut.find(k0_1))); } } } @@ -450,7 +450,7 @@ fn ordered_key_k1() { ut.union(k0_1, k1_5); // even though k1 has lower rank, it wins assert!( - vec![k1_5, k1_6].contains(&ut.find(k0_1)), + [k1_5, k1_6].contains(&ut.find(k0_1)), "unexpected choice for root: {:?}", ut.find(k0_1) ); diff --git a/tests/external_undo_log.rs b/tests/external_undo_log.rs index 2537826..ff2164a 100644 --- a/tests/external_undo_log.rs +++ b/tests/external_undo_log.rs @@ -80,7 +80,7 @@ struct TypeVariableTable<'a> { } impl TypeVariableTable<'_> { - fn new(&mut self, i: i32) -> IntKey { + fn new_key(&mut self, i: i32) -> IntKey { self.storage.values.with_log(&mut self.undo_log).push(i); self.storage .eq_relations @@ -93,20 +93,12 @@ struct Snapshot { undo_len: usize, } +#[derive(Default)] struct TypeVariableUndoLogs { logs: Vec, num_open_snapshots: usize, } -impl Default for TypeVariableUndoLogs { - fn default() -> Self { - Self { - logs: Default::default(), - num_open_snapshots: Default::default(), - } - } -} - impl UndoLogs for TypeVariableUndoLogs where UndoLog: From, @@ -193,8 +185,8 @@ fn external_undo_log() { let mut undo_log = TypeVariableUndoLogs::default(); let snapshot = undo_log.start_snapshot(); - storage.with_log(&mut undo_log).new(1); - storage.with_log(&mut undo_log).new(2); + storage.with_log(&mut undo_log).new_key(1); + storage.with_log(&mut undo_log).new_key(2); assert_eq!(storage.len(), 2); undo_log.rollback_to(|| &mut storage, snapshot);