dnrops
/
sycamore
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Make `create_ref` only allow `T: 'static` (#519) 

* Make create_ref only allow `T: 'static`

* Add missing safety docs

* Fix web integration tests

* Remove some old tests and fix effect cleanup

* Try to fix github actions bench workflow
This commit is contained in:
Luke 2022-11-26 14:37:15 +00:00 committed by GitHub
parent a770858ec5
commit 9d7af243d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 113 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.github/workflows/bench.yml vendored
View File

@ -25,6 +25,7 @@ jobs:
- name: Checkout repository head
uses: actions/checkout@v2
with:
repository: ${{ github.repository }}
ref: ${{ github.head_ref }}
clean: false # Do not remove benchmark results from base branch

4
examples/motion/src/main.rs
View File

@ -13,8 +13,8 @@ fn CreateRAF<G: Html>(cx: Scope) -> View<G> {
view! { cx,
div {
p { (state.get()) " frames" }
button(on:click=|_| start()) { "Start" }
button(on:click=|_| stop()) { "Stop" }
button(on:click=move |_| start()) { "Start" }
button(on:click=move |_| stop()) { "Stop" }
}
}
}

10
packages/sycamore-reactive/examples/signal-of-signal.rs
View File

@ -1,10 +0,0 @@
use sycamore_reactive::*;
fn main() {
create_scope_immediate(|cx| {
let data = create_signal(cx, 123);
dbg!(data.get());
let signal_ref = create_signal(cx, data);
dbg!(signal_ref.get().get());
});
}

28
packages/sycamore-reactive/src/arena.rs
View File

@ -23,16 +23,26 @@ impl<'a> ScopeArena<'a> {
/// Allocate a value onto the arena. Returns a mutable reference that lasts as long as the arena
/// itself.
#[allow(clippy::mut_from_ref)] // We return a new reference each time so this is a false-positive.
pub fn alloc<T: 'a>(&'a self, value: T) -> &'a mut T {
pub fn alloc<T: 'static>(&'a self, value: T) -> &'a mut T {
// SAFETY: We know that the lifetime is `'static` so this is completely safe.
unsafe { self.alloc_non_static(value) }
}
/// Allocate a value onto the arena. Returns a mutable reference that lasts as long as the arena
/// itself.
///
/// # Safety
///
/// See docs for [`create_ref`](crate::create_ref).
#[allow(clippy::mut_from_ref)] // We return a new reference each time so this is a false-positive.
pub unsafe fn alloc_non_static<T: 'a>(&'a self, value: T) -> &'a mut T {
let boxed = bumpalo::boxed::Box::new_in(value, &self.bump);
let ptr = bumpalo::boxed::Box::into_raw(boxed);
unsafe {
// SAFETY: The only place where self.inner.get() is mutably borrowed is right here.
// It is impossible to have two alloc() calls on the same ScopeArena at the same time so
// the mutable reference here is effectively unique.
let inner_exclusive = &mut *self.inner.get();
inner_exclusive.push(ptr);
};
// SAFETY: The only place where self.inner.get() is mutably borrowed is right here.
// It is impossible to have two alloc() calls on the same ScopeArena at the same time so
// the mutable reference here is effectively unique.
let inner_exclusive = &mut *self.inner.get();
inner_exclusive.push(ptr);
// SAFETY: the address of the ptr lives as long as 'a because:
// - It is allocated on the heap and therefore has a stable address.
@ -40,7 +50,7 @@ impl<'a> ScopeArena<'a> {
// dropped.
// - The drop code for Scope never reads the allocated value and therefore does not create a
// stacked-borrows violation.
unsafe { &mut *ptr }
&mut *ptr
}
/// Cleanup the resources owned by the [`ScopeArena`]. This is automatically called in [`Drop`].

8
packages/sycamore-reactive/src/effect.rs
View File

