Fix empty borrow location warnings (#4593)

* Fix empty borrow location warnings

* add error tests for rc variants

* fix clippy

packages/generational-box/src/sync.rs

@@ -19,6 +19,9 @@ use crate::{
 type RwLockStorageEntryRef = RwLockReadGuard<'static, StorageEntry<RwLockStorageEntryData>>;
 type RwLockStorageEntryMut = RwLockWriteGuard<'static, StorageEntry<RwLockStorageEntryData>>;
 
+type AnyRef = MappedRwLockReadGuard<'static, Box<dyn Any + Send + Sync + 'static>>;
+type AnyRefMut = MappedRwLockWriteGuard<'static, Box<dyn Any + Send + Sync + 'static>>;
+
 pub(crate) enum RwLockStorageEntryData {
     Reference(GenerationalPointer<SyncStorage>),
     Rc(RcStorageEntry<Box<dyn Any + Send + Sync>>),
@@ -59,13 +62,16 @@ pub struct SyncStorage {
 impl SyncStorage {
     pub(crate) fn read(
         pointer: GenerationalPointer<Self>,
-    ) -> BorrowResult<MappedRwLockReadGuard<'static, Box<dyn Any + Send + Sync + 'static>>> {
-        Self::get_split_ref(pointer).map(|(_, guard)| {
-            RwLockReadGuard::map(guard, |data| match &data.data {
-                RwLockStorageEntryData::Data(data) => data,
-                RwLockStorageEntryData::Rc(data) => &data.data,
-                _ => unreachable!(),
-            })
+    ) -> BorrowResult<(AnyRef, GenerationalPointer<Self>)> {
+        Self::get_split_ref(pointer).map(|(resolved, guard)| {
+            (
+                RwLockReadGuard::map(guard, |data| match &data.data {
+                    RwLockStorageEntryData::Data(data) => data,
+                    RwLockStorageEntryData::Rc(data) => &data.data,
+                    _ => unreachable!(),
+                }),
+                resolved,
+            )
         })
     }
 
@@ -99,14 +105,16 @@ impl SyncStorage {
 
     pub(crate) fn write(
         pointer: GenerationalPointer<Self>,
-    ) -> BorrowMutResult<MappedRwLockWriteGuard<'static, Box<dyn Any + Send + Sync + 'static>>>
-    {
-        Self::get_split_mut(pointer).map(|(_, guard)| {
-            RwLockWriteGuard::map(guard, |data| match &mut data.data {
-                RwLockStorageEntryData::Data(data) => data,
-                RwLockStorageEntryData::Rc(data) => &mut data.data,
-                _ => unreachable!(),
-            })
+    ) -> BorrowMutResult<(AnyRefMut, GenerationalPointer<Self>)> {
+        Self::get_split_mut(pointer).map(|(resolved, guard)| {
+            (
+                RwLockWriteGuard::map(guard, |data| match &mut data.data {
+                    RwLockStorageEntryData::Data(data) => data,
+                    RwLockStorageEntryData::Rc(data) => &mut data.data,
+                    _ => unreachable!(),
+                }),
+                resolved,
+            )
         })
     }
 
@@ -277,7 +285,7 @@ impl<T: Sync + Send + 'static> Storage<T> for SyncStorage {
     fn try_read(
         pointer: GenerationalPointer<Self>,
     ) -> Result<Self::Ref<'static, T>, error::BorrowError> {
-        let read = Self::read(pointer)?;
+        let (read, pointer) = Self::read(pointer)?;
 
         let read = MappedRwLockReadGuard::try_map(read, |any| {
             // Then try to downcast
@@ -298,7 +306,7 @@ impl<T: Sync + Send + 'static> Storage<T> for SyncStorage {
     fn try_write(
         pointer: GenerationalPointer<Self>,
     ) -> Result<Self::Mut<'static, T>, error::BorrowMutError> {
-        let write = Self::write(pointer)?;
+        let (write, pointer) = Self::write(pointer)?;
 
         let write = MappedRwLockWriteGuard::try_map(write, |any| {
             // Then try to downcast

packages/generational-box/src/unsync.rs

@@ -13,8 +13,9 @@ use std::{
 };
 
 type RefCellStorageEntryRef = Ref<'static, StorageEntry<RefCellStorageEntryData>>;
-
 type RefCellStorageEntryMut = RefMut<'static, StorageEntry<RefCellStorageEntryData>>;
+type AnyRef = Ref<'static, Box<dyn Any>>;
+type AnyRefMut = RefMut<'static, Box<dyn Any>>;
 
 thread_local! {
     static UNSYNC_RUNTIME: RefCell<Vec<&'static UnsyncStorage>> = const { RefCell::new(Vec::new()) };
@@ -54,13 +55,16 @@ pub struct UnsyncStorage {
 impl UnsyncStorage {
     pub(crate) fn read(
         pointer: GenerationalPointer<Self>,
-    ) -> BorrowResult<Ref<'static, Box<dyn Any>>> {
-        Self::get_split_ref(pointer).map(|(_, guard)| {
-            Ref::map(guard, |data| match &data.data {
-                RefCellStorageEntryData::Data(data) => data,
-                RefCellStorageEntryData::Rc(data) => &data.data,
-                _ => unreachable!(),
-            })
+    ) -> BorrowResult<(AnyRef, GenerationalPointer<Self>)> {
+        Self::get_split_ref(pointer).map(|(resolved, guard)| {
+            (
+                Ref::map(guard, |data| match &data.data {
+                    RefCellStorageEntryData::Data(data) => data,
+                    RefCellStorageEntryData::Rc(data) => &data.data,
+                    _ => unreachable!(),
+                }),
+                resolved,
+            )
         })
     }
 
@@ -98,13 +102,16 @@ impl UnsyncStorage {
 
     pub(crate) fn write(
         pointer: GenerationalPointer<Self>,
-    ) -> BorrowMutResult<RefMut<'static, Box<dyn Any>>> {
-        Self::get_split_mut(pointer).map(|(_, guard)| {
-            RefMut::map(guard, |data| match &mut data.data {
-                RefCellStorageEntryData::Data(data) => data,
-                RefCellStorageEntryData::Rc(data) => &mut data.data,
-                _ => unreachable!(),
-            })
+    ) -> BorrowMutResult<(AnyRefMut, GenerationalPointer<Self>)> {
+        Self::get_split_mut(pointer).map(|(resolved, guard)| {
+            (
+                RefMut::map(guard, |data| match &mut data.data {
+                    RefCellStorageEntryData::Data(data) => data,
+                    RefCellStorageEntryData::Rc(data) => &mut data.data,
+                    _ => unreachable!(),
+                }),
+                resolved,
+            )
         })
     }
 
@@ -274,7 +281,7 @@ impl<T: 'static> Storage<T> for UnsyncStorage {
     fn try_read(
         pointer: GenerationalPointer<Self>,
     ) -> Result<Self::Ref<'static, T>, error::BorrowError> {
-        let read = Self::read(pointer)?;
+        let (read, pointer) = Self::read(pointer)?;
 
         let ref_ = Ref::filter_map(read, |any| {
             // Then try to downcast
@@ -295,7 +302,7 @@ impl<T: 'static> Storage<T> for UnsyncStorage {
     fn try_write(
         pointer: GenerationalPointer<Self>,
     ) -> Result<Self::Mut<'static, T>, error::BorrowMutError> {
-        let write = Self::write(pointer)?;
+        let (write, pointer) = Self::write(pointer)?;
 
         let ref_mut = RefMut::filter_map(write, |any| {
             // Then try to downcast

packages/generational-box/tests/errors.rs

@@ -33,6 +33,18 @@ fn create_at_location<S: Storage<i32>>(
     (value, location)
 }
 
+#[track_caller]
+fn create_at_location_rc<S: Storage<i32>>(
+    owner: &Owner<S>,
+) -> (
+    GenerationalBox<i32, S>,
+    &'static std::panic::Location<'static>,
+) {
+    let location = std::panic::Location::caller();
+    let value = owner.insert_rc(1);
+    (value, location)
+}
+
 #[test]
 fn read_while_writing_error() {
     fn read_while_writing_error_test<S: Storage<i32> + 'static>() {
@@ -54,6 +66,27 @@ fn read_while_writing_error() {
     read_while_writing_error_test::<UnsyncStorage>();
 }
 
+#[test]
+fn read_while_writing_error_rc() {
+    fn read_while_writing_error_test<S: Storage<i32> + 'static>() {
+        let owner = S::owner();
+        let value = owner.insert_rc(1);
+
+        let (write, location) = write_at_location(value);
+
+        assert_eq!(
+            value.try_read().err(),
+            Some(BorrowError::AlreadyBorrowedMut(
+                AlreadyBorrowedMutError::new(location)
+            ))
+        );
+        drop(write);
+    }
+
+    // For sync storage this will deadlock
+    read_while_writing_error_test::<UnsyncStorage>();
+}
+
 #[test]
 fn read_after_dropped_error() {
     fn read_after_dropped_error_test<S: Storage<i32> + 'static>() {
@@ -70,6 +103,22 @@ fn read_after_dropped_error() {
     read_after_dropped_error_test::<SyncStorage>();
 }
 
+#[test]
+fn read_after_dropped_error_rc() {
+    fn read_after_dropped_error_test<S: Storage<i32> + 'static>() {
+        let owner = S::owner();
+        let (value, location) = create_at_location_rc(&owner);
+        drop(owner);
+        assert_eq!(
+            value.try_read().err(),
+            Some(BorrowError::Dropped(ValueDroppedError::new(location)))
+        );
+    }
+
+    read_after_dropped_error_test::<UnsyncStorage>();
+    read_after_dropped_error_test::<SyncStorage>();
+}
+
 #[test]
 fn write_while_writing_error() {
     fn write_while_writing_error_test<S: Storage<i32> + 'static>() {
@@ -96,6 +145,32 @@ fn write_while_writing_error() {
     write_while_writing_error_test::<UnsyncStorage>();
 }
 
+#[test]
+fn write_while_writing_error_rc() {
+    fn write_while_writing_error_test<S: Storage<i32> + 'static>() {
+        let owner = S::owner();
+        let value = owner.insert_rc(1);
+
+        #[allow(unused)]
+        let (write, location) = write_at_location(value);
+
+        let write_result = value.try_write();
+        assert!(write_result.is_err());
+        #[cfg(any(debug_assertions, feature = "debug_borrows"))]
+        assert_eq!(
+            write_result.err(),
+            Some(BorrowMutError::AlreadyBorrowedMut(
+                AlreadyBorrowedMutError::new(location)
+            ))
+        );
+
+        drop(write);
+    }
+
+    // For sync storage this will deadlock
+    write_while_writing_error_test::<UnsyncStorage>();
+}
+
 #[test]
 fn write_while_reading_error() {
     fn write_while_reading_error_test<S: Storage<i32> + 'static>() {
@@ -117,3 +192,25 @@ fn write_while_reading_error() {
     // For sync storage this will deadlock
     write_while_reading_error_test::<UnsyncStorage>();
 }
+
+#[test]
+fn write_while_reading_error_rc() {
+    fn write_while_reading_error_test<S: Storage<i32> + 'static>() {
+        let owner = S::owner();
+        let value = owner.insert_rc(1);
+
+        let (read, location) = read_at_location(value);
+
+        assert_eq!(
+            value.try_write().err(),
+            Some(BorrowMutError::AlreadyBorrowed(AlreadyBorrowedError::new(
+                vec![location]
+            )))
+        );
+
+        drop(read);
+    }
+
+    // For sync storage this will deadlock
+    write_while_reading_error_test::<UnsyncStorage>();
+}