Don't skip Option for nullable collections where applicable

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.

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

Author: fengalin

src/analysis/class_builder.rs

@@ -141,6 +141,7 @@ fn analyze_property(
         func_name: String::new(),
         func_name_alias: None,
         nullable,
+        is_array: prop.is_array,
         get_out_ref_mode,
         set_in_ref_mode,
         set_bound: None,

src/analysis/properties.rs

@@ -26,6 +26,7 @@ pub struct Property {
     pub func_name: String,
     pub func_name_alias: Option<String>,
     pub nullable: library::Nullable,
+    pub is_array: bool,
     pub get_out_ref_mode: RefMode,
     pub set_in_ref_mode: RefMode,
     pub bounds: Bounds,
@@ -265,6 +266,7 @@ fn analyze_property(
             func_name: get_func_name,
             func_name_alias: get_prop_name,
             nullable,
+            is_array: prop.is_array,
             get_out_ref_mode,
             set_in_ref_mode,
             set_bound: None,
@@ -318,6 +320,7 @@ fn analyze_property(
             func_name: set_func_name,
             func_name_alias: set_prop_name,
             nullable,
+            is_array: prop.is_array,
             get_out_ref_mode,
             set_in_ref_mode,
             set_bound,

src/analysis/return_value.rs

@@ -120,19 +120,32 @@ pub fn analyze(
 
     let parameter = parameter.as_ref().map(|lib_par| {
         let par = analysis::Parameter::from_return_value(env, lib_par, configured_functions);
-        if let Ok(rust_type) = RustType::builder(env, typ)
-            .direction(par.lib_par.direction)
-            .try_from_glib(&par.try_from_glib)
-            .try_build()
+        if par.lib_par.is_array
+            && !par.lib_par.zero_terminated.unwrap_or(true)
+            && par.lib_par.array_length.is_none()
         {
-            used_types.extend(rust_type.into_used_types());
-        }
+            // The array length for the return value of this function can not be inferred
+            // from any argument. How the length should be inferred is usually specified
+            // in the doc of the function, so there's nothing we can do automatically.
+            // Currently the functions matching this condition are explicitly marked
+            // `manual = true` in the matching Gir.toml.
+            // FIXME: `FixedArray`s could be handled automatically
+            commented = true;
+        } else {
+            if let Ok(rust_type) = RustType::builder(env, typ)
+                .direction(par.lib_par.direction)
+                .try_from_glib(&par.try_from_glib)
+                .try_build()
+            {
+                used_types.extend(rust_type.into_used_types());
+            }
 
-        commented = RustType::builder(env, typ)
-            .direction(func.ret.direction)
-            .try_from_glib(&par.try_from_glib)
-            .try_build_param()
-            .is_err();
+            commented = RustType::builder(env, typ)
+                .direction(func.ret.direction)
+                .try_from_glib(&par.try_from_glib)
+                .try_build_param()
+                .is_err();
+        }
 
         par
     });

src/analysis/rust_type.rs

@@ -251,7 +251,6 @@ impl<'env> RustTypeBuilder<'env> {
         let ok = |s: &str| Ok(RustType::from(s));
         let ok_and_use = |s: &str| Ok(RustType::new_and_use(&s));
         let err = |s: &str| Err(TypeError::Unimplemented(s.into()));
-        let mut skip_option = false;
         let type_ = self.env.library.type_(self.type_id);
         let mut rust_type = match *type_ {
             Basic(fund) => {
@@ -355,7 +354,6 @@ impl<'env> RustTypeBuilder<'env> {
             List(inner_tid) | SList(inner_tid) | CArray(inner_tid) | PtrArray(inner_tid)
                 if ConversionType::of(self.env, inner_tid) == ConversionType::Pointer =>
             {
-                skip_option = true;
                 let inner_ref_mode = match self.env.type_(inner_tid) {
                     Class(..) | Interface(..) => RefMode::None,
                     Record(record) => match RecordType::of(record) {
@@ -409,7 +407,6 @@ impl<'env> RustTypeBuilder<'env> {
                     };
 
                     if let Some(s) = array_type {
-                        skip_option = self.direction == ParameterDirection::Return;
                         if self.ref_mode.is_ref() {
                             Ok(format!("[{s}]").into())
                         } else {
@@ -604,7 +601,7 @@ impl<'env> RustTypeBuilder<'env> {
             }
         }
 
-        if *self.nullable && !skip_option {
+        if *self.nullable {
             match ConversionType::of(self.env, self.type_id) {
                 ConversionType::Pointer | ConversionType::Scalar => {
                     rust_type = rust_type.map_any(|rust_type| {

src/chunk/conversion_from_glib.rs

@@ -1,14 +1,15 @@
 use super::parameter_ffi_call_out;
 use crate::{
     analysis::{self, try_from_glib::TryFromGlib},
-    library,
+    library::{self, Nullable},
 };
 
 #[derive(Clone, Debug)]
 pub struct Mode {
     pub typ: library::TypeId,
     pub transfer: library::Transfer,
     pub try_from_glib: TryFromGlib,
+    pub nullable: Nullable,
 }
 
 impl From<&parameter_ffi_call_out::Parameter> for Mode {
@@ -17,6 +18,7 @@ impl From<&parameter_ffi_call_out::Parameter> for Mode {
             typ: orig.typ,
             transfer: orig.transfer,
             try_from_glib: orig.try_from_glib.clone(),
+            nullable: Nullable(false),
         }
     }
 }
@@ -27,6 +29,8 @@ impl From<&analysis::Parameter> for Mode {
             typ: orig.lib_par.typ,
             transfer: orig.lib_par.transfer,
             try_from_glib: orig.try_from_glib.clone(),
+            // FIXME apply override?
+            nullable: orig.lib_par.nullable,
         }
     }
 }

src/codegen/properties.rs

@@ -103,7 +103,7 @@ fn declaration(env: &Env, prop: &Property) -> String {
         let dir = library::ParameterDirection::Return;
         let ret_type = RustType::builder(env, prop.typ)
             .direction(dir)
-            .nullable(prop.nullable)
+            .nullable(prop.nullable & !prop.is_array)
             .ref_mode(prop.get_out_ref_mode)
             .try_build_param()
             .into_string();

src/codegen/translate_from_glib.rs

@@ -62,10 +62,21 @@ impl TranslateFromGlib for Mode {
                     | library::Type::SList(..)
                     | library::Type::PtrArray(..)
                     | library::Type::CArray(..) => {
+                        let (pre1, pre2) = if *self.nullable {
+                            ("Maybe", "maybe_")
+                        } else {
+                            ("", "")
+                        };
                         if array_length.is_some() {
-                            (format!("FromGlibContainer::{}", trans.0), trans.1)
+                            (
+                                format!("{pre1}FromGlibContainer::{pre2}{}", trans.0),
+                                trans.1,
+                            )
                         } else {
-                            (format!("FromGlibPtrContainer::{}", trans.0), trans.1)
+                            (
+                                format!("{pre1}FromGlibPtrContainer::{pre2}{}", trans.0),
+                                trans.1,
+                            )
                         }
                     }
                     _ => trans,

src/library.rs

@@ -3,7 +3,7 @@ use std::{
     collections::{BTreeMap, BTreeSet, HashMap, HashSet},
     fmt,
     iter::Iterator,
-    ops::{Deref, DerefMut},
+    ops::{BitAnd, Deref, DerefMut},
     str::FromStr,
 };
 
@@ -135,6 +135,14 @@ impl DerefMut for Nullable {
     }
 }
 
+impl BitAnd<bool> for Nullable {
+    type Output = Nullable;
+
+    fn bitand(self, rhs: bool) -> Nullable {
+        Nullable(*self & rhs)
+    }
+}
+
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub struct Mandatory(pub bool);
 
@@ -513,6 +521,7 @@ pub struct Property {
     pub typ: TypeId,
     pub c_type: Option<String>,
     pub transfer: Transfer,
+    pub is_array: bool,
     pub version: Option<Version>,
     pub deprecated_version: Option<Version>,
     pub doc: Option<String>,
@@ -531,6 +540,7 @@ pub struct Parameter {
     pub transfer: Transfer,
     pub caller_allocates: bool,
     pub nullable: Nullable,
+    pub is_array: bool,
     pub array_length: Option<u32>,
     pub zero_terminated: Option<bool>,
     pub is_error: bool,