From 66b4d07714e1f9ee4612519ad418fbf8c6b49e28 Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Sun, 11 Aug 2024 10:13:11 -0700 Subject: [PATCH] Clippy + parking_lot --- Cargo.lock | 1 + Cargo.toml | 9 ++++++--- refuse-macros/Cargo.toml | 2 +- refuse-macros/src/lib.rs | 6 ++++++ refuse-pool/Cargo.toml | 3 ++- refuse-pool/src/lib.rs | 21 ++++++++++++++------- src/lib.rs | 39 +++++++++++++++++++++++++++------------ 7 files changed, 57 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fde0d92..80b4114 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -752,6 +752,7 @@ version = "0.0.5" dependencies = [ "ahash", "hashbrown", + "parking_lot", "refuse", ] diff --git a/Cargo.toml b/Cargo.toml index 63a2cf1..f6358d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,12 +16,11 @@ crossbeam-utils = "0.8.19" flume = "0.11.0" intentional = "0.1.1" kempt = "0.2.4" -parking_lot = { version = "0.12.1" } +parking_lot = { workspace = true } refuse-macros = { path = "./refuse-macros", version = "=0.0.5" } [lints] -rust.missing_docs = "warn" - +workspace = true [profile.release] debug = true @@ -35,3 +34,7 @@ members = ["benchmarks", "refuse-macros", "refuse-pool"] [workspace.lints] clippy.pedantic = "warn" +rust.missing_docs = "warn" + +[workspace.dependencies] +parking_lot = { version = "0.12.1" } diff --git a/refuse-macros/Cargo.toml b/refuse-macros/Cargo.toml index c7ceb23..764320d 100644 --- a/refuse-macros/Cargo.toml +++ b/refuse-macros/Cargo.toml @@ -18,4 +18,4 @@ quote = "1.0.35" proc-macro2 = "1.0.79" [lints] -clippy.pedantic = "warn" +workspace = true diff --git a/refuse-macros/src/lib.rs b/refuse-macros/src/lib.rs index 9d0be89..ba803c1 100644 --- a/refuse-macros/src/lib.rs +++ b/refuse-macros/src/lib.rs @@ -9,6 +9,9 @@ use proc_macro2::{Span, TokenStream}; use quote::quote; use syn::{GenericParam, Generics, Lifetime, TraitBound}; +/// Derives the `refuse::MapAs` trait for a given struct or enum. +/// +/// This macro expects the identifier `refuse` to refer to the crate. #[manyhow] #[proc_macro_derive(MapAs, attributes(map_as))] pub fn derive_map_as(input: syn::Item) -> manyhow::Result { @@ -53,6 +56,9 @@ fn derive_map_as_inner( } } +/// Derives the `refuse::Trace` trait for a given struct or enum. +/// +/// This macro expects the identifier `refuse` to refer to the crate. #[manyhow] #[proc_macro_derive(Trace, attributes(trace))] pub fn derive_trace(input: syn::Item) -> manyhow::Result { diff --git a/refuse-pool/Cargo.toml b/refuse-pool/Cargo.toml index 3c326a3..164015e 100644 --- a/refuse-pool/Cargo.toml +++ b/refuse-pool/Cargo.toml @@ -15,6 +15,7 @@ ahash = { version = "0.8.11", default-features = false, features = [ "runtime-rng", ] } hashbrown = "0.14.3" +parking_lot = { workspace = true } [lints] -rust.missing_docs = "warn" +workspace = true diff --git a/refuse-pool/src/lib.rs b/refuse-pool/src/lib.rs index af2358d..0346c2f 100644 --- a/refuse-pool/src/lib.rs +++ b/refuse-pool/src/lib.rs @@ -40,10 +40,11 @@ use std::borrow::Cow; use std::fmt::{Debug, Display}; use std::hash::{Hash, Hasher}; use std::ops::Deref; -use std::sync::{Mutex, OnceLock}; +use std::sync::OnceLock; use ahash::AHasher; use hashbrown::{hash_table, HashTable}; +use parking_lot::Mutex; use refuse::{AnyRef, AnyRoot, CollectionGuard, LocalPool, Ref, Root, SimpleType, Trace}; enum PoolEntry { @@ -92,7 +93,7 @@ impl StringPool { let hash = hash_str(key.as_ref()); match self .strings - .entry(hash, |a| a.equals(&key, guard), |e| e.hash()) + .entry(hash, |a| a.equals(&key, guard), PoolEntry::hash) { hash_table::Entry::Occupied(entry) => { let entry = entry.into_mut(); @@ -149,22 +150,25 @@ impl RootString { /// /// If another [`RootString`] or [`RefString`] exists already with the same /// contents as `s`, it will be returned and `s` will be dropped. - pub fn new<'a>(s: impl Into>, guard: impl AsRef>) -> Self { - let mut pool = StringPool::global().lock().expect("poisoned"); + pub fn new<'a>(s: impl Into>, guard: &impl AsRef>) -> Self { + let mut pool = StringPool::global().lock(); pool.intern(s.into(), guard.as_ref()).clone() } /// Returns a reference to this root string. + #[must_use] pub const fn downgrade(&self) -> RefString { RefString(self.0.downgrade()) } /// Returns a typeless reference to this string. + #[must_use] pub const fn downgrade_any(&self) -> AnyRef { self.0.downgrade_any() } /// Returns the number of root references to this string, `self` included. + #[must_use] pub fn root_count(&self) -> u64 { // We subtract one because the string pool always contains a reference. // Including it in the publicly viewable count would just lead to @@ -180,7 +184,7 @@ impl RootString { /// Returns `Err(root)` if `root` does not contain a pooled string. pub fn try_from_any<'a>( root: AnyRoot, - guard: impl AsRef>, + guard: &impl AsRef>, ) -> Result { Root::try_from_any(root, guard).map(Self) } @@ -191,7 +195,7 @@ impl Drop for RootString { if self.0.root_count() == 2 { // This is the last `RootString` aside from the one stored in the // pool, so we should remove the pool entry. - let mut pool = StringPool::global().lock().expect("poisoned"); + let mut pool = StringPool::global().lock(); let entry = pool .strings .find_entry(self.0.hash, |s| s == &self.0) @@ -345,7 +349,7 @@ impl RefString { /// contents as `s`, it will be returned and `s` will be dropped. pub fn new<'a>(s: impl Into>) -> Self { let guard = CollectionGuard::acquire(); - let mut pool = StringPool::global().lock().expect("poisoned"); + let mut pool = StringPool::global().lock(); pool.intern(s.into(), &guard).downgrade() } @@ -358,16 +362,19 @@ impl RefString { /// Loads a reference to the underlying string, if the string hasn't been /// freed. + #[must_use] pub fn load<'guard>(&self, guard: &'guard CollectionGuard) -> Option<&'guard str> { self.0.load(guard).map(|pooled| &*pooled.string) } /// Loads this string as a root, if the string hasn't been freed. + #[must_use] pub fn as_root(&self, guard: &CollectionGuard) -> Option { self.0.as_root(guard).map(RootString) } /// Returns a typeless reference to this string. + #[must_use] pub fn as_any(&self) -> AnyRef { self.0.as_any() } diff --git a/src/lib.rs b/src/lib.rs index fa5407d..383d1a5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -648,7 +648,7 @@ impl ThreadPool { } fn release_thread_guard() { - Self::map_current(Self::release_guard) + Self::map_current(Self::release_guard); } fn push_guard(&self) -> usize { @@ -1030,6 +1030,7 @@ impl CollectionGuard<'_> { /// /// If any other guards are currently held by this thread, this function /// does nothing. + #[allow(clippy::redundant_closure_for_method_calls)] // produces compiler error pub fn yield_to_collector(&mut self) { self.coordinated_yield(|yielder| yielder.wait()); } @@ -1056,10 +1057,16 @@ impl CollectionGuard<'_> { /// /// If any other guards are currently held by this thread, this function /// does nothing. - pub fn coordinated_yield(&mut self, yielder: impl FnOnce(Yielder<'_>) -> YieldComplete) { + pub fn coordinated_yield( + &mut self, + yielder: impl FnOnce(Yielder<'_>) -> YieldComplete, + ) -> bool { // We only need to attempt yielding if we are the outermost guard. if ThreadPool::current_depth() == 1 && self.collector.release_reader_if_collecting() { - let YieldComplete = yielder(Yielder(&mut self.collector)); + let _complete: YieldComplete = yielder(Yielder(&mut self.collector)); + true + } else { + false } } @@ -1122,15 +1129,18 @@ pub struct Yielder<'a>(&'a mut CollectorReadGuard); impl Yielder<'_> { /// Waits for the garbage collector to finish the current collection. + #[must_use] pub fn wait(self) -> YieldComplete { self.0.acquire_reader(); - YieldComplete + YieldComplete { _priv: PhantomData } } } /// A marker indicating that a [coordinated /// yield](CollectionGuard::coordinated_yield) has completed. -pub struct YieldComplete; +pub struct YieldComplete { + _priv: PhantomData<()>, +} /// A type that can be garbage collected. /// @@ -1571,7 +1581,7 @@ where /// Stores `value` in the garbage collector, returning a root reference to /// the data. - pub fn new<'a>(value: T, guard: impl AsRef>) -> Self { + pub fn new<'a>(value: T, guard: &impl AsRef>) -> Self { let guard = guard.as_ref(); let (type_index, gen, bin) = guard.adopt(Rooted::root(value)); Self::from_parts(type_index, gen, bin, guard) @@ -1584,10 +1594,10 @@ where /// Returns `Err(root)` if `root` does not contain a `T`. pub fn try_from_any<'a>( root: AnyRoot, - guard: impl AsRef>, + guard: &impl AsRef>, ) -> Result { if TypeIndex::of::() == root.any.type_index { - let slot = root.any.load_slot(guard.as_ref()).expect("root missing"); + let slot = root.any.load_slot(guard.as_ref()).assert("root missing"); Ok(Self { data: slot, reference: root.any.downcast_ref(), @@ -1630,6 +1640,7 @@ where } /// Returns this root as an untyped root. + #[must_use] pub fn into_any_root(self) -> AnyRoot { // We transfer ownership of this reference to the AnyRoot, so we want to // avoid calling drop on `self`. @@ -1772,7 +1783,7 @@ where T: Collectable + Hash, { fn hash(&self, state: &mut H) { - (**self).hash(state) + (**self).hash(state); } } @@ -1856,7 +1867,7 @@ where { /// Stores `value` in the garbage collector, returning a "weak" reference to /// it. - pub fn new<'a>(value: T, guard: impl AsRef>) -> Self { + pub fn new<'a>(value: T, guard: &impl AsRef>) -> Self { let guard = guard.as_ref(); let (type_index, slot_generation, bin_id) = guard.adopt(Rooted::reference(value)); @@ -2258,7 +2269,7 @@ where let slot = unsafe { &(*slot.value.get()).allocated }; slot.roots.fetch_add(1, Ordering::Relaxed); Some(AnyRoot { - rooted: (&**slot) as *const Rooted as *const (), + rooted: std::ptr::addr_of!(**slot).cast::<()>(), roots: &slot.roots, any: AnyRef { bin_id, @@ -2681,7 +2692,7 @@ enum SlotSweepStatus { /// yielding by the current thread when invoked. If a guard is held, consider /// calling [`CollectionGuard::collect()`] instead. pub fn collect() { - try_collect().unwrap() + try_collect().unwrap(); } /// Invokes the garbage collector. @@ -2766,6 +2777,7 @@ pub struct AnyRoot { impl AnyRoot { /// Loads a reference to the underlying data. Returns `None` if `T` is not /// the type of the underlying data. + #[must_use] pub fn load(&self) -> Option<&T> where T: Collectable, @@ -2783,6 +2795,7 @@ impl AnyRoot { } /// Returns a [`Root`] if the underlying reference points to a `T`. + #[must_use] pub fn downcast_root(&self) -> Option> where T: Collectable, @@ -2810,6 +2823,7 @@ impl AnyRoot { /// /// This function does not do any type checking. If `T` is not the correct /// type, attempting to load the underyling value will fail. + #[must_use] pub const fn downcast_ref(&self) -> Ref where T: Collectable, @@ -2827,6 +2841,7 @@ impl AnyRoot { } /// Returns an untyped "weak" reference to this root. + #[must_use] pub const fn as_any(&self) -> AnyRef { self.any }