Skip to content

Commit

Permalink
improve documentation around unsafe ItemSliceMut
Browse files Browse the repository at this point in the history
- use `super` instead of `in <module-path>`
- restrict visibliity of fields with items that have unsafe
- further describe usage of ItemSliceSync in `State`
  • Loading branch information
Byron committed Jan 4, 2024
1 parent 4017e69 commit 696cbb8
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
13 changes: 7 additions & 6 deletions gix-pack/src/cache/delta/traverse/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ mod root {
impl<'a, T: Send> Node<'a, T> {
/// SAFETY: The child_items must be unique among between users of the `ItemSliceSync`.
#[allow(unsafe_code)]
pub(in crate::cache::delta::traverse) unsafe fn new(
item: &'a mut Item<T>,
child_items: &'a ItemSliceSync<'a, Item<T>>,
) -> Self {
pub(super) unsafe fn new(item: &'a mut Item<T>, child_items: &'a ItemSliceSync<'a, Item<T>>) -> Self {
Node { item, child_items }
}
}
Expand Down Expand Up @@ -71,13 +68,17 @@ mod root {
}
}

pub(in crate::cache::delta::traverse) struct State<'items, F, MBFN, T: Send> {
pub(super) struct State<'items, F, MBFN, T: Send> {
pub delta_bytes: Vec<u8>,
pub fully_resolved_delta_bytes: Vec<u8>,
pub progress: Box<dyn Progress>,
pub resolve: F,
pub modify_base: MBFN,
pub child_items: &'items ItemSliceSync<'items, Item<T>>,
/// SAFETY: This member is essentially a `&mut [Item<T>]` that is shared between threads
/// and that assumes that no two thread will see overlapping portions of it.
/// This is assured by the way the tree threads are fed with work, which is
/// described in detail in [ItemSliceSync].
pub(super) child_items: &'items ItemSliceSync<'items, Item<T>>,
}

#[allow(clippy::too_many_arguments)]
Expand Down
8 changes: 4 additions & 4 deletions gix-pack/src/cache/delta/traverse/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::marker::PhantomData;
/// more than one base. And that's what one would really have to do for two threads to encounter the same child.
///
/// Thus I believe it's impossible for this data structure to end up in a place where it violates its assumption.
pub(in crate::cache::delta::traverse) struct ItemSliceSync<'a, T>
pub(super) struct ItemSliceSync<'a, T>
where
T: Send,
{
Expand All @@ -30,7 +30,7 @@ impl<'a, T> ItemSliceSync<'a, T>
where
T: Send,
{
pub(in crate::cache::delta::traverse) fn new(items: &'a mut [T]) -> Self {
pub(super) fn new(items: &'a mut [T]) -> Self {
ItemSliceSync {
items: items.as_mut_ptr(),
#[cfg(debug_assertions)]
Expand All @@ -41,7 +41,7 @@ where

// SAFETY: The index must point into the slice and must not be reused concurrently.
#[allow(unsafe_code)]
pub(in crate::cache::delta::traverse) unsafe fn get_mut(&self, index: usize) -> &'a mut T {
pub(super) unsafe fn get_mut(&self, index: usize) -> &'a mut T {
#[cfg(debug_assertions)]
if index >= self.len {
panic!("index out of bounds: the len is {} but the index is {index}", self.len);
Expand All @@ -56,6 +56,6 @@ where
#[allow(unsafe_code)]
unsafe impl<T> Send for ItemSliceSync<'_, T> where T: Send {}
// SAFETY: T is `Send`, and as long as the user follows the contract of
// `get_mut()`, we only ever access one T at a time.
// `get_mut()`, we only ever access one T at a time.
#[allow(unsafe_code)]
unsafe impl<T> Sync for ItemSliceSync<'_, T> where T: Send {}

0 comments on commit 696cbb8

Please sign in to comment.