slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
17.16k stars 582 forks source link

Models: Getting rid of function type parameter #5878

Open Enyium opened 1 month ago

Enyium commented 1 month ago

MapModel, FilterModel and SortModel all have a type parameter F that's a function. When you hold one of these types in a struct, you have to explicitly specify the type, and I find it to be quite an imposition to have such a (from a code writing viewpoint) peripheral type as part of the field type.

I currently have a struct similar to this in my app:

struct MyStruct {
    some_names:
        Rc<MapModel<VecModel<(bool, String)>, fn((bool, String)) -> StandardListViewItem>>,
    other_names: Rc<MapModel<VecModel<(bool, String)>, fn((bool, String)) -> StandardListViewItem>>,
}

I think with one of the Fn... traits, it gets even worse.

Without type parameter F, it would be less confusing:

struct MyStruct {
    some_names: Rc<MapModel<VecModel<(bool, String)>>>,
    other_names: Rc<MapModel<VecModel<(bool, String)>>>,
}

I don't think these model types need to support capturing closures. As far as I can see, it should just be "input -> output". With function pointers (fn(T) -> U), the type parameter could be removed and the user could still use a closure - just not a capturing, but only a static one (it's like a constant or a literal, but for code).

I did it like this with ResGuard in a crate of mine (source).

EDIT: once_cell::sync::Lazy and std::cell::LazyCell do it similar, not entirely getting rid of F, but providing a function pointer default F = fn() -> T.

ogoffart commented 1 month ago

I don't think these model types need to support capturing closures. As far as I can see, it should just be "input -> output".

Sometimes you need to have some capture, eg, you are mapping from id to the content of a hashmap or something.

once_cell::sync::Lazy and std::cell::LazyCell do it similar, not entirely getting rid of F, but providing a function pointer default F = fn() -> T.

That's a good idea, we could do the same as them

tronical commented 1 month ago

Alternatively, what if we go the trait route? Like this:

trait ModelMapper<T, U> {
    fn map(&self, t: T) -> U;
}

impl<T, U, F: Fn(T) -> U> ModelMapper<T, U> for F {
    fn map(&self, t: T) -> U {
        self(t)
    }
}

Then you can implement trait for your own data structure that includes additional fields. Would that still be source compatible?

Enyium commented 1 month ago

If this is implemented via type parameter defaults, it seems that the current MapModel<M, F> would need to be changed to MapModel<M, U, F>, where the defaults are chosen in a way that the user would have to provide either U, which is the map function's output, or F (and probably write _ as U). Hopefully, this works with the Rust compiler. Then, the type of a struct field may look like this: Rc<MapModel<VecModel<(bool, String)>, StandardListViewItem>>.

crai0 commented 3 weeks ago

I think the default type parameter approach doesn't work out of the box because the default function pointer needs to be fn(T) -> U, doesn't it?

This would defeat the goal of this issue though, since MapModel would then require both U and T as type parameters, forcing the type of a struct field to look like this Rc<MapModel<VecModel<(bool, String)>, (bool, String), StandardListViewItem>> instead of this Rc<MapModel<VecModel<(bool, String)>, fn(bool, String) -> StandardListViewItem>>.

Even if we were to do something like this

pub struct MapModel<M, T, U, F = fn(T) -> U>
where
    M: Model<Data = T>,
{
    wrapped_model: M,
    map_function: F,
    map_function_output: PhantomData<U>,
}

we still couldn't write Rc<MapModel<VecModel<(bool, String)>, _, StandardListViewItem>> because _ cannot be used in types on item signatures.

Maybe I'm missing something here, though.

Enyium commented 3 weeks ago

Can't you use F = fn(<M as Model>::Data) -> U to be spared of type parameter T on the struct itself?

The fact that _ isn't allowed within types on item signatures, though, means that, if someone needs to specify the function type, the whole thing would be longer, and the output type would be specified twice. Perhaps this plan is still a win, because this use case may be rare.

ogoffart commented 3 weeks ago

Right, it seems this can't be done. Given the circumstance, I'm not sure it is an issue worth solving.