gtk-rs / gir

Tool to generate rust bindings and user API for glib-based libraries
https://gtk-rs.org/gir/book/
MIT License
230 stars 102 forks source link

Make use of "not nullable" annotation for return types #655

Open sdroege opened 5 years ago

sdroege commented 5 years ago

Since gobject-introspection 1.48 there's "not nullable". This would allow us the directly generate a non-Option return type for such functions instead of requiring a user override and assuming that the "nullable" annotation was just missing.

EPashkin commented 5 years ago

@sdroege Thanks for information

EPashkin commented 5 years ago

Problem that "(not nullable)" annotation in code still saves as the absence of "nullable" attribute in .gir: (no nullable="0" in gtk-rs or gstreamer also no not_nullable or something like this), Main your problem with nullable is that it not set on places where it must.

Main place when we don't agree with nullable:

  1. https://github.com/gtk-rs/gir/blob/master/src/analysis/return_value.rs#L36-L38 If I remove these functions will be not nullable that wrong gdk_pixbuf_scale_simple, gtk_about_dialog_get_logo

  2. https://github.com/gtk-rs/gir/blob/master/src/analysis/out_parameters.rs#L99-L102 It don't work until first fixed and without it produce many Result<Option etc. Ex. for gtk_cell_area_get_cell_at_position, gtk_icon_info_load_surface

Last override seems reasonable for me https://github.com/gtk-rs/gir/blob/master/src/analysis/special_functions.rs#L46-L54

sdroege commented 5 years ago

Ah I was hoping that it ends up as a nullable="0" in the XML so that we can distinguish the case of no attribute being given (and we take the safe default) and this case. So it's completely useless for us in the end.

sdroege commented 5 years ago

@EPashkin did you confirm that this annotation does not add a nullable="0" to the XML or similar? If so, I'd like to report this to gobject-introspection so it can be fixed.

We're not using that annotation anywhere in GStreamer yet.

sdroege commented 5 years ago

did you confirm that this annotation does not add a nullable="0"

I can confirm

sdroege commented 5 years ago

See https://gitlab.gnome.org/GNOME/gobject-introspection/issues/259

EPashkin commented 5 years ago

@sdroege Thanks for confirming this and adding issue. I seen if return_.nullable and not return_.not_nullable: but was to lazy to try really run introspection.

EPashkin commented 5 years ago

There also third case https://github.com/gtk-rs/gir/blob/master/src/analysis/return_value.rs#L100 seems it can changed

+++ b/src/analysis/return_value.rs
@@ -97,7 +97,7 @@ pub fn analyze(
             }
             parameter = Some(library::Parameter {
                 typ: type_tid,
-                nullable: nullable_override.unwrap_or(Nullable(false)),
+                nullable: nullable_override.unwrap_or(func.ret.nullable),
EPashkin commented 5 years ago

After checking on current gir files I see only 2 changes: https://developer.gnome.org/gdk3/stable/gdk3-Cursors.html#gdk-cursor-new-from-name https://developer.gnome.org/glib/stable/glib-GDateTime.html#g-date-time-new-from-iso8601 both is good

GuillaumeGomez commented 5 years ago

Can we close this issue?

sdroege commented 5 years ago

Did you implement support for it? If not, then no :) We make use of nullable, but not of not nullable