Skip to content

Commit 289b595

Browse files
authored
properties: a bit more type safety in the BindingCallable trait (#9997)
1 parent 156930f commit 289b595

File tree

3 files changed

+72
-68
lines changed

3 files changed

+72
-68
lines changed

internal/core/properties.rs

Lines changed: 48 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -298,10 +298,10 @@ struct BindingVTable {
298298
/// # Safety
299299
///
300300
/// IS_TWO_WAY_BINDING cannot be true if Self is not a TwoWayBinding
301-
unsafe trait BindingCallable {
301+
unsafe trait BindingCallable<T> {
302302
/// This function is called by the property to evaluate the binding and produce a new value. The
303303
/// previous property value is provided in the value parameter.
304-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult;
304+
fn evaluate(self: Pin<&Self>, value: &mut T) -> BindingResult;
305305

306306
/// This function is used to notify the binding that one of the dependencies was changed
307307
/// and therefore this binding may evaluate to a different value, too.
@@ -312,7 +312,7 @@ unsafe trait BindingCallable {
312312
/// the property will get the new value.
313313
/// When returning true, the call was intercepted and the binding will not be removed,
314314
/// but the property will still have that value
315-
unsafe fn intercept_set(self: Pin<&Self>, _value: *const ()) -> bool {
315+
fn intercept_set(self: Pin<&Self>, _value: &T) -> bool {
316316
false
317317
}
318318

@@ -327,8 +327,8 @@ unsafe trait BindingCallable {
327327
const IS_TWO_WAY_BINDING: bool = false;
328328
}
329329

330-
unsafe impl<F: Fn(*mut ()) -> BindingResult> BindingCallable for F {
331-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult {
330+
unsafe impl<T, F: Fn(&mut T) -> BindingResult> BindingCallable<T> for F {
331+
fn evaluate(self: Pin<&Self>, value: &mut T) -> BindingResult {
332332
self(value)
333333
}
334334
}
@@ -420,59 +420,61 @@ impl BindingHolder {
420420
}
421421
}
422422

423-
fn alloc_binding_holder<B: BindingCallable + 'static>(binding: B) -> *mut BindingHolder {
423+
fn alloc_binding_holder<T, B: BindingCallable<T> + 'static>(binding: B) -> *mut BindingHolder {
424424
/// Safety: _self must be a pointer that comes from a `Box<BindingHolder<B>>::into_raw()`
425425
unsafe fn binding_drop<B>(_self: *mut BindingHolder) {
426426
drop(Box::from_raw(_self as *mut BindingHolder<B>));
427427
}
428428

429429
/// Safety: _self must be a pointer to a `BindingHolder<B>`
430430
/// and value must be a pointer to T
431-
unsafe fn evaluate<B: BindingCallable>(
431+
unsafe fn evaluate<T, B: BindingCallable<T>>(
432432
_self: *const BindingHolder,
433433
value: *mut (),
434434
) -> BindingResult {
435-
Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding)).evaluate(value)
435+
Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding))
436+
.evaluate(&mut *(value as *mut T))
436437
}
437438

438439
/// Safety: _self must be a pointer to a `BindingHolder<B>`
439-
unsafe fn mark_dirty<B: BindingCallable>(_self: *const BindingHolder, _: bool) {
440+
unsafe fn mark_dirty<T, B: BindingCallable<T>>(_self: *const BindingHolder, _: bool) {
440441
Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding)).mark_dirty()
441442
}
442443

443444
/// Safety: _self must be a pointer to a `BindingHolder<B>`
444-
unsafe fn intercept_set<B: BindingCallable>(
445+
unsafe fn intercept_set<T, B: BindingCallable<T>>(
445446
_self: *const BindingHolder,
446447
value: *const (),
447448
) -> bool {
448-
Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding)).intercept_set(value)
449+
Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding))
450+
.intercept_set(&*(value as *const T))
449451
}
450452

