Skip to content

Commit 696cbb8

Browse files
committed
improve documentation around unsafe ItemSliceMut
- use `super` instead of `in <module-path>` - restrict visibliity of fields with items that have unsafe - further describe usage of ItemSliceSync in `State`
1 parent 4017e69 commit 696cbb8

File tree

2 files changed

+11
-10
lines changed

2 files changed

+11
-10
lines changed

gix-pack/src/cache/delta/traverse/resolve.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@ mod root {
2626
impl<'a, T: Send> Node<'a, T> {
2727
/// SAFETY: The child_items must be unique among between users of the `ItemSliceSync`.
2828
#[allow(unsafe_code)]
29-
pub(in crate::cache::delta::traverse) unsafe fn new(
30-
item: &'a mut Item<T>,
31-
child_items: &'a ItemSliceSync<'a, Item<T>>,
32-
) -> Self {
29+
pub(super) unsafe fn new(item: &'a mut Item<T>, child_items: &'a ItemSliceSync<'a, Item<T>>) -> Self {
3330
Node { item, child_items }
3431
}
3532
}
@@ -71,13 +68,17 @@ mod root {
7168
}
7269
}
7370

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

8384
#[allow(clippy::too_many_arguments)]

gix-pack/src/cache/delta/traverse/util.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::marker::PhantomData;
1616
/// more than one base. And that's what one would really have to do for two threads to encounter the same child.
1717
///
1818
/// Thus I believe it's impossible for this data structure to end up in a place where it violates its assumption.
19-
pub(in crate::cache::delta::traverse) struct ItemSliceSync<'a, T>
19+
pub(super) struct ItemSliceSync<'a, T>
2020
where
2121
T: Send,
2222
{
@@ -30,7 +30,7 @@ impl<'a, T> ItemSliceSync<'a, T>
3030
where
3131
T: Send,
3232
{
33-
pub(in crate::cache::delta::traverse) fn new(items: &'a mut [T]) -> Self {
33+
pub(super) fn new(items: &'a mut [T]) -> Self {
3434
ItemSliceSync {
3535
items: items.as_mut_ptr(),
3636
#[cfg(debug_assertions)]
@@ -41,7 +41,7 @@ where
4141

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

0 commit comments

Comments
 (0)