@ -69,13 +69,13 @@ impl<'a> EffectState<'a> {
/// # });
/// ```
pub fn create_effect<'a>(cx: Scope<'a>, f: impl FnMut() + 'a) {
let f = cx.alloc(f);
_create_effect(cx, f)
_create_effect(cx, Box::new(f))
}
/// Internal implementation for `create_effect`. Use dynamic dispatch to reduce code-bloat.
fn _create_effect<'a>(cx: Scope<'a>, f: &'a mut (dyn FnMut() + 'a)) {
let effect = &*cx.alloc(RefCell::new(None::<EffectState<'a>>));
fn _create_effect<'a>(cx: Scope<'a>, mut f: Box<(dyn FnMut() + 'a)>) {
// SAFETY: We do not access the scope in the Drop implementation for EffectState.
let effect = unsafe { create_ref_unsafe(cx, RefCell::new(None::<EffectState<'a>>)) };
let cb = Rc::new(RefCell::new({
move || {
EFFECTS.with(|effects| {

4
packages/sycamore-reactive/src/iter.rs
View File

@ -21,7 +21,7 @@ use crate::*;
/// * `key_fn` - A closure that returns an _unique_ key to each entry.
///
/// _Credits: Based on TypeScript implementation in <https://github.com/solidjs/solid>_
pub fn map_keyed<'a, T, K, U>(
pub fn map_keyed<'a, T, K, U: 'static>(
cx: Scope<'a>,
list: &'a ReadSignal<Vec<T>>,
map_fn: impl for<'child_lifetime> Fn(BoundedScope<'child_lifetime, 'a>, T) -> U + 'a,
@ -194,7 +194,7 @@ where
/// * `list` - The list to be mapped. The list must be a [`ReadSignal`] (obtained from a [`Signal`])
/// and therefore reactive.
/// * `map_fn` - A closure that maps from the input type to the output type.
pub fn map_indexed<'a, T, U>(
pub fn map_indexed<'a, T, U: 'static>(
cx: Scope<'a>,
list: &'a ReadSignal<Vec<T>>,
map_fn: impl for<'child_lifetime> Fn(BoundedScope<'child_lifetime, 'a>, T) -> U + 'a,

68
packages/sycamore-reactive/src/lib.rs
View File

@ -99,11 +99,6 @@ impl<'a, 'b: 'a> BoundedScope<'a, 'b> {
_phantom: PhantomData,
}
}
/// Alias for `self.raw.arena.alloc`.
fn alloc<T>(&self, value: T) -> &'a mut T {
self.raw.arena.alloc(value)
}
}
/// A type-alias for [`BoundedScope`] where both lifetimes are the same.
@ -342,10 +337,43 @@ pub fn create_scope_immediate(f: impl for<'a> FnOnce(Scope<'a>)) {
/// let _ = outer.unwrap();
/// # });
/// ```
pub fn create_ref<T>(cx: Scope, value: T) -> &T {
pub fn create_ref<T: 'static>(cx: Scope, value: T) -> &T {
cx.raw.arena.alloc(value)
}
/// Allocate a new arbitrary value under the current [`Scope`].
/// The allocated value lasts as long as the scope and cannot be used outside of the scope.
///
/// # Ref lifetime
///
/// The lifetime of the returned ref is the same as the [`Scope`].
/// As such, the reference cannot escape the [`Scope`].
/// ```compile_fail
/// # use sycamore_reactive::*;
/// # create_scope_immediate(|cx| {
/// let mut outer = None;
/// let disposer = create_child_scope(cx, |cx| {
/// let data = create_ref(cx, 0);
/// let raw: &i32 = &data;
/// outer = Some(raw);
/// // ^^^
/// });
/// disposer();
/// let _ = outer.unwrap();
/// # });
/// ```
///
/// # Safety
///
/// The allocated value must not access any value allocated on the scope in its `Drop`
/// implementation.
///
/// This should almost never happen in the wild, but beware that accessing allocated data in `Drop`
/// might cause an use-after-free because the accessed value might have been dropped already.
pub unsafe fn create_ref_unsafe<'a, T: 'a>(cx: Scope<'a>, value: T) -> &'a T {
cx.raw.arena.alloc_non_static(value)
}
/// Adds a callback that is called when the scope is destroyed.
pub fn on_cleanup<'a>(cx: Scope<'a>, f: impl FnOnce() + 'a) {
cx.raw.inner.borrow_mut().cleanups.push(Box::new(f));
@ -511,15 +539,6 @@ mod tests {
});
}
#[test]
fn can_store_disposer_in_own_signal() {
create_scope_immediate(|cx| {
let signal = create_signal(cx, None);
let disposer = create_child_scope(cx, |_cx| {});
signal.set(Some(disposer));
});
}
#[test]
fn refs_are_dropped_on_dispose() {
thread_local! {
@ -561,23 +580,4 @@ mod tests {
unsafe { disposer.dispose() };
}
#[test]
fn access_previous_ref_in_drop() {
struct ReadRefOnDrop<'a> {
r: &'a i32,
expect: i32,
}
impl<'a> Drop for ReadRefOnDrop<'a> {
fn drop(&mut self) {
assert_eq!(*self.r, self.expect);
}
}
let disposer = create_scope(|cx| {
let r = create_ref(cx, 123);
create_ref(cx, ReadRefOnDrop { r, expect: 123 });
});
unsafe { disposer.dispose() };
}
}

29
packages/sycamore-reactive/src/memo.rs
View File

@ -45,7 +45,7 @@ use crate::*;
/// assert_eq!(*double.get(), 2);
/// # });
/// ```
pub fn create_memo<'a, U: 'a>(cx: Scope<'a>, f: impl FnMut() -> U + 'a) -> &'a ReadSignal<U> {
pub fn create_memo<'a, U: 'static>(cx: Scope<'a>, f: impl FnMut() -> U + 'a) -> &'a ReadSignal<U> {
create_selector_with(cx, f, |_, _| false)
}
@ -69,7 +69,7 @@ pub fn create_memo<'a, U: 'a>(cx: Scope<'a>, f: impl FnMut() -> U + 'a) -> &'a R
/// assert_eq!(*double.get(), 2);
/// # });
/// ```
pub fn create_selector<'a, U: PartialEq + 'a>(
pub fn create_selector<'a, U: PartialEq + 'static>(
cx: Scope<'a>,
f: impl FnMut() -> U + 'a,
) -> &'a ReadSignal<U> {
@ -85,22 +85,25 @@ pub fn create_selector<'a, U: PartialEq + 'a>(
///
/// To use the type's [`PartialEq`] implementation instead of a custom function, use
/// [`create_selector`].
pub fn create_selector_with<'a, U: 'a>(
pub fn create_selector_with<'a, U: 'static>(
cx: Scope<'a>,
mut f: impl FnMut() -> U + 'a,
eq_f: impl Fn(&U, &U) -> bool + 'a,
) -> &'a ReadSignal<U> {
let signal: &Cell<Option<&Signal<U>>> = create_ref(cx, Cell::new(None));
let signal: Rc<Cell<Option<&Signal<U>>>> = Rc::new(Cell::new(None));
create_effect(cx, move || {
let new = f();
if let Some(signal) = signal.get() {
// Check if new value is different from old value.
if !eq_f(&new, &*signal.get_untracked()) {
signal.set(new)
create_effect(cx, {
let signal = Rc::clone(&signal);
move || {
let new = f();
if let Some(signal) = signal.get() {
// Check if new value is different from old value.
if !eq_f(&new, &*signal.get_untracked()) {
signal.set(new)
}
} else {
signal.set(Some(create_signal(cx, new)))
}
} else {
signal.set(Some(create_signal(cx, new)))
}
});
@ -140,7 +143,7 @@ pub fn create_selector_with<'a, U: 'a>(
/// assert_eq!(*state.get(), 0);
/// # });
/// ```
pub fn create_reducer<'a, U, Msg>(
pub fn create_reducer<'a, U: 'static, Msg>(
cx: Scope<'a>,
initial: U,
reduce: impl Fn(&U, Msg) -> U + 'a,

6
packages/sycamore-reactive/src/signal.rs
View File

@ -164,7 +164,7 @@ impl<T> ReadSignal<T> {
/// # });
/// ```
#[must_use]
pub fn map<'a, U>(
pub fn map<'a, U: 'static>(
&'a self,
cx: Scope<'a>,
mut f: impl FnMut(&T) -> U + 'a,
@ -461,7 +461,7 @@ impl<'a, T> AnyReadSignal<'a> for ReadSignal<T> {
/// outer = Some(signal);
/// });
/// ```
pub fn create_signal<T>(cx: Scope, value: T) -> &Signal<T> {
pub fn create_signal<T: 'static>(cx: Scope, value: T) -> &Signal<T> {
let signal = Signal::new(value);
create_ref(cx, signal)
}
@ -469,7 +469,7 @@ pub fn create_signal<T>(cx: Scope, value: T) -> &Signal<T> {
/// Create a new [`Signal`] under the current [`Scope`] but with an initial value wrapped in a
/// [`Rc`]. This is useful to avoid having to clone a value that is already wrapped in a [`Rc`] when
/// creating a new signal. Otherwise, this is identical to [`create_signal`].
pub fn create_signal_from_rc<T>(cx: Scope, value: Rc<T>) -> &Signal<T> {
pub fn create_signal_from_rc<T: 'static>(cx: Scope, value: Rc<T>) -> &Signal<T> {
let signal = Signal(ReadSignal {
value: RefCell::new(value),
emitter: Default::default(),

4
packages/sycamore-router/src/router.rs
View File

@ -211,7 +211,7 @@ where
#[component]
pub fn Router<'a, G: Html, R, F, I>(cx: Scope<'a>, props: RouterProps<'a, R, F, I, G>) -> View<G>
where
R: Route + 'a,
R: Route + 'static,
F: FnOnce(Scope<'a>, &'a ReadSignal<R>) -> View<G> + 'a,
I: Integration + 'static,
{
@ -235,7 +235,7 @@ pub fn RouterBase<'a, G: Html, R, F, I>(
props: RouterBaseProps<'a, R, F, I, G>,
) -> View<G>
where
R: Route + 'a,
R: Route + 'static,
F: FnOnce(Scope<'a>, &'a ReadSignal<R>) -> View<G> + 'a,
I: Integration + 'static,
{

2
packages/sycamore/src/builder.rs
View File

@ -397,7 +397,7 @@ impl<'a, G: GenericNode, F: FnOnce(Scope<'a>) -> G + 'a> ElementBuilder<'a, G, F
/// .dyn_t(|| name.get().to_string())
/// # .view(cx) }
/// ```
pub fn dyn_t<S: AsRef<str> + 'a>(
pub fn dyn_t<S: AsRef<str> + 'static>(
self,
f: impl FnMut() -> S + 'a,
) -> ElementBuilder<'a, G, impl FnOnce(Scope<'a>) -> G + 'a> {

40
packages/sycamore/src/motion.rs
View File

@ -1,12 +1,12 @@
//! Utilities for smooth transitions and animations.
use std::cell::{Cell, RefCell};
use std::cell::RefCell;
use std::rc::Rc;
use crate::reactive::*;
/// Type returned by [`create_raf`] and [`create_raf_loop`].
type RafState<'a> = (RcSignal<bool>, &'a dyn Fn(), &'a dyn Fn());
type RafState<'a> = (RcSignal<bool>, Rc<dyn Fn() + 'a>, Rc<dyn Fn() + 'a>);
/// Schedule a callback to be called on each animation frame.
/// Does nothing if not on `wasm32` target.
@ -18,8 +18,8 @@ type RafState<'a> = (RcSignal<bool>, &'a dyn Fn(), &'a dyn Fn());
/// The raf is not started by default. Call the `start` function to initiate the raf.
pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> {
let running = create_ref(cx, create_rc_signal(false));
let start: &dyn Fn();
let stop: &dyn Fn();
let start: Rc<dyn Fn()>;
let stop: Rc<dyn Fn()>;
// Only run on wasm32 architecture.
#[cfg(all(target_arch = "wasm32", feature = "web"))]
@ -53,7 +53,7 @@ pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> {
}
}
})));
start = create_ref(cx, move || {
start = Rc::new(move || {
if !*running.get() {
running.set(true);
web_sys::window()
@ -64,12 +64,12 @@ pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> {
.unwrap_throw();
}
});
stop = create_ref(cx, || running.set(false));
stop = Rc::new(|| running.set(false));
}
#[cfg(not(all(target_arch = "wasm32", feature = "web")))]
{
start = create_ref(cx, || running.set(true));
stop = create_ref(cx, || running.set(false));
start = Rc::new(|| running.set(true));
stop = Rc::new(|| running.set(false));
}
(running.clone(), start, stop)
@ -84,13 +84,16 @@ pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> {
///
/// The raf is not started by default. Call the `start` function to initiate the raf.
pub fn create_raf_loop<'a>(cx: Scope<'a>, mut f: impl FnMut() -> bool + 'a) -> RafState<'a> {
let stop_shared = create_ref(cx, Cell::new(None::<&dyn Fn()>));
let (running, start, stop) = create_raf(cx, move || {
if !f() {
stop_shared.get().unwrap()();
let stop_shared = Rc::new(RefCell::new(None::<Rc<dyn Fn() + 'a>>));
let (running, start, stop) = create_raf(cx, {
let stop_shared = Rc::clone(&stop_shared);
move || {
if !f() {
stop_shared.borrow().as_ref().unwrap()();
}
}
});
stop_shared.set(Some(stop));
*stop_shared.borrow_mut() = Some(Rc::clone(&stop));
(running, start, stop)
}
@ -101,10 +104,13 @@ pub fn create_tweened_signal<'a, T: Lerp + Clone + 'a>(
transition_duration: std::time::Duration,
easing_fn: impl Fn(f32) -> f32 + 'static,
) -> &'a Tweened<'a, T> {
create_ref(
cx,
Tweened::new(cx, initial, transition_duration, easing_fn),
)
// SAFETY: We do not access any referenced data in the Drop implementation of Tweened.
unsafe {
create_ref_unsafe(
cx,
Tweened::new(cx, initial, transition_duration, easing_fn),
)
}
}
/// Describes a trait that can be linearly interpolate between two points.

3
packages/sycamore/src/suspense.rs
View File

@ -157,7 +157,8 @@ impl<'a> TransitionHandle<'a> {
pub fn use_transition(cx: Scope<'_>) -> &TransitionHandle<'_> {
let is_pending = create_signal(cx, false);
create_ref(cx, TransitionHandle { cx, is_pending })
// SAFETY: We do not access any referenced data in the Drop implementation for TransitionHandle.
unsafe { create_ref_unsafe(cx, TransitionHandle { cx, is_pending }) }
}
#[cfg(all(test, feature = "ssr", not(miri)))]

8
packages/sycamore/tests/web/indexed.rs
View File

@ -270,7 +270,7 @@ fn nested_reactivity() {
cx,
vec![1, 2, 3]
.into_iter()
.map(|x| create_signal(cx, x))
.map(|x| create_rc_signal(x))
.collect(),
);
@ -293,11 +293,7 @@ fn nested_reactivity() {
count.get()[0].set(4);
assert_text_content!(p, "423");
count.set({
let mut tmp = (*count.get()).clone();
tmp.push(create_signal(cx, 5));
tmp
});
count.modify().push(create_rc_signal(5));
assert_text_content!(p, "4235");
});
}

8
packages/sycamore/tests/web/keyed.rs
View File

@ -279,7 +279,7 @@ fn nested_reactivity() {
cx,
vec![1, 2, 3]
.into_iter()
.map(|x| create_signal(cx, x))
.map(|x| create_rc_signal(x))
.collect(),
);
@ -303,11 +303,7 @@ fn nested_reactivity() {
count.get()[0].set(4);
assert_text_content!(p, "423");
count.set({
let mut tmp = (*count.get()).clone();
tmp.push(create_signal(cx, 5));
tmp
});
count.modify().push(create_rc_signal(5));
assert_text_content!(p, "4235");
});
}