451-
unsafe fn intercept_set_binding<B: BindingCallable>(
453+
unsafe fn intercept_set_binding<T, B: BindingCallable<T>>(
452454
_self: *const BindingHolder,
453455
new_binding: *mut BindingHolder,
454456
) -> bool {
455457
Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding))
456458
.intercept_set_binding(new_binding)
457459
}
458460

459-
trait HasBindingVTable {
461+
trait HasBindingVTable<T> {
460462
const VT: &'static BindingVTable;
461463
}
462-
impl<B: BindingCallable> HasBindingVTable for B {
464+
impl<T, B: BindingCallable<T>> HasBindingVTable<T> for B {
463465
const VT: &'static BindingVTable = &BindingVTable {
464466
drop: binding_drop::<B>,
465-
evaluate: evaluate::<B>,
466-
mark_dirty: mark_dirty::<B>,
467-
intercept_set: intercept_set::<B>,
468-
intercept_set_binding: intercept_set_binding::<B>,
467+
evaluate: evaluate::<T, B>,
468+
mark_dirty: mark_dirty::<T, B>,
469+
intercept_set: intercept_set::<T, B>,
470+
intercept_set_binding: intercept_set_binding::<T, B>,
469471
};
470472
}
471473

472474
let holder: BindingHolder<B> = BindingHolder {
473475
dependencies: Cell::new(0),
474476
dep_nodes: Default::default(),
475-
vtable: <B as HasBindingVTable>::VT,
477+
vtable: <B as HasBindingVTable<T>>::VT,
476478
dirty: Cell::new(true), // starts dirty so it evaluates the property when used
477479
is_two_way_binding: B::IS_TWO_WAY_BINDING,
478480
pinned: PhantomPinned,
@@ -571,12 +573,12 @@ impl PropertyHandle {
571573
}
572574

573575
/// Safety: the BindingCallable must be valid for the type of this property
574-
unsafe fn set_binding<B: BindingCallable + 'static>(
576+
unsafe fn set_binding<T, B: BindingCallable<T> + 'static>(
575577
&self,
576578
binding: B,
577579
#[cfg(slint_debug_property)] debug_name: &str,
578580
) {
579-
let binding = alloc_binding_holder::<B>(binding);
581+
let binding = alloc_binding_holder::<T, B>(binding);
580582
#[cfg(slint_debug_property)]
581583
{
582584
(*binding).debug_name = debug_name.into();
@@ -964,8 +966,7 @@ impl<T: Clone> Property<T> {
964966
// Safety: This will make a binding callable for the type T
965967
unsafe {
966968
self.handle.set_binding(
967-
move |val: *mut ()| {
968-
let val = &mut *(val as *mut T);
969+
move |val: &mut T| {
969970
*val = binding.evaluate(val);
970971
BindingResult::KeepBinding
971972
},
@@ -1045,14 +1046,14 @@ fn properties_simple_test() {
10451046
struct TwoWayBinding<T> {
10461047
common_property: Pin<Rc<Property<T>>>,
10471048
}
1048-
unsafe impl<T: PartialEq + Clone + 'static> BindingCallable for TwoWayBinding<T> {
1049-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult {
1050-
*(value as *mut T) = self.common_property.as_ref().get();
1049+
unsafe impl<T: PartialEq + Clone + 'static> BindingCallable<T> for TwoWayBinding<T> {
1050+
fn evaluate(self: Pin<&Self>, value: &mut T) -> BindingResult {
1051+
*value = self.common_property.as_ref().get();
10511052
BindingResult::KeepBinding
10521053
}
10531054

1054-
unsafe fn intercept_set(self: Pin<&Self>, value: *const ()) -> bool {
1055-
self.common_property.as_ref().set((*(value as *const T)).clone());
1055+
fn intercept_set(self: Pin<&Self>, value: &T) -> bool {
1056+
self.common_property.as_ref().set(value.clone());
10561057
true
10571058
}
10581059

@@ -1212,16 +1213,16 @@ impl<T: PartialEq + Clone + 'static> Property<T> {
12121213
T2: PartialEq + Clone + 'static,
12131214
M1: Fn(&T) -> T2 + Clone + 'static,
12141215
M2: Fn(&mut T, &T2) + Clone + 'static,
1215-
> BindingCallable for TwoWayBindingWithMap<T, T2, M1, M2>
1216+
> BindingCallable<T2> for TwoWayBindingWithMap<T, T2, M1, M2>
12161217
{
1217-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult {
1218-
*(value as *mut T2) = (self.map_to)(&self.common_property.as_ref().get());
1218+
fn evaluate(self: Pin<&Self>, value: &mut T2) -> BindingResult {
1219+
*value = (self.map_to)(&self.common_property.as_ref().get());
12191220
BindingResult::KeepBinding
12201221
}
12211222

1222-
unsafe fn intercept_set(self: Pin<&Self>, value: *const ()) -> bool {
1223+
fn intercept_set(self: Pin<&Self>, value: &T2) -> bool {
12231224
let mut old = self.common_property.as_ref().get();
1224-
(self.map_from)(&mut old, &*(value as *const T2));
1225+
(self.map_from)(&mut old, value);
12251226
self.common_property.as_ref().set(old);
12261227
true
12271228
}
@@ -1243,7 +1244,7 @@ impl<T: PartialEq + Clone + 'static> Property<T> {
12431244

12441245
/// Given a binding for T2, maps to a binding for T
12451246
struct BindingMapper<T, T2, M1, M2> {
1246-
/// Binding that returns a `T`
1247+
/// Binding that returns a `T2`
12471248
b: *mut BindingHolder,
12481249
map_to: M1,
12491250
map_from: M2,
@@ -1254,20 +1255,24 @@ impl<T: PartialEq + Clone + 'static> Property<T> {
12541255
T2: PartialEq + Clone + 'static,
12551256
M1: Fn(&T) -> T2 + 'static,
12561257
M2: Fn(&mut T, &T2) + 'static,
1257-
> BindingCallable for BindingMapper<T, T2, M1, M2>
1258+
> BindingCallable<T> for BindingMapper<T, T2, M1, M2>
12581259
{
1259-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult {
1260-
let value = &mut *(value as *mut T);
1260+
fn evaluate(self: Pin<&Self>, value: &mut T) -> BindingResult {
12611261
let mut sub_value = (self.map_to)(value);
1262-
((*self.b).vtable.evaluate)(self.b, &mut sub_value as *mut T2 as *mut ());
1262+
// Safety: `self.b` is a BindingHolder that expects a `T2`
1263+
unsafe {
1264+
((*self.b).vtable.evaluate)(self.b, &mut sub_value as *mut T2 as *mut ());
1265+
}
12631266
(self.map_from)(value, &sub_value);
12641267
BindingResult::KeepBinding
12651268
}
12661269

1267-
unsafe fn intercept_set(self: Pin<&Self>, value: *const ()) -> bool {
1268-
let value = &mut *(value as *mut T);
1270+
fn intercept_set(self: Pin<&Self>, value: &T) -> bool {
12691271
let sub_value = (self.map_to)(value);
1270-
((*self.b).vtable.intercept_set)(self.b, &sub_value as *const T2 as *const ())
1272+
// Safety: `self.b` is a BindingHolder that expects a `T2`
1273+
unsafe {
1274+
((*self.b).vtable.intercept_set)(self.b, &sub_value as *const T2 as *const ())
1275+
}
12711276
}
12721277
}
12731278
impl<T, T2, M1, M2> Drop for BindingMapper<T, T2, M1, M2> {
@@ -1582,10 +1587,8 @@ struct StateInfoBinding<F> {
15821587
binding: F,
15831588
}
15841589

1585-
unsafe impl<F: Fn() -> i32> crate::properties::BindingCallable for StateInfoBinding<F> {
1586-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult {
1587-
// Safety: We should only set this binding on a property of type StateInfo
1588-
let value = &mut *(value as *mut StateInfo);
1590+
unsafe impl<F: Fn() -> i32> crate::properties::BindingCallable<StateInfo> for StateInfoBinding<F> {
1591+
fn evaluate(self: Pin<&Self>, value: &mut StateInfo) -> BindingResult {
15891592
let new_state = (self.binding)();
15901593
let timestamp = self.dirty_time.take();
15911594
if new_state != value.current_state {

internal/core/properties/ffi.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ fn make_c_function_binding(
5353
intercept_set_binding: Option<
5454
extern "C" fn(user_data: *mut c_void, new_binding: *mut c_void) -> bool,
5555
>,
56-
) -> impl BindingCallable {
56+
) -> impl BindingCallable<c_void> {
5757
struct CFunctionBinding<T> {
5858
binding_function: extern "C" fn(*mut c_void, *mut T),
5959
user_data: *mut c_void,
6060
drop_user_data: Option<extern "C" fn(*mut c_void)>,
6161
intercept_set:
62-
Option<extern "C" fn(user_data: *mut c_void, pointer_to_value: *const c_void) -> bool>,
62+
Option<extern "C" fn(user_data: *mut c_void, pointer_to_value: *const T) -> bool>,
6363
intercept_set_binding:
6464
Option<extern "C" fn(user_data: *mut c_void, new_binding: *mut c_void) -> bool>,
6565
}
@@ -72,15 +72,15 @@ fn make_c_function_binding(
7272
}
7373
}
7474

75-
unsafe impl<T> BindingCallable for CFunctionBinding<T> {
76-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult {
75+
unsafe impl<T> BindingCallable<T> for CFunctionBinding<T> {
76+
fn evaluate(self: Pin<&Self>, value: &mut T) -> BindingResult {
7777
(self.binding_function)(self.user_data, value as *mut T);
7878
BindingResult::KeepBinding
7979
}
80-
unsafe fn intercept_set(self: Pin<&Self>, value: *const ()) -> bool {
80+
fn intercept_set(self: Pin<&Self>, value: &T) -> bool {
8181
match self.intercept_set {
8282
None => false,
83-
Some(intercept_set) => intercept_set(self.user_data, value),
83+
Some(intercept_set) => intercept_set(self.user_data, value as *const T),
8484
}
8585
}
8686
unsafe fn intercept_set_binding(self: Pin<&Self>, new_binding: *mut BindingHolder) -> bool {
@@ -192,9 +192,9 @@ fn c_set_animated_value<T: InterpolatedPropertyValue + Clone>(
192192
));
193193
// Safety: The BindingCallable is for type T
194194
unsafe {
195-
handle.0.set_binding(move |val: *mut ()| {
195+
handle.0.set_binding(move |val: &mut T| {
196196
let (value, finished) = d.borrow_mut().compute_interpolated_value();
197-
*(val as *mut T) = value;
197+
*val = value;
198198
if finished {
199199
BindingResult::RemoveBinding
200200
} else {

internal/core/properties/properties_animations.rs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,9 @@ pub(super) enum AnimatedBindingState {
128128
ShouldStart,
129129
}
130130

131+
#[pin_project::pin_project]
131132
pub(super) struct AnimatedBindingCallable<T, A> {
133+
#[pin]
132134
pub(super) original_binding: PropertyHandle,
133135
pub(super) state: Cell<AnimatedBindingState>,
134136
pub(super) animation_data: RefCell<PropertyValueAnimationData<T>>,
@@ -137,19 +139,19 @@ pub(super) struct AnimatedBindingCallable<T, A> {
137139

138140
pub(super) type AnimationDetail = Option<(PropertyAnimation, crate::animations::Instant)>;
139141

140-
unsafe impl<T: InterpolatedPropertyValue + Clone, A: Fn() -> AnimationDetail> BindingCallable
142+
unsafe impl<T: InterpolatedPropertyValue + Clone, A: Fn() -> AnimationDetail> BindingCallable<T>
141143
for AnimatedBindingCallable<T, A>
142144
{
143-
unsafe fn evaluate(self: Pin<&Self>, value: *mut ()) -> BindingResult {
144-
let original_binding = Pin::new_unchecked(&self.original_binding);
145+
fn evaluate(self: Pin<&Self>, value: &mut T) -> BindingResult {
146+
let original_binding = self.project_ref().original_binding;
145147
original_binding.register_as_dependency_to_current_binding(
146148
#[cfg(slint_debug_property)]
147149
"<AnimatedBindingCallable>",
148150
);
149151
match self.state.get() {
150152
AnimatedBindingState::Animating => {
151153
let (val, finished) = self.animation_data.borrow_mut().compute_interpolated_value();
152-
*(value as *mut T) = val;
154+
*value = val;
153155
if finished {
154156
self.state.set(AnimatedBindingState::NotAnimating)
155157
} else {
@@ -158,15 +160,16 @@ unsafe impl<T: InterpolatedPropertyValue + Clone, A: Fn() -> AnimationDetail> Bi
158160
}
159161
}
160162
AnimatedBindingState::NotAnimating => {
161-
self.original_binding.update(value);
163+
// Safety: `value` is a valid mutable reference
164+
unsafe { self.original_binding.update(value as *mut T) };
162165
}
163166
AnimatedBindingState::ShouldStart => {
164-
let value = &mut *(value as *mut T);
165167
self.state.set(AnimatedBindingState::Animating);
166168
let mut animation_data = self.animation_data.borrow_mut();
167169
// animation_data.details.iteration_count = 1.;
168170
animation_data.from_value = value.clone();
169-
self.original_binding.update((&mut animation_data.to_value) as *mut T as *mut ());
171+
// Safety: `animation_data.to_value` is a valid mutable reference
172+
unsafe { self.original_binding.update((&mut animation_data.to_value) as *mut T) };
170173
if let Some((details, start_time)) = (self.compute_animation_details)() {
171174
animation_data.start_time = start_time;
172175
animation_data.details = details;
@@ -254,9 +257,9 @@ impl<T: Clone + InterpolatedPropertyValue + 'static> Property<T> {
254257
// Safety: the BindingCallable will cast its argument to T
255258
unsafe {
256259
self.handle.set_binding(
257-
move |val: *mut ()| {
260+
move |val: &mut T| {
258261
let (value, finished) = d.borrow_mut().compute_interpolated_value();
259-
*(val as *mut T) = value;
262+
*val = value;
260263
if finished {
261264
BindingResult::RemoveBinding
262265
} else {
@@ -285,9 +288,8 @@ impl<T: Clone + InterpolatedPropertyValue + 'static> Property<T> {
285288
let binding_callable = properties_animations::AnimatedBindingCallable::<T, _> {
286289
original_binding: PropertyHandle {
287290
handle: Cell::new(
288-
(alloc_binding_holder(move |val: *mut ()| unsafe {
289-
let val = &mut *(val as *mut T);
290-
*(val as *mut T) = binding.evaluate(val);
291+
(alloc_binding_holder(move |val: &mut T| {
292+
*val = binding.evaluate(val);
291293
BindingResult::KeepBinding
292294
}) as usize)
293295
| 0b10,
@@ -327,9 +329,8 @@ impl<T: Clone + InterpolatedPropertyValue + 'static> Property<T> {
327329
let binding_callable = properties_animations::AnimatedBindingCallable::<T, _> {
328330
original_binding: PropertyHandle {
329331
handle: Cell::new(
330-
(alloc_binding_holder(move |val: *mut ()| unsafe {
331-
let val = &mut *(val as *mut T);
332-
*(val as *mut T) = binding.evaluate(val);
332+
(alloc_binding_holder(move |val: &mut T| {
333+
*val = binding.evaluate(val);
333334
BindingResult::KeepBinding
334335
}) as usize)
335336
| 0b10,

0 commit comments

Comments
 (0)