Skip to content

Commit

Permalink
fix!: use dyn trait where possible.
Browse files Browse the repository at this point in the history
This reduces compile time due to avoiding duplication.
  • Loading branch information
Byron committed Sep 5, 2023
1 parent e72aa50 commit 8ba2cfc
Show file tree
Hide file tree
Showing 18 changed files with 152 additions and 182 deletions.
36 changes: 16 additions & 20 deletions gix-odb/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,7 @@ mod impls {
where
S: crate::Write,
{
type Error = S::Error;

fn write_stream(&self, kind: Kind, size: u64, from: impl Read) -> Result<ObjectId, Self::Error> {
fn write_stream(&self, kind: Kind, size: u64, from: impl Read) -> Result<ObjectId, crate::write::Error> {
self.inner.write_stream(kind, size, from)
}
}
Expand All @@ -156,24 +154,24 @@ mod impls {
where
S: gix_pack::Find,
{
type Error = S::Error;

fn contains(&self, id: impl AsRef<oid>) -> bool {
self.inner.contains(id)
self.inner.contains(id.as_ref())
}

fn try_find<'a>(&self, id: impl AsRef<oid>, buffer: &'a mut Vec<u8>) -> Result<Option<Data<'a>>, Self::Error> {
gix_pack::Find::try_find(self, id, buffer).map(|t| t.map(|t| t.0))
fn try_find<'a>(
&self,
id: impl AsRef<oid>,
buffer: &'a mut Vec<u8>,
) -> Result<Option<Data<'a>>, crate::find::Error> {
gix_pack::Find::try_find(self, id.as_ref(), buffer).map(|t| t.map(|t| t.0))
}
}

