From df85b25548cfff642af2b4ac4a4521b53027cb52 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 21 Aug 2023 14:23:02 -0500 Subject: [PATCH 1/4] Add debug information to borrows and ownership in signals --- packages/generational-box/Cargo.toml | 1 + packages/generational-box/src/lib.rs | 445 ++++++++++++++++++++++++--- packages/signals/src/effect.rs | 2 +- packages/signals/src/impls.rs | 33 +- packages/signals/src/rt.rs | 33 +- packages/signals/src/signal.rs | 58 +++- 6 files changed, 497 insertions(+), 75 deletions(-) diff --git a/packages/generational-box/Cargo.toml b/packages/generational-box/Cargo.toml index 6c5a5a841..cc0db9cec 100644 --- a/packages/generational-box/Cargo.toml +++ b/packages/generational-box/Cargo.toml @@ -15,3 +15,4 @@ rand = "0.8.5" [features] default = ["check_generation"] check_generation = [] +debug_borrows = [] diff --git a/packages/generational-box/src/lib.rs b/packages/generational-box/src/lib.rs index 60bd41419..7d64a1ed3 100644 --- a/packages/generational-box/src/lib.rs +++ b/packages/generational-box/src/lib.rs @@ -2,9 +2,13 @@ #![warn(missing_docs)] use std::{ + any::Any, cell::{Cell, Ref, RefCell, RefMut}, - fmt::Debug, + error::Error, + fmt::{Debug, Display}, marker::PhantomData, + ops::{Deref, DerefMut}, + panic::Location, rc::Rc, }; @@ -153,6 +157,8 @@ pub struct GenerationalBox { raw: MemoryLocation, #[cfg(any(debug_assertions, feature = "check_generation"))] generation: u32, + #[cfg(any(debug_assertions, feature = "check_generation"))] + created_at: &'static std::panic::Location<'static>, _marker: PhantomData, } @@ -161,7 +167,7 @@ impl Debug for GenerationalBox { #[cfg(any(debug_assertions, feature = "check_generation"))] f.write_fmt(format_args!( "{:?}@{:?}", - self.raw.data.as_ptr(), + self.raw.0.data.as_ptr(), self.generation ))?; #[cfg(not(any(debug_assertions, feature = "check_generation")))] @@ -175,7 +181,7 @@ impl GenerationalBox { fn validate(&self) -> bool { #[cfg(any(debug_assertions, feature = "check_generation"))] { - self.raw.generation.get() == self.generation + self.raw.0.generation.get() == self.generation } #[cfg(not(any(debug_assertions, feature = "check_generation")))] { @@ -184,43 +190,51 @@ impl GenerationalBox { } /// Try to read the value. Returns None if the value is no longer valid. - pub fn try_read(&self) -> Option> { - self.validate() - .then(|| { - Ref::filter_map(self.raw.data.borrow(), |any| { - any.as_ref()?.downcast_ref::() - }) - .ok() - }) - .flatten() + #[track_caller] + pub fn try_read(&self) -> Result, BorrowError> { + if !self.validate() { + return Err(BorrowError::Dropped(ValueDroppedError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + created_at: self.created_at, + })); + } + self.raw.try_borrow( + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + self.created_at, + ) } /// Read the value. Panics if the value is no longer valid. - pub fn read(&self) -> Ref<'_, T> { + #[track_caller] + pub fn read(&self) -> GenerationalRef<'_, T> { self.try_read().unwrap() } /// Try to write the value. Returns None if the value is no longer valid. - pub fn try_write(&self) -> Option> { - self.validate() - .then(|| { - RefMut::filter_map(self.raw.data.borrow_mut(), |any| { - any.as_mut()?.downcast_mut::() - }) - .ok() - }) - .flatten() + #[track_caller] + pub fn try_write(&self) -> Result, BorrowMutError> { + if !self.validate() { + return Err(BorrowMutError::Dropped(ValueDroppedError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + created_at: self.created_at, + })); + } + self.raw.try_borrow_mut( + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + self.created_at, + ) } /// Write the value. Panics if the value is no longer valid. - pub fn write(&self) -> RefMut<'_, T> { + #[track_caller] + pub fn write(&self) -> GenerationalRefMut<'_, T> { self.try_write().unwrap() } /// Set the value. Panics if the value is no longer valid. pub fn set(&self, value: T) { self.validate().then(|| { - *self.raw.data.borrow_mut() = Some(Box::new(value)); + *self.raw.0.data.borrow_mut() = Some(Box::new(value)); }); } @@ -228,7 +242,8 @@ impl GenerationalBox { pub fn ptr_eq(&self, other: &Self) -> bool { #[cfg(any(debug_assertions, feature = "check_generation"))] { - self.raw.data.as_ptr() == other.raw.data.as_ptr() && self.generation == other.generation + self.raw.0.data.as_ptr() == other.raw.0.data.as_ptr() + && self.generation == other.generation } #[cfg(not(any(debug_assertions, feature = "check_generation")))] { @@ -246,26 +261,38 @@ impl Clone for GenerationalBox { } #[derive(Clone, Copy)] -struct MemoryLocation { - data: &'static RefCell>>, +struct MemoryLocation(&'static MemoryLocationInner); + +struct MemoryLocationInner { + data: RefCell>>, #[cfg(any(debug_assertions, feature = "check_generation"))] - generation: &'static Cell, + generation: Cell, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_at: RefCell>>, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_mut_at: Cell>>, } impl MemoryLocation { #[allow(unused)] fn drop(&self) { - let old = self.data.borrow_mut().take(); + let old = self.0.data.borrow_mut().take(); #[cfg(any(debug_assertions, feature = "check_generation"))] if old.is_some() { drop(old); - let new_generation = self.generation.get() + 1; - self.generation.set(new_generation); + let new_generation = self.0.generation.get() + 1; + self.0.generation.set(new_generation); } } - fn replace(&mut self, value: T) -> GenerationalBox { - let mut inner_mut = self.data.borrow_mut(); + fn replace_with_caller( + &mut self, + value: T, + #[cfg(any(debug_assertions, feature = "check_generation"))] caller: &'static Location< + 'static, + >, + ) -> GenerationalBox { + let mut inner_mut = self.0.data.borrow_mut(); let raw = Box::new(value); let old = inner_mut.replace(raw); @@ -273,10 +300,318 @@ impl MemoryLocation { GenerationalBox { raw: *self, #[cfg(any(debug_assertions, feature = "check_generation"))] - generation: self.generation.get(), + generation: self.0.generation.get(), + #[cfg(any(debug_assertions, feature = "check_generation"))] + created_at: caller, _marker: PhantomData, } } + + #[track_caller] + fn try_borrow( + &self, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + created_at: &'static std::panic::Location<'static>, + ) -> Result, BorrowError> { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + self.0 + .borrowed_at + .borrow_mut() + .push(std::panic::Location::caller()); + match self.0.data.try_borrow() { + Ok(borrow) => match Ref::filter_map(borrow, |any| any.as_ref()?.downcast_ref::()) { + Ok(reference) => Ok(GenerationalRef { + inner: reference, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow: GenerationalRefBorrowInfo { + borrowed_at: std::panic::Location::caller(), + borrowed_from: self.0, + }, + }), + Err(_) => Err(BorrowError::Dropped(ValueDroppedError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + created_at, + })), + }, + Err(_) => Err(BorrowError::AlreadyBorrowedMut(AlreadyBorrowedMutError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_mut_at: self.0.borrowed_mut_at.get().unwrap(), + })), + } + } + + #[track_caller] + fn try_borrow_mut( + &self, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + created_at: &'static std::panic::Location<'static>, + ) -> Result, BorrowMutError> { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + { + self.0 + .borrowed_mut_at + .set(Some(std::panic::Location::caller())); + } + match self.0.data.try_borrow_mut() { + Ok(borrow_mut) => { + match RefMut::filter_map(borrow_mut, |any| any.as_mut()?.downcast_mut::()) { + Ok(reference) => Ok(GenerationalRefMut { + inner: reference, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow: GenerationalRefMutBorrowInfo { + borrowed_from: self.0, + }, + }), + Err(_) => Err(BorrowMutError::Dropped(ValueDroppedError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + created_at, + })), + } + } + Err(_) => Err(BorrowMutError::AlreadyBorrowed(AlreadyBorrowedError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_at: self.0.borrowed_at.borrow().clone(), + })), + } + } +} + +#[derive(Debug, Clone)] +/// An error that can occur when trying to borrow a value. +pub enum BorrowError { + /// The value was dropped. + Dropped(ValueDroppedError), + /// The value was already borrowed mutably. + AlreadyBorrowedMut(AlreadyBorrowedMutError), +} + +impl Display for BorrowError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + BorrowError::Dropped(error) => Display::fmt(error, f), + BorrowError::AlreadyBorrowedMut(error) => Display::fmt(error, f), + } + } +} + +impl Error for BorrowError {} + +#[derive(Debug, Clone)] +/// An error that can occur when trying to borrow a value mutably. +pub enum BorrowMutError { + /// The value was dropped. + Dropped(ValueDroppedError), + /// The value was already borrowed. + AlreadyBorrowed(AlreadyBorrowedError), + /// The value was already borrowed mutably. + AlreadyBorrowedMut(AlreadyBorrowedMutError), +} + +impl Display for BorrowMutError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + BorrowMutError::Dropped(error) => Display::fmt(error, f), + BorrowMutError::AlreadyBorrowedMut(error) => Display::fmt(error, f), + BorrowMutError::AlreadyBorrowed(error) => Display::fmt(error, f), + } + } +} + +impl Error for BorrowMutError {} + +/// An error that can occur when trying to use a value that has been dropped. +#[derive(Debug, Copy, Clone)] +pub struct ValueDroppedError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + created_at: &'static std::panic::Location<'static>, +} + +impl Display for ValueDroppedError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Failed to borrow because the value was dropped.")?; + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + f.write_fmt(format_args!("created_at: {}", self.created_at))?; + Ok(()) + } +} + +impl std::error::Error for ValueDroppedError {} + +/// An error that can occur when trying to borrow a value that has already been borrowed mutably. +#[derive(Debug, Copy, Clone)] +pub struct AlreadyBorrowedMutError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_mut_at: &'static std::panic::Location<'static>, +} + +impl Display for AlreadyBorrowedMutError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Failed to borrow because the value was already borrowed mutably.")?; + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + f.write_fmt(format_args!("borrowed_mut_at: {}", self.borrowed_mut_at))?; + Ok(()) + } +} + +impl std::error::Error for AlreadyBorrowedMutError {} + +/// An error that can occur when trying to borrow a value mutably that has already been borrowed immutably. +#[derive(Debug, Clone)] +pub struct AlreadyBorrowedError { + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_at: Vec<&'static std::panic::Location<'static>>, +} + +impl Display for AlreadyBorrowedError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Failed to borrow mutably because the value was already borrowed immutably.")?; + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + f.write_str("borrowed_at:")?; + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + for location in self.borrowed_at.iter() { + f.write_fmt(format_args!("\t{}", location))?; + } + Ok(()) + } +} + +impl std::error::Error for AlreadyBorrowedError {} + +/// A reference to a value in a generational box. +pub struct GenerationalRef<'a, T: 'static> { + inner: Ref<'a, T>, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow: GenerationalRefBorrowInfo, +} + +impl<'a, T: 'static> GenerationalRef<'a, T> { + /// Map one ref type to another. + pub fn map(orig: GenerationalRef<'a, T>, f: F) -> GenerationalRef<'a, U> + where + F: FnOnce(&T) -> &U, + { + GenerationalRef { + inner: Ref::map(orig.inner, f), + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow: GenerationalRefBorrowInfo { + borrowed_at: orig.borrow.borrowed_at, + borrowed_from: orig.borrow.borrowed_from, + }, + } + } + + /// Filter one ref type to another. + pub fn filter_map(orig: GenerationalRef<'a, T>, f: F) -> Option> + where + F: FnOnce(&T) -> Option<&U>, + { + let Self { + inner, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow, + } = orig; + Ref::filter_map(inner, f).ok().map(|inner| GenerationalRef { + inner, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow: GenerationalRefBorrowInfo { + borrowed_at: borrow.borrowed_at, + borrowed_from: borrow.borrowed_from, + }, + }) + } +} + +impl<'a, T: 'static> Deref for GenerationalRef<'a, T> { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.inner.deref() + } +} + +#[cfg(any(debug_assertions, feature = "debug_borrows"))] +struct GenerationalRefBorrowInfo { + borrowed_at: &'static std::panic::Location<'static>, + borrowed_from: &'static MemoryLocationInner, +} + +#[cfg(any(debug_assertions, feature = "debug_borrows"))] +impl Drop for GenerationalRefBorrowInfo { + fn drop(&mut self) { + self.borrowed_from + .borrowed_at + .borrow_mut() + .retain(|location| std::ptr::eq(*location, self.borrowed_at as *const _)); + } +} + +/// A mutable reference to a value in a generational box. +pub struct GenerationalRefMut<'a, T: 'static> { + inner: RefMut<'a, T>, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow: GenerationalRefMutBorrowInfo, +} + +impl<'a, T: 'static> GenerationalRefMut<'a, T> { + /// Map one ref type to another. + pub fn map(orig: GenerationalRefMut<'a, T>, f: F) -> GenerationalRefMut<'a, U> + where + F: FnOnce(&mut T) -> &mut U, + { + GenerationalRefMut { + inner: RefMut::map(orig.inner, f), + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow: orig.borrow, + } + } + + /// Filter one ref type to another. + pub fn filter_map( + orig: GenerationalRefMut<'a, T>, + f: F, + ) -> Option> + where + F: FnOnce(&mut T) -> Option<&mut U>, + { + let Self { + inner, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow, + } = orig; + RefMut::filter_map(inner, f) + .ok() + .map(|inner| GenerationalRefMut { + inner, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrow, + }) + } +} + +impl<'a, T: 'static> Deref for GenerationalRefMut<'a, T> { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.inner.deref() + } +} + +impl<'a, T: 'static> DerefMut for GenerationalRefMut<'a, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.inner.deref_mut() + } +} + +#[cfg(any(debug_assertions, feature = "debug_borrows"))] +struct GenerationalRefMutBorrowInfo { + borrowed_from: &'static MemoryLocationInner, +} + +#[cfg(any(debug_assertions, feature = "debug_borrows"))] +impl Drop for GenerationalRefMutBorrowInfo { + fn drop(&mut self) { + self.borrowed_from.borrowed_mut_at.take(); + } } /// Handles recycling generational boxes that have been dropped. Your application should have one store or one store per thread. @@ -305,12 +640,16 @@ impl Store { if let Some(location) = self.recycled.borrow_mut().pop() { location } else { - let data: &'static RefCell<_> = self.bump.alloc(RefCell::new(None)); - MemoryLocation { - data, + let data: &'static MemoryLocationInner = self.bump.alloc(MemoryLocationInner { + data: RefCell::new(None), #[cfg(any(debug_assertions, feature = "check_generation"))] - generation: self.bump.alloc(Cell::new(0)), - } + generation: Cell::new(0), + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_at: Default::default(), + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + borrowed_mut_at: Default::default(), + }); + MemoryLocation(data) } } @@ -331,9 +670,31 @@ pub struct Owner { impl Owner { /// Insert a value into the store. The value will be dropped when the owner is dropped. + #[track_caller] pub fn insert(&self, value: T) -> GenerationalBox { let mut location = self.store.claim(); - let key = location.replace(value); + let key = location.replace_with_caller( + value, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + std::panic::Location::caller(), + ); + self.owned.borrow_mut().push(location); + key + } + + /// Insert a value into the store with a specific location blamed for creating the value. The value will be dropped when the owner is dropped. + pub fn insert_with_caller( + &self, + value: T, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + caller: &'static std::panic::Location<'static>, + ) -> GenerationalBox { + let mut location = self.store.claim(); + let key = location.replace_with_caller( + value, + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + caller, + ); self.owned.borrow_mut().push(location); key } @@ -344,7 +705,9 @@ impl Owner { GenerationalBox { raw: location, #[cfg(any(debug_assertions, feature = "check_generation"))] - generation: location.generation.get(), + generation: location.0.generation.get(), + #[cfg(any(debug_assertions, feature = "check_generation"))] + created_at: std::panic::Location::caller(), _marker: PhantomData, } } diff --git a/packages/signals/src/effect.rs b/packages/signals/src/effect.rs index 0685938a0..0110ce4d1 100644 --- a/packages/signals/src/effect.rs +++ b/packages/signals/src/effect.rs @@ -83,7 +83,7 @@ impl Effect { /// Run the effect callback immediately. Returns `true` if the effect was run. Returns `false` is the effect is dead. pub fn try_run(&self) { - if let Some(mut callback) = self.callback.try_write() { + if let Ok(mut callback) = self.callback.try_write() { { get_effect_stack().effects.borrow_mut().push(*self); } diff --git a/packages/signals/src/impls.rs b/packages/signals/src/impls.rs index 6427c2e2b..8029ac312 100644 --- a/packages/signals/src/impls.rs +++ b/packages/signals/src/impls.rs @@ -1,7 +1,7 @@ use crate::rt::CopyValue; use crate::signal::{ReadOnlySignal, Signal, Write}; - -use std::cell::{Ref, RefMut}; +use generational_box::GenerationalRef; +use generational_box::GenerationalRefMut; use std::{ fmt::{Debug, Display}, @@ -38,8 +38,8 @@ macro_rules! read_impls { impl $ty> { /// Read a value from the inner vector. - pub fn get(&self, index: usize) -> Option> { - Ref::filter_map(self.read(), |v| v.get(index)).ok() + pub fn get(&self, index: usize) -> Option> { + GenerationalRef::filter_map(self.read(), |v| v.get(index)) } } @@ -52,9 +52,9 @@ macro_rules! read_impls { self.with(|v| v.clone()).unwrap() } - /// Attemps to read the inner value of the Option. - pub fn as_ref(&self) -> Option> { - Ref::filter_map(self.read(), |v| v.as_ref()).ok() + /// Attempts to read the inner value of the Option. + pub fn as_ref(&self) -> Option> { + GenerationalRef::filter_map(self.read(), |v| v.as_ref()) } } }; @@ -182,19 +182,22 @@ macro_rules! write_impls { } /// Gets the value out of the Option, or inserts the given value if the Option is empty. - pub fn get_or_insert(&self, default: T) -> Ref<'_, T> { + pub fn get_or_insert(&self, default: T) -> GenerationalRef<'_, T> { self.get_or_insert_with(|| default) } /// Gets the value out of the Option, or inserts the value returned by the given function if the Option is empty. - pub fn get_or_insert_with(&self, default: impl FnOnce() -> T) -> Ref<'_, T> { + pub fn get_or_insert_with( + &self, + default: impl FnOnce() -> T, + ) -> GenerationalRef<'_, T> { let borrow = self.read(); if borrow.is_none() { drop(borrow); self.with_mut(|v| *v = Some(default())); - Ref::map(self.read(), |v| v.as_ref().unwrap()) + GenerationalRef::map(self.read(), |v| v.as_ref().unwrap()) } else { - Ref::map(borrow, |v| v.as_ref().unwrap()) + GenerationalRef::map(borrow, |v| v.as_ref().unwrap()) } } } @@ -238,15 +241,15 @@ impl IntoIterator for CopyValue> { impl CopyValue> { /// Write to an element in the inner vector. - pub fn get_mut(&self, index: usize) -> Option> { - RefMut::filter_map(self.write(), |v| v.get_mut(index)).ok() + pub fn get_mut(&self, index: usize) -> Option> { + GenerationalRefMut::filter_map(self.write(), |v| v.get_mut(index)) } } impl CopyValue> { /// Deref the inner value mutably. - pub fn as_mut(&self) -> Option> { - RefMut::filter_map(self.write(), |v| v.as_mut()).ok() + pub fn as_mut(&self) -> Option> { + GenerationalRefMut::filter_map(self.write(), |v| v.as_mut()) } } diff --git a/packages/signals/src/rt.rs b/packages/signals/src/rt.rs index 7322d43b4..acf598217 100644 --- a/packages/signals/src/rt.rs +++ b/packages/signals/src/rt.rs @@ -1,14 +1,15 @@ -use std::cell::{Ref, RefMut}; - +use std::panic::Location; use std::rc::Rc; use dioxus_core::prelude::{ - consume_context, consume_context_from_scope, current_scope_id, provide_context, + consume_context, consume_context_from_scope, current_scope_id, has_context, provide_context, provide_context_to_scope, provide_root_context, }; use dioxus_core::ScopeId; -use generational_box::{GenerationalBox, Owner, Store}; +use generational_box::{ + BorrowError, BorrowMutError, GenerationalBox, GenerationalRef, GenerationalRefMut, Owner, Store, +}; fn current_store() -> Store { match consume_context() { @@ -21,7 +22,7 @@ fn current_store() -> Store { } fn current_owner() -> Rc { - match consume_context() { + match has_context() { Some(rt) => rt, None => { let owner = Rc::new(current_store().owner()); @@ -74,6 +75,7 @@ impl CopyValue { /// Create a new CopyValue. The value will be stored in the current component. /// /// Once the component this value is created in is dropped, the value will be dropped. + #[track_caller] pub fn new(value: T) -> Self { let owner = current_owner(); @@ -83,6 +85,15 @@ impl CopyValue { } } + pub(crate) fn new_with_caller(value: T, caller: &'static Location<'static>) -> Self { + let owner = current_owner(); + + Self { + value: owner.insert_with_caller(value, caller), + origin_scope: current_scope_id().expect("in a virtual dom"), + } + } + /// Create a new CopyValue. The value will be stored in the given scope. When the specified scope is dropped, the value will be dropped. pub fn new_in_scope(value: T, scope: ScopeId) -> Self { let owner = owner_in_scope(scope); @@ -108,22 +119,26 @@ impl CopyValue { } /// Try to read the value. If the value has been dropped, this will return None. - pub fn try_read(&self) -> Option> { + #[track_caller] + pub fn try_read(&self) -> Result, BorrowError> { self.value.try_read() } /// Read the value. If the value has been dropped, this will panic. - pub fn read(&self) -> Ref<'_, T> { + #[track_caller] + pub fn read(&self) -> GenerationalRef<'_, T> { self.value.read() } /// Try to write the value. If the value has been dropped, this will return None. - pub fn try_write(&self) -> Option> { + #[track_caller] + pub fn try_write(&self) -> Result, BorrowMutError> { self.value.try_write() } /// Write the value. If the value has been dropped, this will panic. - pub fn write(&self) -> RefMut<'_, T> { + #[track_caller] + pub fn write(&self) -> GenerationalRefMut<'_, T> { self.value.write() } diff --git a/packages/signals/src/signal.rs b/packages/signals/src/signal.rs index a5599a1ce..1656f559e 100644 --- a/packages/signals/src/signal.rs +++ b/packages/signals/src/signal.rs @@ -1,6 +1,7 @@ use std::{ - cell::{Ref, RefCell, RefMut}, + cell::RefCell, ops::{Deref, DerefMut}, + panic::Location, rc::Rc, sync::Arc, }; @@ -9,6 +10,7 @@ use dioxus_core::{ prelude::{current_scope_id, has_context, provide_context, schedule_update_any}, ScopeId, ScopeState, }; +use generational_box::{GenerationalRef, GenerationalRefMut}; use crate::{CopyValue, Effect}; @@ -43,8 +45,18 @@ use crate::{CopyValue, Effect}; /// } /// } /// ``` +#[track_caller] pub fn use_signal(cx: &ScopeState, f: impl FnOnce() -> T) -> Signal { - *cx.use_hook(|| Signal::new(f())) + #[cfg(debug_assertions)] + let caller = Location::caller(); + + *cx.use_hook(|| { + Signal::new_with_caller( + f(), + #[cfg(debug_assertions)] + caller, + ) + }) } #[derive(Clone)] @@ -134,6 +146,7 @@ impl<'de, T: serde::Deserialize<'de> + 'static> serde::Deserialize<'de> for Sign impl Signal { /// Creates a new Signal. Signals are a Copy state management solution with automatic dependency tracking. + #[track_caller] pub fn new(value: T) -> Self { Self { inner: CopyValue::new(SignalData { @@ -145,6 +158,25 @@ impl Signal { } } + /// Creates a new Signal. Signals are a Copy state management solution with automatic dependency tracking. + fn new_with_caller( + value: T, + #[cfg(debug_assertions)] caller: &'static Location<'static>, + ) -> Self { + Self { + inner: CopyValue::new_with_caller( + SignalData { + subscribers: Default::default(), + effect_subscribers: Default::default(), + update_any: schedule_update_any().expect("in a virtual dom"), + value, + }, + #[cfg(debug_assertions)] + caller, + ), + } + } + /// Get the scope the signal was created in. pub fn origin_scope(&self) -> ScopeId { self.inner.origin_scope() @@ -152,7 +184,8 @@ impl Signal { /// Get the current value of the signal. This will subscribe the current scope to the signal. /// If the signal has been dropped, this will panic. - pub fn read(&self) -> Ref { + #[track_caller] + pub fn read(&self) -> GenerationalRef { let inner = self.inner.read(); if let Some(effect) = Effect::current() { let mut effect_subscribers = inner.effect_subscribers.borrow_mut(); @@ -176,14 +209,15 @@ impl Signal { } } } - Ref::map(inner, |v| &v.value) + GenerationalRef::map(inner, |v| &v.value) } /// Get a mutable reference to the signal's value. /// If the signal has been dropped, this will panic. + #[track_caller] pub fn write(&self) -> Write<'_, T> { let inner = self.inner.write(); - let borrow = RefMut::map(inner, |v| &mut v.value); + let borrow = GenerationalRefMut::map(inner, |v| &mut v.value); Write { write: borrow, signal: SignalSubscriberDrop { signal: *self }, @@ -219,12 +253,14 @@ impl Signal { } /// Set the value of the signal. This will trigger an update on all subscribers. + #[track_caller] pub fn set(&self, value: T) { *self.write() = value; } /// Run a closure with a reference to the signal's value. /// If the signal has been dropped, this will panic. + #[track_caller] pub fn with(&self, f: impl FnOnce(&T) -> O) -> O { let write = self.read(); f(&*write) @@ -232,6 +268,7 @@ impl Signal { /// Run a closure with a mutable reference to the signal's value. /// If the signal has been dropped, this will panic. + #[track_caller] pub fn with_mut(&self, f: impl FnOnce(&mut T) -> O) -> O { let mut write = self.write(); f(&mut *write) @@ -241,6 +278,7 @@ impl Signal { impl Signal { /// Get the current value of the signal. This will subscribe the current scope to the signal. /// If the signal has been dropped, this will panic. + #[track_caller] pub fn value(&self) -> T { self.read().clone() } @@ -264,7 +302,7 @@ impl Drop for SignalSubscriberDrop { /// A mutable reference to a signal's value. pub struct Write<'a, T: 'static, I: 'static = T> { - write: RefMut<'a, T>, + write: GenerationalRefMut<'a, T>, signal: SignalSubscriberDrop, } @@ -273,7 +311,7 @@ impl<'a, T: 'static, I: 'static> Write<'a, T, I> { pub fn map(myself: Self, f: impl FnOnce(&mut T) -> &mut O) -> Write<'a, O, I> { let Self { write, signal } = myself; Write { - write: RefMut::map(write, f), + write: GenerationalRefMut::map(write, f), signal, } } @@ -284,7 +322,7 @@ impl<'a, T: 'static, I: 'static> Write<'a, T, I> { f: impl FnOnce(&mut T) -> Option<&mut O>, ) -> Option> { let Self { write, signal } = myself; - let write = RefMut::filter_map(write, f).ok(); + let write = GenerationalRefMut::filter_map(write, f); write.map(|write| Write { write, signal }) } } @@ -320,11 +358,13 @@ impl ReadOnlySignal { } /// Get the current value of the signal. This will subscribe the current scope to the signal. - pub fn read(&self) -> Ref { + #[track_caller] + pub fn read(&self) -> GenerationalRef { self.inner.read() } /// Run a closure with a reference to the signal's value. + #[track_caller] pub fn with(&self, f: impl FnOnce(&T) -> O) -> O { self.inner.with(f) } From f09a2e2280f74a9ac4e50d298e6f665bad1f8ea2 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 21 Aug 2023 14:28:45 -0500 Subject: [PATCH 2/4] fix release builds --- packages/generational-box/Cargo.toml | 1 + packages/generational-box/src/lib.rs | 26 ++++++++++++-------------- packages/signals/src/rt.rs | 12 +++++++++--- packages/signals/src/signal.rs | 5 ++--- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/packages/generational-box/Cargo.toml b/packages/generational-box/Cargo.toml index cc0db9cec..7151ec103 100644 --- a/packages/generational-box/Cargo.toml +++ b/packages/generational-box/Cargo.toml @@ -16,3 +16,4 @@ rand = "0.8.5" default = ["check_generation"] check_generation = [] debug_borrows = [] +debug_ownership = [] diff --git a/packages/generational-box/src/lib.rs b/packages/generational-box/src/lib.rs index 7d64a1ed3..1f8cbbe11 100644 --- a/packages/generational-box/src/lib.rs +++ b/packages/generational-box/src/lib.rs @@ -8,7 +8,6 @@ use std::{ fmt::{Debug, Display}, marker::PhantomData, ops::{Deref, DerefMut}, - panic::Location, rc::Rc, }; @@ -157,7 +156,7 @@ pub struct GenerationalBox { raw: MemoryLocation, #[cfg(any(debug_assertions, feature = "check_generation"))] generation: u32, - #[cfg(any(debug_assertions, feature = "check_generation"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at: &'static std::panic::Location<'static>, _marker: PhantomData, } @@ -288,9 +287,8 @@ impl MemoryLocation { fn replace_with_caller( &mut self, value: T, - #[cfg(any(debug_assertions, feature = "check_generation"))] caller: &'static Location< - 'static, - >, + #[cfg(any(debug_assertions, feature = "debug_ownership"))] + caller: &'static std::panic::Location<'static>, ) -> GenerationalBox { let mut inner_mut = self.0.data.borrow_mut(); @@ -301,7 +299,7 @@ impl MemoryLocation { raw: *self, #[cfg(any(debug_assertions, feature = "check_generation"))] generation: self.0.generation.get(), - #[cfg(any(debug_assertions, feature = "check_generation"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at: caller, _marker: PhantomData, } @@ -310,7 +308,7 @@ impl MemoryLocation { #[track_caller] fn try_borrow( &self, - #[cfg(any(debug_assertions, feature = "debug_borrows"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at: &'static std::panic::Location<'static>, ) -> Result, BorrowError> { #[cfg(any(debug_assertions, feature = "debug_borrows"))] @@ -329,7 +327,7 @@ impl MemoryLocation { }, }), Err(_) => Err(BorrowError::Dropped(ValueDroppedError { - #[cfg(any(debug_assertions, feature = "debug_borrows"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at, })), }, @@ -343,7 +341,7 @@ impl MemoryLocation { #[track_caller] fn try_borrow_mut( &self, - #[cfg(any(debug_assertions, feature = "debug_borrows"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at: &'static std::panic::Location<'static>, ) -> Result, BorrowMutError> { #[cfg(any(debug_assertions, feature = "debug_borrows"))] @@ -363,7 +361,7 @@ impl MemoryLocation { }, }), Err(_) => Err(BorrowMutError::Dropped(ValueDroppedError { - #[cfg(any(debug_assertions, feature = "debug_borrows"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at, })), } @@ -422,14 +420,14 @@ impl Error for BorrowMutError {} /// An error that can occur when trying to use a value that has been dropped. #[derive(Debug, Copy, Clone)] pub struct ValueDroppedError { - #[cfg(any(debug_assertions, feature = "debug_borrows"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at: &'static std::panic::Location<'static>, } impl Display for ValueDroppedError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str("Failed to borrow because the value was dropped.")?; - #[cfg(any(debug_assertions, feature = "debug_borrows"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] f.write_fmt(format_args!("created_at: {}", self.created_at))?; Ok(()) } @@ -686,7 +684,7 @@ impl Owner { pub fn insert_with_caller( &self, value: T, - #[cfg(any(debug_assertions, feature = "debug_borrows"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] caller: &'static std::panic::Location<'static>, ) -> GenerationalBox { let mut location = self.store.claim(); @@ -706,7 +704,7 @@ impl Owner { raw: location, #[cfg(any(debug_assertions, feature = "check_generation"))] generation: location.0.generation.get(), - #[cfg(any(debug_assertions, feature = "check_generation"))] + #[cfg(any(debug_assertions, feature = "debug_ownership"))] created_at: std::panic::Location::caller(), _marker: PhantomData, } diff --git a/packages/signals/src/rt.rs b/packages/signals/src/rt.rs index acf598217..aee3b0858 100644 --- a/packages/signals/src/rt.rs +++ b/packages/signals/src/rt.rs @@ -1,4 +1,3 @@ -use std::panic::Location; use std::rc::Rc; use dioxus_core::prelude::{ @@ -85,11 +84,18 @@ impl CopyValue { } } - pub(crate) fn new_with_caller(value: T, caller: &'static Location<'static>) -> Self { + pub(crate) fn new_with_caller( + value: T, + #[cfg(debug_assertions)] caller: &'static std::panic::Location<'static>, + ) -> Self { let owner = current_owner(); Self { - value: owner.insert_with_caller(value, caller), + value: owner.insert_with_caller( + value, + #[cfg(debug_assertions)] + caller, + ), origin_scope: current_scope_id().expect("in a virtual dom"), } } diff --git a/packages/signals/src/signal.rs b/packages/signals/src/signal.rs index 1656f559e..70a809e85 100644 --- a/packages/signals/src/signal.rs +++ b/packages/signals/src/signal.rs @@ -1,7 +1,6 @@ use std::{ cell::RefCell, ops::{Deref, DerefMut}, - panic::Location, rc::Rc, sync::Arc, }; @@ -48,7 +47,7 @@ use crate::{CopyValue, Effect}; #[track_caller] pub fn use_signal(cx: &ScopeState, f: impl FnOnce() -> T) -> Signal { #[cfg(debug_assertions)] - let caller = Location::caller(); + let caller = std::panic::Location::caller(); *cx.use_hook(|| { Signal::new_with_caller( @@ -161,7 +160,7 @@ impl Signal { /// Creates a new Signal. Signals are a Copy state management solution with automatic dependency tracking. fn new_with_caller( value: T, - #[cfg(debug_assertions)] caller: &'static Location<'static>, + #[cfg(debug_assertions)] caller: &'static std::panic::Location<'static>, ) -> Self { Self { inner: CopyValue::new_with_caller( From 18f8daf0bdc30a6eb71ab7e91863442a13d47ca6 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 21 Aug 2023 15:00:31 -0500 Subject: [PATCH 3/4] fix generational box tests --- packages/generational-box/README.md | 2 ++ packages/generational-box/src/lib.rs | 13 ++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/generational-box/README.md b/packages/generational-box/README.md index 5ab7926ba..f8d4885b8 100644 --- a/packages/generational-box/README.md +++ b/packages/generational-box/README.md @@ -11,6 +11,8 @@ Three main types manage state in Generational Box: Example: ```rust +use generational_box::Store; + // Create a store for this thread let store = Store::default(); diff --git a/packages/generational-box/src/lib.rs b/packages/generational-box/src/lib.rs index 1f8cbbe11..84a81769d 100644 --- a/packages/generational-box/src/lib.rs +++ b/packages/generational-box/src/lib.rs @@ -32,12 +32,12 @@ fn reused() { let first_ptr; { let owner = store.owner(); - first_ptr = owner.insert(1).raw.data.as_ptr(); + first_ptr = owner.insert(1).raw.0.data.as_ptr(); drop(owner); } { let owner = store.owner(); - let second_ptr = owner.insert(1234).raw.data.as_ptr(); + let second_ptr = owner.insert(1234).raw.0.data.as_ptr(); assert_eq!(first_ptr, second_ptr); drop(owner); } @@ -56,7 +56,10 @@ fn leaking_is_ok() { // don't drop the owner std::mem::forget(owner); } - assert_eq!(key.try_read().as_deref(), Some(&"hello world".to_string())); + assert_eq!( + key.try_read().as_deref().unwrap(), + &"hello world".to_string() + ); } #[test] @@ -71,7 +74,7 @@ fn drops() { key = owner.insert(data); // drop the owner } - assert!(key.try_read().is_none()); + assert!(key.try_read().is_err()); } #[test] @@ -132,7 +135,7 @@ fn fuzz() { println!("{:?}", path); for key in valid_keys.iter() { let value = key.read(); - println!("{:?}", value); + println!("{:?}", &*value); assert!(value.starts_with("hello world")); } #[cfg(any(debug_assertions, feature = "check_generation"))] From fdec47b90bc09945ccb5f26012b492f38587d872 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 1 Nov 2023 08:19:35 -0500 Subject: [PATCH 4/4] fix clippy --- packages/generational-box/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/generational-box/src/lib.rs b/packages/generational-box/src/lib.rs index 628308340..f56904ede 100644 --- a/packages/generational-box/src/lib.rs +++ b/packages/generational-box/src/lib.rs @@ -485,7 +485,7 @@ pub struct GenerationalRef { borrow: GenerationalRefBorrowInfo, } -impl<'a, T: 'static> GenerationalRef { +impl GenerationalRef { /// Map one ref type to another. pub fn map(orig: GenerationalRef, f: F) -> GenerationalRef where @@ -522,7 +522,7 @@ impl<'a, T: 'static> GenerationalRef { } } -impl<'a, T: 'static> Deref for GenerationalRef { +impl Deref for GenerationalRef { type Target = T; fn deref(&self) -> &Self::Target { @@ -586,7 +586,7 @@ impl GenerationalRefMut { } } -impl<'a, T: 'static> Deref for GenerationalRefMut { +impl Deref for GenerationalRefMut { type Target = T; fn deref(&self) -> &Self::Target { @@ -594,7 +594,7 @@ impl<'a, T: 'static> Deref for GenerationalRefMut { } } -impl<'a, T: 'static> DerefMut for GenerationalRefMut { +impl DerefMut for GenerationalRefMut { fn deref_mut(&mut self) -> &mut Self::Target { self.inner.deref_mut() }