Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn on pedantic clippy lints where possible #886

Merged
merged 38 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
794d557
Deny clippy::all lint group
casey Nov 1, 2024
bf47dbc
clippy::drop_non_drop
casey Nov 1, 2024
d72dd11
clippy::cast_lossless
casey Nov 1, 2024
32bca32
clippy::checked_conversions
casey Nov 1, 2024
8956d78
clippy::cloned_instead_of_copied
casey Nov 1, 2024
e73ff5d
clippy::default_trait_access
casey Nov 1, 2024
d8dc12f
clippy::doc_markdown
casey Nov 1, 2024
bba2cd5
Ignore benchmark and test temporary files
casey Nov 1, 2024
40f8cb5
clippy::explicit_iter_loop
casey Nov 1, 2024
6d1e4dd
Allow clippy::if_not_else
casey Nov 1, 2024
7b6f0c1
Allow clippy::inline_always
casey Nov 1, 2024
350bff5
Allow clippy::iter_not_returning_iterator
casey Nov 1, 2024
0ac4c45
Allow clippy::let_underscore_drop
casey Nov 1, 2024
184710d
Deny clippy::map_unwrap_or
casey Nov 1, 2024
8c89ad2
Allow clippy::missing_errors_doc and clippy::missing_panics_doc
casey Nov 1, 2024
5133f2c
Deny clippy::match_wildcard_for_single_variants
casey Nov 1, 2024
1b00918
Allow clippy::module_name_repetitions
casey Nov 1, 2024
7b38788
Allow clippy::needless_pass_by_value
casey Nov 1, 2024
336cfe1
Allow clippy::option_option
casey Nov 1, 2024
94e04f5
Deny clippy::range_plus_one
casey Nov 1, 2024
ada26a6
Deny clippy::type_repetition_in_bounds
casey Nov 1, 2024
4afb26d
Deny clippy::uninlined_format_args
casey Nov 1, 2024
b29613c
Allow clippy::unnecessary_wraps
casey Nov 1, 2024
39f2774
Deny clippy::semicolon_if_nothing_returned
casey Nov 1, 2024
9457f4f
Allow clippy::too_many_lines
casey Nov 1, 2024
29cbee0
Allow clippy::similar_names
casey Nov 1, 2024
39de8d7
Allow clippy::wildcard_imports
casey Nov 1, 2024
97c5ece
Allow clippy::unreadable_literal
casey Nov 1, 2024
7ddfcae
Deny clippy::redundant_else
casey Nov 1, 2024
e807a45
Deny clippy::unused_self
casey Nov 1, 2024
9165919
Allow clippy::must_use_candidate
casey Nov 1, 2024
a8f6294
Deny clippy::match_same_arms
casey Nov 1, 2024
be75847
Deny clippy::trivially_copy_pass_by_ref
casey Nov 1, 2024
b3299f5
Allow clippy::redundant_closure_for_method_calls
casey Nov 1, 2024
185c262
Deny clippy::transmute_ptr_to_ptr
casey Nov 1, 2024
5c13a93
Sort and clean up allow and deny blocks
casey Nov 1, 2024
d2f12e8
Use pointer::cast
casey Nov 3, 2024
e78e8f2
Revert changes to xxh3 and allow lints
casey Nov 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ Cargo.lock
# Profiling
perf.data*
flamegraph.svg

# benchmark and test temporary files
/.tmp*
6 changes: 2 additions & 4 deletions src/complex_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ fn encode_varint_len(len: usize, output: &mut Vec<u8>) {
} else if len <= u16::MAX.into() {
let u16_len: u16 = len.try_into().unwrap();
output.push(254);
output.extend_from_slice(&u16_len.to_le_bytes())
output.extend_from_slice(&u16_len.to_le_bytes());
} else {
assert!(len <= u32::MAX as usize);
let u32_len: u32 = len.try_into().unwrap();
output.push(255);
output.extend_from_slice(&u32_len.to_le_bytes())
output.extend_from_slice(&u32_len.to_le_bytes());
}
}