impl<S> crate::Header for Cache<S>
where
S: crate::Header,
{
type Error = S::Error;

fn try_header(&self, id: impl AsRef<oid>) -> Result<Option<Header>, Self::Error> {
fn try_header(&self, id: impl AsRef<oid>) -> Result<Option<Header>, crate::find::Error> {
self.inner.try_header(id)
}
}
Expand All @@ -182,17 +180,15 @@ mod impls {
where
S: gix_pack::Find,
{
type Error = S::Error;

fn contains(&self, id: impl AsRef<oid>) -> bool {
fn contains(&self, id: &oid) -> bool {
self.inner.contains(id)
}

fn try_find<'a>(
&self,
id: impl AsRef<oid>,
id: &oid,
buffer: &'a mut Vec<u8>,
) -> Result<Option<(Data<'a>, Option<Location>)>, Self::Error> {
) -> Result<Option<(Data<'a>, Option<Location>)>, crate::find::Error> {
match self.pack_cache.as_ref().map(RefCell::borrow_mut) {
Some(mut pack_cache) => self.try_find_cached(id, buffer, pack_cache.deref_mut()),
None => self.try_find_cached(id, buffer, &mut gix_pack::cache::Never),
Expand All @@ -201,10 +197,10 @@ mod impls {

fn try_find_cached<'a>(
&self,
id: impl AsRef<oid>,
id: &oid,
buffer: &'a mut Vec<u8>,
pack_cache: &mut impl gix_pack::cache::DecodeEntry,
) -> Result<Option<(Data<'a>, Option<gix_pack::data::entry::Location>)>, Self::Error> {
pack_cache: &mut dyn gix_pack::cache::DecodeEntry,
) -> Result<Option<(Data<'a>, Option<gix_pack::data::entry::Location>)>, crate::find::Error> {
if let Some(mut obj_cache) = self.object_cache.as_ref().map(RefCell::borrow_mut) {
if let Some(kind) = obj_cache.get(&id.as_ref().to_owned(), buffer) {
return Ok(Some((Data::new(kind, buffer), None)));
Expand All @@ -219,7 +215,7 @@ mod impls {
Ok(possibly_obj)
}

fn location_by_oid(&self, id: impl AsRef<oid>, buf: &mut Vec<u8>) -> Option<gix_pack::data::entry::Location> {
fn location_by_oid(&self, id: &oid, buf: &mut Vec<u8>) -> Option<gix_pack::data::entry::Location> {
self.inner.location_by_oid(id, buf)
}

Expand Down
14 changes: 8 additions & 6 deletions gix-odb/src/find.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
/// The error type returned by the [`Find`](crate::Find) and [`Header`](crate::Header) traits.
pub type Error = Box<dyn std::error::Error + Send + Sync + 'static>;
///
pub mod existing {
use gix_hash::ObjectId;

/// The error returned by the [`find(…)`][crate::FindExt::find()] trait methods.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error<T: std::error::Error + 'static> {
pub enum Error {
#[error(transparent)]
Find(T),
Find(crate::find::Error),
#[error("An object with id {} could not be found", .oid)]
NotFound { oid: ObjectId },
}
Expand All @@ -20,9 +22,9 @@ pub mod existing_object {
/// The error returned by the various [`find_*()`][crate::FindExt::find_commit()] trait methods.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error<T: std::error::Error + 'static> {
pub enum Error {
#[error(transparent)]
Find(T),
Find(crate::find::Error),
#[error(transparent)]
Decode(gix_object::decode::Error),
#[error("An object with id {oid} could not be found")]
Expand All @@ -39,9 +41,9 @@ pub mod existing_iter {
/// The error returned by the various [`find_*_iter()`][crate::FindExt::find_commit_iter()] trait methods.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error<T: std::error::Error + 'static> {
pub enum Error {
#[error(transparent)]
Find(T),
Find(crate::find::Error),
#[error("An object with id {oid} could not be found")]
NotFound { oid: ObjectId },
#[error("Expected object of kind {expected}")]
Expand Down
6 changes: 6 additions & 0 deletions gix-odb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ mod traits;

pub use traits::{Find, FindExt, Header, HeaderExt, Write};

///
pub mod write {
/// The error type returned by the [`Write`](crate::Write) trait.
pub type Error = Box<dyn std::error::Error + Send + Sync + 'static>;
}

/// A thread-local handle to access any object.
pub type Handle = Cache<store::Handle<OwnShared<Store>>>;
/// A thread-local handle to access any object, but thread-safe and independent of the actual type of `OwnShared` or feature toggles in `gix-features`.
Expand Down
12 changes: 5 additions & 7 deletions gix-odb/src/sink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@ impl Sink {
}

impl crate::traits::Write for Sink {
type Error = io::Error;

fn write_stream(
&self,
kind: gix_object::Kind,
size: u64,
mut from: impl io::Read,
) -> Result<gix_hash::ObjectId, Self::Error> {
) -> Result<gix_hash::ObjectId, crate::write::Error> {
let mut size = size.try_into().expect("object size to fit into usize");
let mut buf = [0u8; 8096];
let header = gix_object::encode::loose_header(kind, size);
Expand All @@ -42,18 +40,18 @@ impl crate::traits::Write for Sink {

let mut hasher = gix_features::hash::hasher(self.object_hash);
hasher.update(&header);
possibly_compress(&header)?;
possibly_compress(&header).map_err(Box::new)?;

while size != 0 {
let bytes = size.min(buf.len());
from.read_exact(&mut buf[..bytes])?;
from.read_exact(&mut buf[..bytes]).map_err(Box::new)?;
hasher.update(&buf[..bytes]);
possibly_compress(&buf[..bytes])?;
possibly_compress(&buf[..bytes]).map_err(Box::new)?;
size -= bytes;
}
if let Some(compressor) = self.compressor.as_ref() {
let mut c = compressor.borrow_mut();
c.flush()?;
c.flush().map_err(Box::new)?;
c.reset();
}

Expand Down
31 changes: 12 additions & 19 deletions gix-odb/src/store_impls/dynamic/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ where
mut id: &'b gix_hash::oid,
buffer: &'a mut Vec<u8>,
inflate: &mut zlib::Inflate,
pack_cache: &mut impl DecodeEntry,
pack_cache: &mut dyn DecodeEntry,
snapshot: &mut load_index::Snapshot,
recursion: Option<error::DeltaBaseRecursion<'_>>,
) -> Result<Option<(gix_object::Data<'a>, Option<gix_pack::data::entry::Location>)>, Error> {
Expand Down Expand Up @@ -150,7 +150,7 @@ where
entry,
buffer,
inflate,
|id, _out| {
&|id, _out| {
index_file.pack_offset_by_id(id).map(|pack_offset| {
gix_pack::data::decode::entry::ResolvedBase::InPack(pack.entry(pack_offset))
})
Expand Down Expand Up @@ -236,7 +236,7 @@ where
entry,
buffer,
inflate,
|id, out| {
&|id, out| {
index_file
.pack_offset_by_id(id)
.map(|pack_offset| {
Expand Down Expand Up @@ -311,10 +311,8 @@ impl<S> gix_pack::Find for super::Handle<S>
where
S: Deref<Target = super::Store> + Clone,
{
type Error = Error;

// TODO: probably make this method fallible, but that would mean its own error type.
fn contains(&self, id: impl AsRef<gix_hash::oid>) -> bool {
fn contains(&self, id: &gix_hash::oid) -> bool {
let id = id.as_ref();
let mut snapshot = self.snapshot.borrow_mut();
loop {
Expand Down Expand Up @@ -346,21 +344,18 @@ where

fn try_find_cached<'a>(
&self,
id: impl AsRef<gix_hash::oid>,
id: &gix_hash::oid,
buffer: &'a mut Vec<u8>,
pack_cache: &mut impl DecodeEntry,
) -> Result<Option<(gix_object::Data<'a>, Option<gix_pack::data::entry::Location>)>, Self::Error> {
pack_cache: &mut dyn DecodeEntry,
) -> Result<Option<(gix_object::Data<'a>, Option<gix_pack::data::entry::Location>)>, gix_pack::find::Error> {
let id = id.as_ref();
let mut snapshot = self.snapshot.borrow_mut();
let mut inflate = self.inflate.borrow_mut();
self.try_find_cached_inner(id, buffer, &mut inflate, pack_cache, &mut snapshot, None)
.map_err(|err| Box::new(err) as _)
}

fn location_by_oid(
&self,
id: impl AsRef<gix_hash::oid>,
buf: &mut Vec<u8>,
) -> Option<gix_pack::data::entry::Location> {
fn location_by_oid(&self, id: &gix_hash::oid, buf: &mut Vec<u8>) -> Option<gix_pack::data::entry::Location> {
assert!(
matches!(self.token.as_ref(), Some(handle::Mode::KeepDeletedPacksAvailable)),
"BUG: handle must be configured to `prevent_pack_unload()` before using this method"
Expand Down Expand Up @@ -511,17 +506,15 @@ where
S: Deref<Target = super::Store> + Clone,
Self: gix_pack::Find,
{
type Error = <Self as gix_pack::Find>::Error;

fn contains(&self, id: impl AsRef<gix_hash::oid>) -> bool {
gix_pack::Find::contains(self, id)
gix_pack::Find::contains(self, id.as_ref())
}

fn try_find<'a>(
&self,
id: impl AsRef<gix_hash::oid>,
buffer: &'a mut Vec<u8>,
) -> Result<Option<gix_object::Data<'a>>, Self::Error> {
gix_pack::Find::try_find(self, id, buffer).map(|t| t.map(|t| t.0))
) -> Result<Option<gix_object::Data<'a>>, crate::find::Error> {
gix_pack::Find::try_find(self, id.as_ref(), buffer).map(|t| t.map(|t| t.0))
}
}
9 changes: 4 additions & 5 deletions gix-odb/src/store_impls/dynamic/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ where
},
};
let entry = pack.entry(pack_offset);
let res = match pack.decode_header(entry, inflate, |id| {
let res = match pack.decode_header(entry, inflate, &|id| {
index_file.pack_offset_by_id(id).map(|pack_offset| {
gix_pack::data::decode::header::ResolvedBase::InPack(pack.entry(pack_offset))
})
Expand Down Expand Up @@ -130,7 +130,7 @@ where
.as_ref()
.expect("pack to still be available like just now");
let entry = pack.entry(pack_offset);
pack.decode_header(entry, inflate, |id| {
pack.decode_header(entry, inflate, &|id| {
index_file
.pack_offset_by_id(id)
.map(|pack_offset| {
Expand Down Expand Up @@ -182,12 +182,11 @@ impl<S> crate::Header for super::Handle<S>
where
S: Deref<Target = super::Store> + Clone,
{
type Error = Error;

fn try_header(&self, id: impl AsRef<oid>) -> Result<Option<Header>, Self::Error> {
fn try_header(&self, id: impl AsRef<oid>) -> Result<Option<Header>, crate::find::Error> {
let id = id.as_ref();
let mut snapshot = self.snapshot.borrow_mut();
let mut inflate = self.inflate.borrow_mut();
self.try_header_inner(id, &mut inflate, &mut snapshot, None)
.map_err(|err| Box::new(err) as _)
}
}
4 changes: 2 additions & 2 deletions gix-odb/src/store_impls/dynamic/prefix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ pub mod disambiguate {
/// matching this prefix.
pub fn new(id: impl Into<gix_hash::ObjectId>, hex_len: usize) -> Result<Self, gix_hash::prefix::Error> {
let id = id.into();
gix_hash::Prefix::new(id, hex_len)?;
gix_hash::Prefix::new(&id, hex_len)?;
Ok(Candidate { id, hex_len })
}

/// Transform ourselves into a `Prefix` with our current hex lengths.
pub fn to_prefix(&self) -> gix_hash::Prefix {
gix_hash::Prefix::new(self.id, self.hex_len).expect("our hex-len to always be in bounds")
gix_hash::Prefix::new(&self.id, self.hex_len).expect("our hex-len to always be in bounds")
}

pub(crate) fn inc_hex_len(&mut self) {
Expand Down
Loading

0 comments on commit 8ba2cfc

Please sign in to comment.