Don't skip Option for nullable collections

[fengalin]

Nullable collection were represented as a Vec<_> or [_], but an empty collection is not the same as an undefined collection. Rust code can express this using an Option<_>.

Don't do this for return and output values though: in practice the C implementations return NULL when the size is 0. So it doesn't make much sense using an Option<_> in that case.

Finally, Gir now parses the zero-terminated annotation which allows detecting functions returning an array for which the length is not defined by a length argument. Those functions used to require an explicit manual declaration in the Gir.toml and are automatically commented out now.

Sample regen results (no changes for GStreamer):

• gtk-rs-core: fengalin/gtk-rs-core@4f0786b
• gtk4-rs: fengalin/gtk4-rs@4bf3298

Fixes: #1133
See also: gtk-rs/gtk-rs-core#1257

[sdroege]

[ ] Add support for glib::value Option<Vec<_>> conversions.

I think this also involves untangling our existing implementations a bit as they currently map NULL to empty arrays, and possibly also the other way around?

[sdroege]

This looks too easy, I didn't expect that :)

[sdroege]

There's also #1551 doing more or less the same (but a bit more?). I think the main problem here now is fixing glib/translate.rs

[sdroege]

@fengalin What's the plan to continue here, especially with the translation traits?

[fengalin]

My plan was to start over from the PR you linked and implement the changes in the glib bindings. But I got involved with other stuffs. I'll try to resume this later this week.

[fengalin]

Status on this:

• Rebased
• Started over from the commit from codegen: Fix nullable array in function params #1551
• Added basic handling of the zero-terminated annotation. Note: I also considered handling optional & allow-none annotations, but they seem to actually indicate whether the API allows the caller to pass NULL for an in-out argument.

Quoting the last commit:

Nullable collection were represented as a Vec<_> or [_], but an empty
collection is not the same as an undefined collection.

Don't do this for return and output values though: in practice the C
implementations return NULL when the size is 0. So it doesn't make much
sense using an Option<_> in that case.

Finally, Gir now parses the zero-terminated annotation which allows
detecting functions returning an array for which the length is not defined
by a length argument. Those functions used to require an explicit manual
declaration in the Gir.toml and are automatically commented out now.

Sample regen results (no update for GStreamer):

• gtk-rs-core: fengalin/gtk-rs-core@4f0786b
• gtk4-rs: fengalin/gtk4-rs@4bf3298

The outcome is that content_type_guess() is generated as expected by #1133:

pub fn content_type_guess(
    filename: Option<impl AsRef<std::path::Path>>,
    data: Option<&[u8]>,
) -> (glib::GString, bool) { .. }

data is optional, which could makes sense. Do we want this as an API instead of passing an empty array? I don't really know.

Because similar annotations don't necessarily mean that the argument can be an Option, I'm unsure we want this applied systematically anyway. Take gsk4::GskStroke::set_dash for instance:

pub fn set_dash(&self, dash: Option<&[f32]>) { .. }

We could think passing None would discard a previous dash for current GskStroke. But the documentation doesn't mention this possibility.

Another example is gdk4::ContentFormats::new().

pub fn new(mime_types: Option<&[&str]>) -> ContentFormats { .. }

The doc doesn't say anything about a difference between an empty array and an undefined argument.

See the gtk4-rs regen commit for more examples.

Another approach could be to not generate the Option for these use cases and let the bindings override this (using optional = true or nullable = true) in their Gir.toml. @sdroege @bilelmoussaoui what do you think?

[bilelmoussaoui]

The naming here confused me a bit, could use a small comment on what it is.

[fengalin]

Updated

[fengalin]

@bilelmoussaoui: when you get a chance, I'd be interested by your opinion on the question in this comment #1622 (comment)

[bilelmoussaoui]

Another approach could be to not generate the Option for these use cases and let the bindings override this (using optional = true or nullable = true) in their Gir.toml. @sdroege @bilelmoussaoui what do you think?

I would generate the Option and let the user override if they want to disable that. As we to tend to trust the nullability annotations

[bilelmoussaoui]

can you open PRs for the changes for gtk4-rs/gtk-rs-core? i will review and merge this one afterwards.

[fengalin]

Can you create an issue for that against gobject-introspection?

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

Can you create an MR for that?

https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4789