Skip to content

Commit

Permalink
fix(core): 🐛 remove PartData to prevent the use of a reference to cre…
Browse files Browse the repository at this point in the history
…ate a write reference
  • Loading branch information
M-Adoo committed Jan 11, 2025
1 parent 32ef3fd commit 9cec0da
Show file tree
Hide file tree
Showing 16 changed files with 193 additions and 114 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he
- **macros**: Added the `part_reader!` macro to generate a partial reader from a reference of a reader. (#688 @M-Adoo)
- **macros**: The `simple_declare` now supports the `stateless` meta attribute, `#[simple_declare(stateless)]`. (#688 @M-Adoo)

### Fixed

- **Core**: `PartData` allows the use of a reference to create a write reference, which is unsafe. Introduce `PartRef` and `PartMut` to replace it. (#pr @M-Adoo)


### Breading

- **core**: Removed `PartData`. (#pr @M-Adoo)

## [0.4.0-alpha.22] - 2025-01-08

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion core/src/builtin_widgets/painting_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl PaintingStyleWidget {
match this.try_into_value() {
Ok(this) => Provider::new(this.painting_style),
Err(this) => Provider::value_of_writer(
this.map_writer(|w| PartData::from_ref_mut(&mut w.painting_style)),
this.map_writer(|w| PartMut::new(&mut w.painting_style)),
Some(DirtyPhase::LayoutSubtree),
),
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/builtin_widgets/text_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl TextStyleWidget {
match this.try_into_value() {
Ok(this) => Provider::new(this.text_style),
Err(this) => Provider::value_of_writer(
this.map_writer(|w| PartData::from_ref_mut(&mut w.text_style)),
this.map_writer(|w| PartMut::new(&mut w.text_style)),
Some(DirtyPhase::LayoutSubtree),
),
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/builtin_widgets/theme.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,11 @@ impl Theme {
load_fonts(&this);
let container_color = this.map_reader(|t| {
// Safety Note: In this instance, a copied value of the palette is utilized,
// which is not the correct method of using `PartData`. However, in this case,
// which is not the correct method of using `PartRef`. However, in this case,
// it is only a read-only value, and once added to the providers, neither the
// state reader nor its read reference can be accessed by anyone. Therefore, it
// is considered safe.
unsafe { PartData::from_ptr(ContainerColor(t.palette.secondary_container())) }
unsafe { PartRef::from_ptr(ContainerColor(t.palette.secondary_container())) }
});

let providers = smallvec![
Expand Down
4 changes: 2 additions & 2 deletions core/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ impl dyn QueryAny {
#[cfg(test)]
mod tests {
use super::*;
use crate::{prelude::PartData, reset_test_env, state::State};
use crate::{prelude::PartMut, reset_test_env, state::State};

#[test]
fn query_ref() {
Expand Down Expand Up @@ -430,7 +430,7 @@ mod tests {
}

let x = State::value(X { a: 0, _b: 1 });
let y = x.split_writer(|x| PartData::from_ref_mut(&mut x.a));
let y = x.split_writer(|x| PartMut::new(&mut x.a));
{
let h = y.query(&QueryId::of::<i32>()).unwrap();
assert!(h.downcast_ref::<i32>().is_some());
Expand Down
46 changes: 24 additions & 22 deletions core/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub use prior_op::*;
use ribir_algo::Sc;
use rxrust::ops::box_it::{BoxOp, CloneableBoxOp};
pub use splitted_state::*;
pub use state_cell::{PartData, ReadRef};
pub use state_cell::*;
use state_cell::{StateCell, ValueMutRef};
pub use stateful::*;
pub use watcher::*;
Expand Down Expand Up @@ -42,7 +42,7 @@ pub trait StateReader: 'static {
#[inline]
fn map_reader<U: ?Sized, F>(&self, map: F) -> MapReader<Self::Reader, F>
where
F: Fn(&Self::Value) -> PartData<U> + Clone,
F: Fn(&Self::Value) -> PartRef<U> + Clone,
{
MapReader { origin: self.clone_reader(), part_map: map }
}
Expand Down Expand Up @@ -123,7 +123,7 @@ pub trait StateWriter: StateWatcher {
#[inline]
fn split_writer<V: ?Sized, W>(&self, mut_map: W) -> SplittedWriter<Self::Writer, W>
where
W: Fn(&mut Self::Value) -> PartData<V> + Clone,
W: Fn(&mut Self::Value) -> PartMut<V> + Clone,
{
SplittedWriter::new(self.clone_writer(), mut_map)
}
Expand All @@ -142,7 +142,7 @@ pub trait StateWriter: StateWatcher {
#[inline]
fn map_writer<V: ?Sized, M>(&self, part_map: M) -> MapWriter<Self::Writer, M>
where
M: Fn(&mut Self::Value) -> PartData<V> + Clone,
M: Fn(&mut Self::Value) -> PartMut<V> + Clone,
{
let origin = self.clone_writer();
MapWriter { origin, part_map }
Expand Down Expand Up @@ -270,9 +270,9 @@ impl<W> State<W> {
impl<'a, V: ?Sized> WriteRef<'a, V> {
pub fn map<U: ?Sized, M>(mut orig: WriteRef<'a, V>, part_map: M) -> WriteRef<'a, U>
where
M: Fn(&mut V) -> PartData<U>,
M: Fn(&mut V) -> PartMut<U>,
{
let inner = part_map(&mut orig.value);
let inner = part_map(&mut orig.value).inner;
let borrow = orig.value.borrow.clone();
let value = ValueMutRef { inner, borrow };

Expand All @@ -295,17 +295,18 @@ impl<'a, V: ?Sized> WriteRef<'a, V> {
/// let c = Stateful::new(vec![1, 2, 3]);
/// let b1: WriteRef<'_, Vec<u32>> = c.write();
/// let b2: Result<WriteRef<'_, u32>, _> =
/// WriteRef::filter_map(b1, |v| v.get(1).map(PartData::from_ref));
/// WriteRef::filter_map(b1, |v| v.get_mut(1).map(PartMut::<u32>::new));
/// assert_eq!(*b2.unwrap(), 2);
/// ```
pub fn filter_map<U: ?Sized, M>(
mut orig: WriteRef<'a, V>, part_map: M,
) -> Result<WriteRef<'a, U>, Self>
where
M: Fn(&mut V) -> Option<PartData<U>>,
M: Fn(&mut V) -> Option<PartMut<U>>,
{
match part_map(&mut orig.value) {
Some(inner) => {
let inner = inner.inner;
let borrow = orig.value.borrow.clone();
let value = ValueMutRef { inner, borrow };
let WriteRef { modify_scope, info, .. } = orig;
Expand All @@ -320,10 +321,11 @@ impl<'a, V: ?Sized> WriteRef<'a, V> {
mut orig: WriteRef<'a, V>, f: F,
) -> (WriteRef<'a, U1>, WriteRef<'a, U2>)
where
F: FnOnce(&mut V) -> (PartData<U1>, PartData<U2>),
F: FnOnce(&mut V) -> (PartMut<U1>, PartMut<U2>),
{
let WriteRef { info, modify_scope, modified, .. } = orig;
let (a, b) = f(&mut *orig.value);
let (a, b) = (a.inner, b.inner);
let borrow = orig.value.borrow.clone();
let a = ValueMutRef { inner: a, borrow: borrow.clone() };
let b = ValueMutRef { inner: b, borrow };
Expand Down Expand Up @@ -445,7 +447,7 @@ mod tests {
reset_test_env!();

let origin = State::value(Origin { a: 0, b: 0 });
let map_state = origin.map_writer(|v| PartData::from_ref_mut(&mut v.b));
let map_state = origin.map_writer(|v| PartMut::new(&mut v.b));

let track_origin = Sc::new(Cell::new(0));
let track_map = Sc::new(Cell::new(0));
Expand Down Expand Up @@ -481,7 +483,7 @@ mod tests {
reset_test_env!();

let origin = State::value(Origin { a: 0, b: 0 });
let split = origin.split_writer(|v| PartData::from_ref_mut(&mut v.b));
let split = origin.split_writer(|v| PartMut::new(&mut v.b));

let track_origin = Sc::new(Cell::new(0));
let track_split = Sc::new(Cell::new(0));
Expand Down Expand Up @@ -536,11 +538,11 @@ mod tests {

let _map_writer_compose_widget = fn_widget! {
Stateful::new((C, 0))
.map_writer(|v| PartData::from_ref_mut(&mut v.0))
.map_writer(|v| PartMut::new(&mut v.0))
};
let _split_writer_compose_widget = fn_widget! {
Stateful::new((C, 0))
.split_writer(|v| PartData::from_ref_mut(&mut v.0))
.split_writer(|v| PartMut::new(&mut v.0))
};
}

Expand Down Expand Up @@ -586,24 +588,24 @@ mod tests {

let _map_writer_with_child = fn_widget! {
let w = Stateful::new((CC, 0))
.map_writer(|v| PartData::from_ref_mut(&mut v.0));
.map_writer(|v| PartMut::new(&mut v.0));
@$w { @{ Void } }
};

let _map_writer_without_child = fn_widget! {
Stateful::new((CC, 0))
.map_writer(|v| PartData::from_ref_mut(&mut v.0))
.map_writer(|v| PartMut::new(&mut v.0))
};

let _split_writer_with_child = fn_widget! {
let w = Stateful::new((CC, 0))
.split_writer(|v| PartData::from_ref_mut(&mut v.0));
.split_writer(|v| PartMut::new(&mut v.0));
@$w { @{ Void } }
};

let _split_writer_without_child = fn_widget! {
Stateful::new((CC, 0))
.split_writer(|v| PartData::from_ref_mut(&mut v.0))
.split_writer(|v| PartMut::new(&mut v.0))
};
}

Expand All @@ -625,17 +627,17 @@ mod tests {
};

let _map_reader_render_widget = fn_widget! {
Stateful::new((Void, 0)).map_reader(|v| PartData::from_ref(&v.0))
Stateful::new((Void, 0)).map_reader(|v| PartRef::new(&v.0))
};

let _map_writer_render_widget = fn_widget! {
Stateful::new((Void, 0))
.map_writer(|v| PartData::from_ref_mut(&mut v.0))
.map_writer(|v| PartMut::new(&mut v.0))
};

let _split_writer_render_widget = fn_widget! {
Stateful::new((Void, 0))
.split_writer(|v| PartData::from_ref_mut(&mut v.0))
.split_writer(|v| PartMut::new(&mut v.0))
};
}

Expand All @@ -644,11 +646,11 @@ mod tests {
fn trait_object_part_data() {
reset_test_env!();
let s = State::value(0);
let m = s.split_writer(|v| PartData::from_ref(v as &mut dyn Any));
let m = s.split_writer(|v| PartMut::new(v as &mut dyn Any));
let v: ReadRef<dyn Any> = m.read();
assert_eq!(*v.downcast_ref::<i32>().unwrap(), 0);

let s = s.map_writer(|v| PartData::from_ref(v as &mut dyn Any));
let s = s.map_writer(|v| PartMut::new(v as &mut dyn Any));
let v: ReadRef<dyn Any> = s.read();
assert_eq!(*v.downcast_ref::<i32>().unwrap(), 0);
}
Expand Down
14 changes: 7 additions & 7 deletions core/src/state/map_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl<S, V: ?Sized, M> StateReader for MapReader<S, M>
where
Self: 'static,
S: StateReader,
M: Fn(&S::Value) -> PartData<V> + Clone,
M: Fn(&S::Value) -> PartRef<V> + Clone,
{
type Value = V;
type OriginReader = S;
Expand Down Expand Up @@ -52,7 +52,7 @@ impl<S, V: ?Sized, M> StateReader for MapWriterAsReader<S, M>
where
Self: 'static,
S: StateReader,
M: Fn(&mut S::Value) -> PartData<V> + Clone,
M: Fn(&mut S::Value) -> PartMut<V> + Clone,
{
type Value = V;
type OriginReader = S;
Expand Down Expand Up @@ -84,7 +84,7 @@ impl<V: ?Sized, S, M> StateReader for MapWriter<S, M>
where
Self: 'static,
S: StateWriter,
M: Fn(&mut S::Value) -> PartData<V> + Clone,
M: Fn(&mut S::Value) -> PartMut<V> + Clone,
{
type Value = V;
type OriginReader = S;
Expand Down Expand Up @@ -116,7 +116,7 @@ impl<V: ?Sized, W, M> StateWatcher for MapWriter<W, M>
where
Self: 'static,
W: StateWriter,
M: Fn(&mut W::Value) -> PartData<V> + Clone,
M: Fn(&mut W::Value) -> PartMut<V> + Clone,
{
#[inline]
fn raw_modifies(&self) -> CloneableBoxOp<'static, ModifyScope, Infallible> {
Expand All @@ -128,7 +128,7 @@ impl<V: ?Sized, W, M> StateWriter for MapWriter<W, M>
where
Self: 'static,
W: StateWriter,
M: Fn(&mut W::Value) -> PartData<V> + Clone,
M: Fn(&mut W::Value) -> PartMut<V> + Clone,
{
type Writer = MapWriter<W::Writer, M>;
type OriginWriter = W;
Expand Down Expand Up @@ -165,7 +165,7 @@ impl<V: ?Sized, S, F> RenderProxy for MapReader<S, F>
where
Self: 'static,
S: StateReader,
F: Fn(&S::Value) -> PartData<V> + Clone,
F: Fn(&S::Value) -> PartRef<V> + Clone,
V: Render,
{
#[inline]
Expand All @@ -176,7 +176,7 @@ impl<V: ?Sized, S, F> RenderProxy for MapWriterAsReader<S, F>
where
Self: 'static,
S: StateReader,
F: Fn(&mut S::Value) -> PartData<V> + Clone,
F: Fn(&mut S::Value) -> PartMut<V> + Clone,
V: Render,
{
fn proxy(&self) -> impl Deref<Target = impl Render + ?Sized> { self.read() }
Expand Down
14 changes: 8 additions & 6 deletions core/src/state/splitted_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ impl<V: ?Sized, O, W> StateReader for SplittedWriter<O, W>
where
Self: 'static,
O: StateWriter,
W: Fn(&mut O::Value) -> PartData<V> + Clone,
W: Fn(&mut O::Value) -> PartMut<V> + Clone,
{
type Value = V;
type OriginReader = O;
Expand Down Expand Up @@ -48,7 +48,7 @@ impl<V: ?Sized, O, W> StateWatcher for SplittedWriter<O, W>
where
Self: 'static,
O: StateWriter,
W: Fn(&mut O::Value) -> PartData<V> + Clone,
W: Fn(&mut O::Value) -> PartMut<V> + Clone,
{
fn raw_modifies(&self) -> CloneableBoxOp<'static, ModifyScope, std::convert::Infallible> {
self.info.notifier.raw_modifies().box_it()
Expand All @@ -59,7 +59,7 @@ impl<V: ?Sized, O, W> StateWriter for SplittedWriter<O, W>
where
Self: 'static,
O: StateWriter,
W: Fn(&mut O::Value) -> PartData<V> + Clone,
W: Fn(&mut O::Value) -> PartMut<V> + Clone,
{
type Writer = SplittedWriter<O::Writer, W>;
type OriginWriter = O;
Expand Down Expand Up @@ -93,7 +93,7 @@ where
impl<V: ?Sized, O, W> SplittedWriter<O, W>
where
O: StateWriter,
W: Fn(&mut O::Value) -> PartData<V> + Clone,
W: Fn(&mut O::Value) -> PartMut<V> + Clone,
{
pub(super) fn new(origin: O, mut_map: W) -> Self {
Self { origin, splitter: mut_map, info: Sc::new(WriterInfo::new()) }
Expand All @@ -108,8 +108,10 @@ where
assert!(!orig.modified);
orig.modify_scope.remove(ModifyScope::FRAMEWORK);
orig.modified = true;
let value =
ValueMutRef { inner: (self.splitter)(&mut orig.value), borrow: orig.value.borrow.clone() };
let value = ValueMutRef {
inner: (self.splitter)(&mut orig.value).inner,
borrow: orig.value.borrow.clone(),
};

WriteRef { value, modified: false, modify_scope, info: &self.info }
}
Expand Down
Loading

0 comments on commit 9cec0da

Please sign in to comment.