Expand Down Expand Up @@ -67,7 +66,6 @@ impl<T: Value> Value for Vec<T> {

fn as_bytes<'a, 'b: 'a>(value: &'a Vec<T::SelfType<'b>>) -> Vec<u8>
where
Self: 'a,
Self: 'b,
{
let mut result = if let Some(width) = T::fixed_width() {
Expand Down
8 changes: 4 additions & 4 deletions src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,9 +453,9 @@ impl Database {

if !progress {
break;
} else {
compacted = true;
}

compacted = true;
}

Ok(compacted)
Expand Down Expand Up @@ -804,7 +804,7 @@ impl RepairSession {
self.aborted
}

/// Abort the repair process. The coorresponding call to [Builder::open] or [Builder::create] will return an error
/// Abort the repair process. The coorresponding call to [`Builder::open`] or [`Builder::create`] will return an error
pub fn abort(&mut self) {
self.aborted = true;
}
Expand Down Expand Up @@ -852,7 +852,7 @@ impl Builder {
/// Set a callback which will be invoked periodically in the event that the database file needs
/// to be repaired.
///
/// The [RepairSession] argument can be used to control the repair process.
/// The [`RepairSession`] argument can be used to control the repair process.
///
/// If the database file needs repair, the callback will be invoked at least once.
/// There is no upper limit on the number of times it may be called.
Expand Down
2 changes: 1 addition & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl TableError {
| TableError::TypeDefinitionChanged { .. }
| TableError::TableDoesNotExist(_)
| TableError::TableAlreadyOpen(_, _) => {
StorageError::Corrupted(format!("{}: {}", msg, &self))
StorageError::Corrupted(format!("{msg}: {self}"))
}
TableError::Storage(storage) => storage,
}
Expand Down
26 changes: 19 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
#![allow(clippy::drop_non_drop)]
#![deny(
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::cast_precision_loss,
clippy::cast_sign_loss,
clippy::disallowed_methods
#![deny(clippy::all, clippy::pedantic, clippy::disallowed_methods)]
#![allow(
clippy::default_trait_access,
clippy::if_not_else,
clippy::inline_always,
clippy::iter_not_returning_iterator,
clippy::let_underscore_drop,
clippy::missing_errors_doc,
clippy::missing_panics_doc,
clippy::module_name_repetitions,
clippy::must_use_candidate,
clippy::needless_pass_by_value,
clippy::option_option,
clippy::redundant_closure_for_method_calls,
clippy::similar_names,
clippy::too_many_lines,
clippy::unnecessary_wraps,
clippy::unreadable_literal,
clippy::wildcard_imports
)]
// TODO remove this once wasi no longer requires nightly
#![cfg_attr(target_os = "wasi", feature(wasi_ext))]
Expand Down
14 changes: 6 additions & 8 deletions src/multimap_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,6 @@ pub(crate) fn finalize_tree_and_subtree_checksums(
}
}
}
drop(accessor);
// TODO: maybe there's a better abstraction, so that we don't need to call into this low-level method?
let mut mutator = LeafMutator::new(
&mut leaf_page,
Expand Down Expand Up @@ -525,7 +524,7 @@ impl Into<u8> for DynamicCollectionType {
/// (when type = 2) root (8 bytes): sub tree root page number
/// (when type = 2) checksum (16 bytes): sub tree checksum
///
/// NOTE: Even though the [PhantomData] is zero-sized, the inner data DST must be placed last.
/// NOTE: Even though the [`PhantomData`] is zero-sized, the inner data DST must be placed last.
/// See [Exotically Sized Types](https://doc.rust-lang.org/nomicon/exotic-sizes.html#dynamically-sized-types-dsts)
/// section of the Rustonomicon for more details.
#[repr(transparent)]
Expand Down Expand Up @@ -563,7 +562,6 @@ impl<V: Key> Value for &DynamicCollection<V> {

fn as_bytes<'a, 'b: 'a>(value: &'a Self::SelfType<'b>) -> &'a [u8]
where
Self: 'a,
Self: 'b,
{
&value.data
Expand All @@ -576,7 +574,7 @@ impl<V: Key> Value for &DynamicCollection<V> {

impl<V: Key> DynamicCollection<V> {
fn new(data: &[u8]) -> &Self {
unsafe { mem::transmute(data) }
unsafe { &*(data as *const [u8] as *const DynamicCollection<V>) }
}

fn collection_type(&self) -> DynamicCollectionType {
Expand All @@ -591,7 +589,7 @@ impl<V: Key> DynamicCollection<V> {
fn as_subtree(&self) -> BtreeHeader {
assert!(matches!(self.collection_type(), SubtreeV2));
BtreeHeader::from_le_bytes(
self.data[1..(1 + BtreeHeader::serialized_size())]
self.data[1..=BtreeHeader::serialized_size()]
.try_into()
.unwrap(),
)
Expand Down Expand Up @@ -702,7 +700,7 @@ impl UntypedDynamicCollection {
}

fn new(data: &[u8]) -> &Self {
unsafe { mem::transmute(data) }
unsafe { &*(data as *const [u8] as *const UntypedDynamicCollection) }
}

fn make_subtree_data(header: BtreeHeader) -> Vec<u8> {
Expand All @@ -727,7 +725,7 @@ impl UntypedDynamicCollection {
fn as_subtree(&self) -> BtreeHeader {
assert!(matches!(self.collection_type(), SubtreeV2));
BtreeHeader::from_le_bytes(
self.data[1..(1 + BtreeHeader::serialized_size())]
self.data[1..=BtreeHeader::serialized_size()]
.try_into()
.unwrap(),
)
Expand Down Expand Up @@ -851,7 +849,7 @@ impl<'a, V: Key + 'static> Drop for MultimapValue<'a, V> {
drop(mem::take(&mut self.inner));
if !self.free_on_drop.is_empty() {
let mut freed_pages = self.freed_pages.as_ref().unwrap().lock().unwrap();
for page in self.free_on_drop.iter() {
for page in &self.free_on_drop {
if !self.mem.as_ref().unwrap().free_if_uncommitted(*page) {
freed_pages.push(*page);
}
Expand Down
4 changes: 2 additions & 2 deletions src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl<'txn, K: Key + 'static, V: MutInPlaceValue + 'static> Table<'txn, K, V> {
///
/// If key is already present it is replaced
///
/// The returned reference will have length equal to value_length
/// The returned reference will have length equal to `value_length`
pub fn insert_reserve<'a>(
&mut self,
key: impl Borrow<K::SelfType<'a>>,
Expand Down Expand Up @@ -304,7 +304,7 @@ fn debug_helper<K: Key + 'static, V: Value + 'static>(
first: Result<Option<(AccessGuard<K>, AccessGuard<V>)>>,
last: Result<Option<(AccessGuard<K>, AccessGuard<V>)>>,
) -> std::fmt::Result {
write!(f, "Table [ name: \"{}\", ", name)?;
write!(f, "Table [ name: \"{name}\", ")?;
if let Ok(len) = len {
if len == 0 {
write!(f, "No entries")?;
Expand Down
11 changes: 5 additions & 6 deletions src/transaction_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ impl TransactionId {
Self(value)
}

pub(crate) fn raw_id(&self) -> u64 {
pub(crate) fn raw_id(self) -> u64 {
self.0
}

pub(crate) fn next(&self) -> TransactionId {
pub(crate) fn next(self) -> TransactionId {
TransactionId(self.0 + 1)
}

Expand All @@ -30,7 +30,7 @@ impl TransactionId {
next
}

pub(crate) fn parent(&self) -> Option<TransactionId> {
pub(crate) fn parent(self) -> Option<TransactionId> {
if self.0 == 0 {
None
} else {
Expand All @@ -43,7 +43,7 @@ impl TransactionId {
pub(crate) struct SavepointId(pub u64);

impl SavepointId {
pub(crate) fn next(&self) -> SavepointId {
pub(crate) fn next(self) -> SavepointId {
SavepointId(self.0 + 1)
}
}
Expand All @@ -65,7 +65,6 @@ impl Value for SavepointId {

fn as_bytes<'a, 'b: 'a>(value: &'a Self::SelfType<'b>) -> Self::AsBytes<'a>
where
Self: 'a,
Self: 'b,
{
value.0.to_le_bytes()
Expand Down Expand Up @@ -243,6 +242,6 @@ impl TransactionTracker {
.live_read_transactions
.keys()
.next()
.cloned()
.copied()
}
}
11 changes: 5 additions & 6 deletions src/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ impl WriteTransaction {
println!("Tables");
for p in table_pages {
all_allocated.remove(&p);
println!("{p:?}")
println!("{p:?}");
}

let system_table_allocators = self
Expand All @@ -550,15 +550,15 @@ impl WriteTransaction {
println!("System tables");
for p in system_table_pages {
all_allocated.remove(&p);
println!("{p:?}")
println!("{p:?}");
}

println!("Free table");
if let Some(freed_iter) = self.freed_tree.lock().unwrap().all_pages_iter().unwrap() {
for p in freed_iter {
let p = p.unwrap();
all_allocated.remove(&p);
println!("{p:?}")
println!("{p:?}");
}
}
println!("Pending free (i.e. in freed table)");
Expand All @@ -574,7 +574,7 @@ impl WriteTransaction {
for i in 0..value.len() {
let p = value.get(i);
all_allocated.remove(&p);
println!("{p:?}")
println!("{p:?}");
}
}
if !all_allocated.is_empty() {
Expand Down Expand Up @@ -1065,8 +1065,7 @@ impl WriteTransaction {
let free_until_transaction = self
.transaction_tracker
.oldest_live_read_transaction()
.map(|x| x.next())
.unwrap_or(self.transaction_id);
.map_or(self.transaction_id, |x| x.next());

let user_root = self
.tables
Expand Down
8 changes: 3 additions & 5 deletions src/tree_store/btree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,11 @@ impl UntypedBtreeMut {
}
}

drop(accessor);
let mut mutator = BranchMutator::new(&mut page);
for (child_index, child_page, child_checksum) in new_children.into_iter().flatten()
{
mutator.write_child_page(child_index, child_page, child_checksum);
}
drop(mutator);

branch_checksum(&page, self.key_width)
}
Expand Down Expand Up @@ -553,7 +551,7 @@ impl<K: Key + 'static, V: Value + 'static> BtreeMut<'_, K, V> {

impl<'a, K: Key + 'a, V: MutInPlaceValue + 'a> BtreeMut<'a, K, V> {
/// Reserve space to insert a key-value pair
/// The returned reference will have length equal to value_length
/// The returned reference will have length equal to `value_length`
// Return type has the same lifetime as &self, because the tree must not be modified until the mutable guard is dropped
pub(crate) fn insert_reserve(
&mut self,
Expand Down Expand Up @@ -613,7 +611,7 @@ impl RawBtree {
}

pub(crate) fn len(&self) -> Result<u64> {
Ok(self.root.map(|x| x.length).unwrap_or(0))
Ok(self.root.map_or(0, |x| x.length))
}

pub(crate) fn verify_checksum(&self) -> Result<bool> {
Expand Down Expand Up @@ -811,7 +809,7 @@ impl<K: Key, V: Value> Btree<K, V> {
}

pub(crate) fn len(&self) -> Result<u64> {
Ok(self.root.map(|x| x.length).unwrap_or(0))
Ok(self.root.map_or(0, |x| x.length))
}

pub(crate) fn stats(&self) -> Result<BtreeStats> {
Expand Down
13 changes: 5 additions & 8 deletions src/tree_store/btree_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,9 @@ impl<'a> LeafAccessor<'a> {
pub(super) fn print_node<K: Key, V: Value>(&self, include_value: bool) {
let mut i = 0;
while let Some(entry) = self.entry(i) {
eprint!(" key_{}={:?}", i, K::from_bytes(entry.key()));
eprint!(" key_{i}={:?}", K::from_bytes(entry.key()));
if include_value {
eprint!(" value_{}={:?}", i, V::from_bytes(entry.value()));
eprint!(" value_{i}={:?}", V::from_bytes(entry.value()));
}
i += 1;
}
Expand Down Expand Up @@ -510,7 +510,7 @@ impl<'a, 'b> LeafBuilder<'a, 'b> {
pub(super) fn push(&mut self, key: &'a [u8], value: &'a [u8]) {
self.total_key_bytes += key.len();
self.total_value_bytes += value.len();
self.pairs.push((key, value))
self.pairs.push((key, value));
}

pub(super) fn push_all_except(
Expand Down Expand Up @@ -871,7 +871,6 @@ impl<'b> LeafMutator<'b> {
.value_range(i)
.map(|(start, end)| end - start)
.unwrap_or_default();
drop(accessor);

let value_delta = if overwrite {
isize::try_from(value.len()).unwrap() - isize::try_from(existing_value_len).unwrap()
Expand Down Expand Up @@ -989,7 +988,6 @@ impl<'b> LeafMutator<'b> {
let value_start = accessor.value_start(i).unwrap();
let value_end = accessor.value_end(i).unwrap();
let last_value_end = accessor.value_end(accessor.num_pairs() - 1).unwrap();
drop(accessor);

// Update all the pointers
let key_ptr_size = if self.fixed_key_size.is_none() {
Expand Down Expand Up @@ -1086,7 +1084,6 @@ impl<'b> LeafMutator<'b> {
self.fixed_value_size,
);
let num_pairs = accessor.num_pairs();
drop(accessor);
let mut offset = 4 + size_of::<u32>() * i;
if self.fixed_key_size.is_none() {
offset += size_of::<u32>() * num_pairs;
Expand Down Expand Up @@ -1132,8 +1129,8 @@ impl<'a: 'b, 'b, T: Page + 'a> BranchAccessor<'a, 'b, T> {
for i in 0..(self.count_children() - 1) {
if let Some(child) = self.child_page(i + 1) {
let key = self.key(i).unwrap();
eprint!(" key_{}={:?}", i, K::from_bytes(key));
eprint!(" child_{}={:?}", i + 1, child);
eprint!(" key_{i}={:?}", K::from_bytes(key));
eprint!(" child_{}={child:?}", i + 1);
}
}
eprint!("]");
Expand Down
5 changes: 2 additions & 3 deletions src/tree_store/btree_iters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ pub enum RangeIterState {
impl RangeIterState {
fn page_number(&self) -> PageNumber {
match self {
Leaf { page, .. } => page.get_page_number(),
Internal { page, .. } => page.get_page_number(),
Leaf { page, .. } | Internal { page, .. } => page.get_page_number(),
}
}

Expand Down Expand Up @@ -138,7 +137,7 @@ impl RangeIterState {
.entry_ranges(*entry)?;
Some(EntryGuard::new(page.clone(), key, value))
}
_ => None,
Internal { .. } => None,
}
}
}
Expand Down
Loading