Add per-crate and per-object configuration to trust return value nullability #970
Conversation
|
I'm not sure this is a good idea to have such a setting... In any case, please put a multi-line warning (on the console when running gir with this setting) to say that it needs to be use with a lot of caution. |
|
As I said in the gtk issue, I think for libraries like gtk this feature would only be realistic on a per .c-file/class basis. Otherwise we would have to check thousands of functions before being able to use this feature. |
|
On 17 October 2020 15:03:04 EEST, Guillaume Gomez ***@***.***> wrote:
I'm not sure this is a good idea to have such a setting... In any case, please put a multi-line warning (on the console when running gir with this setting) to say that it needs to be use with a lot of caution.
I would like to use it in gstreamer where it's almost correct now and where I can take care of fixing it. I don't want a warning there :)
…--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
On 17 October 2020 15:19:53 EEST, Sophie Herold ***@***.***> wrote:
As I said in the gtk issue, I think for libraries like gtk this feature would only be realistic on a per .c-file/class basis. Otherwise we would have to check thousands of functions before being able to use this feature.
That's doable, yes. I'll also add a per type configuration for this in addition.
…--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
On 17 October 2020 15:03:04 EEST, Guillaume Gomez ***@***.***> wrote:
I'm not sure this is a good idea to have such a setting... In any case, please put a multi-line warning (on the console when running gir with this setting) to say that it needs to be use with a lot of caution.
Also the goal here would be to slowly move all crates to this by fixing upstream annotations so that we don't have to worry about it anymore and simply have to review new APIs like we have to do now.
…--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
@GuillaumeGomez Also following https://gitlab.gnome.org/GNOME/gtk/-/issues/3267#note_937101 this is basically the only way forward here. We can't change gobject-introspection and have to live with how wonderfully broken reality is :) |
sdroege
force-pushed
the
trust_return_value_nullability
branch
from
a0ab623
to
908a457
Compare
…ability This has to be used carefully as many libraries forgot to put `nullability` annotations on return values, which then causes a panic if an unexpected `NULL` is returned.
sdroege
force-pushed
the
trust_return_value_nullability
branch
from
908a457
to
3303549
Compare
|
@sophie-h This can now also be configured per object if you want to give it a try |
sdroege
changed the title
|
@GuillaumeGomez This is also ready for review now |
sdroege
requested review from
GuillaumeGomez and
EPashkin
and removed request for
GuillaumeGomez
|
I'm very happy about this. Set one object to |
Thanks for taking care of that :) I'll go through gio one of these days unless you want to do that. |
No, thanks :) Not sure if I will have a lot of time for that. |
See https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1708 . I'll wait until that is in a GLib release, otherwise this needs too many overrides in |
|
Is there anything to be done here or can this be merged? |
|
It needs @GuillaumeGomez or @EPashkin to review it :) |
@GuillaumeGomez already did that here: #973 Let's get this in then? :) |
|
@GuillaumeGomez Can you take a look at this? The new GLib release fixes all the remaining missing annotations, so we could have a clean version of GLib now with improved API. |
|
After a discussion between @sdroege and I, I finally agreed on his position so let's get this in! |
|
Thanks! |
This has to be used carefully as many libraries forgot to put
nullabilityannotations on return values, which then causes a panic ifan unexpected
NULLis returned.CC @GuillaumeGomez @EPashkin @sophie-h
With this you can check what kind of changes are generated when trusting the gir files, and then report any missing nullable annotations to the C libraries.