Skip to content

Commit

Permalink
fix: remove unsound Compose impl for Map and create MapUnchecked struct
Browse files Browse the repository at this point in the history
  • Loading branch information
matthunz committed Dec 3, 2024
1 parent fd390af commit 20897dc
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/compose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl<C: Compose> Compose for Catch<'_, C> {
let f: &dyn Fn(Box<dyn StdError>) = unsafe { mem::transmute(f) };
use_provider(&cx, move || CatchContext { f: Box::new(f) });

Signal::map(cx.me(), |me| &me.content)
unsafe { Signal::map_unchecked(cx.me(), |me| &me.content) }
}
}

Expand Down Expand Up @@ -349,7 +349,7 @@ where
cx.is_parent_changed.set(true);
}

Signal::map(cx.me(), |me| &me.content)
unsafe { Signal::map_unchecked(cx.me(), |me| &me.content) }
}

fn name() -> Option<Cow<'static, str>> {
Expand Down
4 changes: 2 additions & 2 deletions src/ecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl<C: Compose> Compose for CompositionContent<C> {
parent_entity: cx.me().target,
});

Signal::map(cx.me(), |me| &me.content)
unsafe { Signal::map_unchecked(cx.me(), |me| &me.content) }
}
}

Expand Down Expand Up @@ -618,7 +618,7 @@ impl<C: Compose> Compose for Spawn<'_, C> {
*guard.lock().unwrap() = false;
});

Signal::map(cx.me(), |me| &me.content)
unsafe { Signal::map_unchecked(cx.me(), |me| &me.content) }
}
}

Expand Down
29 changes: 25 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,16 @@ impl<T> Deref for Map<'_, T> {
}
}

// Safety: The `Map` is dereferenced every re-compose, so it's guranteed not to point to
// an invalid memory location (e.g. an `Option` that previously returned `Some` is now `None`).
impl<C: Compose> Compose for Map<'_, C> {
/// Unchecked, mapped immutable reference to a value of type `T`.
///
/// This can be created with [`Signal::map_unchecked`].
pub struct MapUnchecked<'a, T> {
map: Map<'a, T>,
}

unsafe impl<T> Data for MapUnchecked<'_, T> {}

impl<C: Compose> Compose for MapUnchecked<'_, C> {
fn compose(cx: Scope<Self>) -> impl Compose {
cx.is_container.set(true);

Expand All @@ -368,7 +375,9 @@ impl<C: Compose> Compose for Map<'_, C> {

state.is_parent_changed.set(cx.is_parent_changed.get());

unsafe { (**cx.me()).any_compose(state) }
// Safety: The `Map` is dereferenced every re-compose, so it's guranteed not to point to
// an invalid memory location (e.g. an `Option` that previously returned `Some` is now `None`).
unsafe { (*cx.me().map).any_compose(state) }
}
}

Expand Down Expand Up @@ -408,6 +417,18 @@ impl<'a, T> Signal<'a, T> {
generation: me.generation,
}
}

/// Unsafely map this reference to a value of type `U`.
/// The returned `MapUnchecked` implements `Compose` to allow for borrowed child composables.
///
/// # Safety
/// The returned `MapUnchecked` must only be returned once.
/// Composing the same `MapUnchecked` at multiple locations in the tree at the same time will result in undefined behavior.
pub unsafe fn map_unchecked<U>(me: Self, f: fn(&T) -> &U) -> MapUnchecked<'a, U> {
MapUnchecked {
map: Signal::map(me, f),
}
}
}

impl<T> Deref for Signal<'_, T> {
Expand Down

0 comments on commit 20897dc

Please sign in to comment.