From 3305e045ef393e82586d12d777c22534e2786ed4 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Fri, 22 Apr 2022 18:37:20 -0700 Subject: [PATCH] cleanup: prefer `Option<&T>` over `&Option` It seems to me like `Option<&T>` is pretty much always better to return than `&Option`. --- lib/src/index.rs | 19 ++++++++----------- lib/src/stacked_table.rs | 10 +++++----- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/lib/src/index.rs b/lib/src/index.rs index 10cc4950a..bbf396658 100644 --- a/lib/src/index.rs +++ b/lib/src/index.rs @@ -634,7 +634,7 @@ trait IndexSegment { fn segment_num_commits(&self) -> u32; - fn segment_parent_file(&self) -> &Option>; + fn segment_parent_file(&self) -> Option<&Arc>; fn segment_name(&self) -> Option; @@ -686,14 +686,14 @@ impl<'a> CompositeIndex<'a> { num_commits: self.0.segment_num_commits(), name: self.0.segment_name(), }]; - let mut parent_file = self.0.segment_parent_file().clone(); + let mut parent_file = self.0.segment_parent_file().cloned(); while parent_file.is_some() { let file = parent_file.as_ref().unwrap(); levels.push(IndexLevelStats { num_commits: file.segment_num_commits(), name: file.segment_name(), }); - parent_file = file.segment_parent_file().clone(); + parent_file = file.segment_parent_file().cloned(); } levels.reverse(); @@ -712,8 +712,7 @@ impl<'a> CompositeIndex<'a> { if pos.0 >= num_parent_commits { self.0.segment_entry_by_pos(pos, pos.0 - num_parent_commits) } else { - let parent_file: &ReadonlyIndex = - self.0.segment_parent_file().as_ref().unwrap().as_ref(); + let parent_file: &ReadonlyIndex = self.0.segment_parent_file().unwrap().as_ref(); // The parent ReadonlyIndex outlives the child let parent_file: &'a ReadonlyIndex = unsafe { std::mem::transmute(parent_file) }; @@ -726,7 +725,6 @@ impl<'a> CompositeIndex<'a> { local_match.or_else(|| { self.0 .segment_parent_file() - .as_ref() .and_then(|file| IndexRef::Readonly(file).commit_id_to_pos(commit_id)) }) } @@ -740,7 +738,6 @@ impl<'a> CompositeIndex<'a> { let parent_match = self .0 .segment_parent_file() - .as_ref() .map_or(PrefixResolution::NoMatch, |file| { file.resolve_prefix(prefix) }); @@ -1051,8 +1048,8 @@ impl IndexSegment for ReadonlyIndex { self.num_local_commits } - fn segment_parent_file(&self) -> &Option> { - &self.parent_file + fn segment_parent_file(&self) -> Option<&Arc> { + self.parent_file.as_ref() } fn segment_name(&self) -> Option { @@ -1164,8 +1161,8 @@ impl IndexSegment for MutableIndex { self.graph.len() as u32 } - fn segment_parent_file(&self) -> &Option> { - &self.parent_file + fn segment_parent_file(&self) -> Option<&Arc> { + self.parent_file.as_ref() } fn segment_name(&self) -> Option { diff --git a/lib/src/stacked_table.rs b/lib/src/stacked_table.rs index d61f8ee5a..05a8b65ea 100644 --- a/lib/src/stacked_table.rs +++ b/lib/src/stacked_table.rs @@ -37,7 +37,7 @@ use crate::lock::FileLock; pub trait TableSegment { fn segment_num_entries(&self) -> usize; - fn segment_parent_file(&self) -> &Option>; + fn segment_parent_file(&self) -> Option<&Arc>; fn segment_get_value(&self, key: &[u8]) -> Option<&[u8]>; fn segment_add_entries_to(&self, mut_table: &mut MutableTable); @@ -130,8 +130,8 @@ impl TableSegment for ReadonlyTable { self.num_local_entries } - fn segment_parent_file(&self) -> &Option> { - &self.parent_file + fn segment_parent_file(&self) -> Option<&Arc> { + self.parent_file.as_ref() } fn segment_get_value(&self, key: &[u8]) -> Option<&[u8]> { @@ -350,8 +350,8 @@ impl TableSegment for MutableTable { self.entries.len() } - fn segment_parent_file(&self) -> &Option> { - &self.parent_file + fn segment_parent_file(&self) -> Option<&Arc> { + self.parent_file.as_ref() } fn segment_get_value(&self, key: &[u8]) -> Option<&[u8]> {