Skip to content

Conversation

carlosmn
Copy link
Contributor

@carlosmn carlosmn commented Jul 5, 2025

When using Rust we prefer to use a Value for everything but gio also lets you return explicitly a gboolean or gssize which avoid boxing.

Usage of these functions to return these kinds of values requires a call to the corresponding propagate function and you cannot mix them. Trying to extract a Value when either of them were used leads to an incorrect value.

Expose methods in Task and LocalTask to return and propagate these kinds instead of assuming everything can go through a Value.

Given that you must match the result setter with its setter or you might get a nonsense result, and worse, without the v2_64 feature flag, you can segfault or read the wrong memory, I've also marked the getters as unsafe. I wasn't sure about marking the setters as unsafe, as they themselves aren't doing anything bad (though for one of them they'll be unsafe anyway because of cloning).

This is meant to address #1730

…m a `Task`

When using Rust we prefer to use a `Value` for everything but gio also lets you
return explicitly a `gboolean` or `gssize` which avoid boxing.

Usage of these functions to return these kinds of values requires a call to the
corresponding propagate function and you cannot mix them. Trying to extract a
`Value` when either of them were used leads to an incorrect value.

Expose methods in `Task` and `LocalTask` to return and propagate these kinds
instead of assuming everything can go through a `Value`.
@carlosmn carlosmn force-pushed the cmn/task-propagate-explicit branch 2 times, most recently from 7104471 to d18087e Compare July 5, 2025 17:39
The three ways you can return a result from the task must be match in how you
set and get the fields.

Document this in the different functions and mark them as unsafe unconditionally
to indicate that there is no way within the type system to ensure that you are
using it safely.
@sdroege
Copy link
Member

sdroege commented Jul 7, 2025

This change makes sense to me. It's unfortunate that the API has to become unsafe, but that seems to be not fixable without changes in GLib. The API is simply not type-safe.

@sdroege sdroege requested a review from bilelmoussaoui July 7, 2025 11:55
@sdroege
Copy link
Member

sdroege commented Jul 7, 2025

@bilelmoussaoui @pbor ?

@sdroege
Copy link
Member

sdroege commented Jul 13, 2025

We should probably get this in before the release on Tuesday

@bilelmoussaoui
Copy link
Member

Not sure about the suggested names but i also don't have any better alternatives. Otherwise lgtm

@sdroege sdroege merged commit 0b67019 into gtk-rs:main Jul 14, 2025
47 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants