[refactor] Simplify handling of trait bounds and aliases

src/analysis/bounds.rs

@@ -38,7 +38,6 @@ pub struct Bound {
     pub parameter_name: String,
     pub alias: char,
     pub type_str: String,
-    pub info_for_next_type: bool,
     pub callback_modified: bool,
 }
 
@@ -53,9 +52,8 @@ pub struct Bounds {
 impl Default for Bounds {
     fn default() -> Bounds {
         Bounds {
-            unused: (TYPE_PARAMETERS_START as u8..)
-                .take_while(|x| *x <= b'Z')
-                .map(|x| x as char)
+            unused: (TYPE_PARAMETERS_START..)
+                .take_while(|x| *x <= 'Z')
                 .collect(),
             used: Vec::new(),
             unused_lifetimes: "abcdefg".chars().collect(),
@@ -153,14 +151,8 @@ impl Bounds {
                         });
                     }
                 }
-                if (!need_is_into_check || !*par.nullable)
-                    && par.c_type != "GDestroyNotify"
-                    && !self.add_parameter(&par.name, &type_string, bound_type, r#async)
-                {
-                    panic!(
-                        "Too many type constraints for {}",
-                        func.c_identifier.as_ref().unwrap()
-                    )
+                if (!need_is_into_check || !*par.nullable) && par.c_type != "GDestroyNotify" {
+                    self.add_parameter(&par.name, &type_string, bound_type, r#async)
                 }
             }
         } else if par.instance_parameter {
@@ -205,69 +197,27 @@ impl Bounds {
         &mut self,
         name: &str,
         type_str: &str,
-        bound_type: BoundType,
+        mut bound_type: BoundType,
         r#async: bool,
-    ) -> bool {
+    ) {
         if r#async && name == "callback" {
-            if let Some(alias) = self.unused.pop_front() {
-                self.used.push(Bound {
-                    bound_type: BoundType::NoWrapper,
-                    parameter_name: name.to_owned(),
-                    alias,
-                    type_str: type_str.to_string(),
-                    info_for_next_type: false,
-                    callback_modified: false,
-                });
-                return true;
-            }
-            return false;
+            bound_type = BoundType::NoWrapper;
         }
         if self.used.iter().any(|n| n.parameter_name == name) {
-            return false;
-        }
-        if let Some(alias) = self.unused.pop_front() {
-            self.used.push(Bound {
-                bound_type,
-                parameter_name: name.to_owned(),
-                alias,
-                type_str: type_str.to_owned(),
-                info_for_next_type: false,
-                callback_modified: false,
-            });
-            true
-        } else {
-            false
+            return;
         }
+        let alias = self.unused.pop_front().expect("No free type aliases!");
+        self.used.push(Bound {
+            bound_type,
+            parameter_name: name.to_owned(),
+            alias,
+            type_str: type_str.to_owned(),
+            callback_modified: false,
+        });
     }
 
-    pub fn get_parameter_alias_info(&self, name: &str) -> Option<(char, BoundType)> {
-        self.used
-            .iter()
-            .find(move |n| {
-                if n.parameter_name == name {
-                    !n.info_for_next_type
-                } else {
-                    false
-                }
-            })
-            .map(|t| (t.alias, t.bound_type.clone()))
-    }
-
-    pub fn get_base_alias(&self, alias: char) -> Option<char> {
-        if alias == TYPE_PARAMETERS_START {
-            return None;
-        }
-        let prev_alias = ((alias as u8) - 1) as char;
-        self.used
-            .iter()
-            .find(move |n| n.alias == prev_alias)
-            .and_then(|b| {
-                if b.info_for_next_type {
-                    Some(b.alias)
-                } else {
-                    None
-                }
-            })
+    pub fn get_parameter_bound(&self, name: &str) -> Option<&Bound> {
+        self.iter().find(move |n| n.parameter_name == name)
     }
 
     pub fn update_imports(&self, imports: &mut Imports) {
@@ -380,34 +330,49 @@ mod tests {
     fn get_new_all() {
         let mut bounds: Bounds = Default::default();
         let typ = BoundType::IsA(None);
-        assert_eq!(bounds.add_parameter("a", "", typ.clone(), false), true);
+        bounds.add_parameter("a", "", typ.clone(), false);
+        assert_eq!(bounds.iter().len(), 1);
         // Don't add second time
-        assert_eq!(bounds.add_parameter("a", "", typ.clone(), false), false);
-        assert_eq!(bounds.add_parameter("b", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("c", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("d", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("e", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("f", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("g", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("h", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("h", "", typ.clone(), false), false);
-        assert_eq!(bounds.add_parameter("i", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("j", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("k", "", typ.clone(), false), true);
-        assert_eq!(bounds.add_parameter("l", "", typ, false), false);
+        bounds.add_parameter("a", "", typ.clone(), false);
+        assert_eq!(bounds.iter().len(), 1);
+        bounds.add_parameter("b", "", typ.clone(), false);
+        bounds.add_parameter("c", "", typ.clone(), false);
+        bounds.add_parameter("d", "", typ.clone(), false);
+        bounds.add_parameter("e", "", typ.clone(), false);
+        bounds.add_parameter("f", "", typ.clone(), false);
+        bounds.add_parameter("g", "", typ.clone(), false);
+        bounds.add_parameter("h", "", typ.clone(), false);
+        assert_eq!(bounds.iter().len(), 8);
+        bounds.add_parameter("h", "", typ.clone(), false);
+        assert_eq!(bounds.iter().len(), 8);
+        bounds.add_parameter("i", "", typ.clone(), false);
+        bounds.add_parameter("j", "", typ.clone(), false);
+        bounds.add_parameter("k", "", typ, false);
+    }
+
+    #[test]
+    #[should_panic(expected = "No free type aliases!")]
+    fn exhaust_type_parameters() {
+        let mut bounds: Bounds = Default::default();
+        let typ = BoundType::NoWrapper;
+        for c in 'a'..='l' {
+            // Should panic on `l` because all type parameters are exhausted
+            bounds.add_parameter(c.to_string().as_str(), "", typ.clone(), false);
+        }
     }
 
     #[test]
-    fn get_parameter_alias_info() {
+    fn get_parameter_bound() {
         let mut bounds: Bounds = Default::default();
         let typ = BoundType::IsA(None);
         bounds.add_parameter("a", "", typ.clone(), false);
         bounds.add_parameter("b", "", typ.clone(), false);
-        assert_eq!(
-            bounds.get_parameter_alias_info("a"),
-            Some(('P', typ.clone()))
-        );
-        assert_eq!(bounds.get_parameter_alias_info("b"), Some(('Q', typ)));
-        assert_eq!(bounds.get_parameter_alias_info("c"), None);
+        let bound = bounds.get_parameter_bound("a").unwrap();
+        assert_eq!(bound.alias, 'P');
+        assert_eq!(bound.bound_type, typ);
+        let bound = bounds.get_parameter_bound("b").unwrap();
+        assert_eq!(bound.alias, 'Q');
+        assert_eq!(bound.bound_type, typ);
+        assert_eq!(bounds.get_parameter_bound("c"), None);
     }
 }

src/analysis/rust_type.rs

@@ -121,12 +121,9 @@ impl RustType {
 
     #[inline]
     fn apply_ref_mode(self, ref_mode: RefMode) -> Self {
-        match ref_mode {
-            RefMode::None | RefMode::ByRefFake => self,
-            RefMode::ByRef | RefMode::ByRefImmut | RefMode::ByRefConst => {
-                self.alter_type(|typ_| format!("&{}", typ_))
-            }
-            RefMode::ByRefMut => self.alter_type(|typ_| format!("&mut {}", typ_)),
+        match ref_mode.for_rust_type() {
+            "" => self,
+            ref_mode => self.alter_type(|typ_| format!("{}{}", ref_mode, typ_)),
         }
     }
 }

src/codegen/bound.rs

@@ -0,0 +1,61 @@
+use crate::{
+    analysis::{
+        bounds::{Bound, BoundType},
+        ref_mode::RefMode,
+    },
+    library::Nullable,
+};
+
+impl Bound {
+    /// Returns the type parameter reference.
+    /// Currently always returns the alias.
+    pub(super) fn type_parameter_reference(&self) -> char {
+        self.alias
+    }
+
+    /// Returns the type parameter reference, with [`BoundType::IsA`] wrapped
+    /// in `ref_mode` and `nullable` as appropriate.
+    pub(super) fn full_type_parameter_reference(
+        &self,
+        ref_mode: RefMode,
+        nullable: Nullable,
+    ) -> String {
+        let t = self.type_parameter_reference();
+        let ref_str = ref_mode.for_rust_type();
+        match self.bound_type {
+            BoundType::IsA(_) if *nullable => {
+                format!("Option<{}{}>", ref_str, t)
+            }
+            BoundType::IsA(_) => format!("{}{}", ref_str, t),
+            BoundType::NoWrapper | BoundType::AsRef(_) => t.to_string(),
+        }
+    }
+
+    /// Returns the type parameter definition for this bound, usually
+    /// of the form `T: SomeTrait` or `T: IsA<Foo>`.
+    pub(super) fn type_parameter_definition(&self, r#async: bool) -> String {
+        format!("{}: {}", self.alias, self.trait_bound(r#async))
+    }
+
+    /// Returns the trait bound, usually of the form `SomeTrait`
+    /// or `IsA<Foo>`.
+    pub(super) fn trait_bound(&self, r#async: bool) -> String {
+        match self.bound_type {
+            BoundType::NoWrapper => self.type_str.clone(),
+            BoundType::IsA(lifetime) => {
+                if r#async {
+                    assert!(lifetime.is_none(), "Async overwrites lifetime");
+                }
+                let is_a = format!("IsA<{}>", self.type_str);
+                let lifetime = r#async
+                    .then(|| " + Clone + 'static".to_string())
+                    .or_else(|| lifetime.map(|l| format!(" + '{}", l)))
+                    .unwrap_or_default();
+
+                format!("{}{}", is_a, lifetime)
+            }
+            BoundType::AsRef(Some(_ /*lifetime*/)) => panic!("AsRef cannot have a lifetime"),
+            BoundType::AsRef(None) => format!("AsRef<{}>", self.type_str),
+        }
+    }
+}

src/codegen/function.rs

@@ -10,11 +10,7 @@ use super::{
 };
 use crate::{
     analysis::{
-        self,
-        bounds::{Bound, Bounds},
-        functions::Visibility,
-        namespaces,
-        try_from_glib::TryFromGlib,
+        self, bounds::Bounds, functions::Visibility, namespaces, try_from_glib::TryFromGlib,
     },
     chunk::{ffi_function_todo, Chunk},
     env::Env,
@@ -214,11 +210,8 @@ pub fn declaration_futures(env: &Env, analysis: &analysis::functions::Info) -> S
 
         if c_par.name == "callback" || c_par.name == "cancellable" {
             skipped += 1;
-            if let Some((t, _)) = analysis.bounds.get_parameter_alias_info(&c_par.name) {
-                skipped_bounds.push(t);
-                if let Some(p) = analysis.bounds.get_base_alias(t) {
-                    skipped_bounds.push(p);
-                }
+            if let Some(bound) = analysis.bounds.get_parameter_bound(&c_par.name) {
+                skipped_bounds.push(bound.type_parameter_reference());
             }
             continue;
         }
@@ -239,36 +232,6 @@ pub fn declaration_futures(env: &Env, analysis: &analysis::functions::Info) -> S
     )
 }
 
-pub fn bound_to_string(bound: &Bound, r#async: bool) -> String {
-    use crate::analysis::bounds::BoundType::*;
-
-    match bound.bound_type {
-        NoWrapper => format!("{}: {}", bound.alias, bound.type_str),
-        IsA(Some(lifetime)) => format!(
-            "{}: IsA<{}> + {}",
-            bound.alias,
-            bound.type_str,
-            if r#async {
-                "Clone + 'static".into()
-            } else {
-                format!("'{}", lifetime)
-            }
-        ),
-        IsA(None) => format!(
-            "{}: IsA<{}>{}",
-            bound.alias,
-            bound.type_str,
-            if r#async { " + Clone + 'static" } else { "" }
-        ),
-        // This case should normally never happened
-        AsRef(Some(_ /*lifetime*/)) => {
-            unreachable!();
-            // format!("{}: AsRef<{}> + '{}", bound.alias, bound.type_str, lifetime)
-        }
-        AsRef(None) => format!("{}: AsRef<{}>", bound.alias, bound.type_str),
-    }
-}
-
 pub fn bounds(
     bounds: &Bounds,
     skip: &[char],
@@ -290,40 +253,34 @@ pub fn bounds(
         })
         .collect::<Vec<_>>();
 
-    let strs: Vec<String> = bounds
+    let lifetimes = bounds
         .iter_lifetimes()
         .filter(|s| !skip_lifetimes.contains(s))
         .map(|s| format!("'{}", s))
-        .chain(
-            bounds
-                .iter()
-                .filter(|bound| {
-                    !skip.contains(&bound.alias)
-                        && (!filter_callback_modified || !bound.callback_modified)
-                })
-                .map(|b| bound_to_string(b, r#async)),
-        )
-        .collect();
-
-    if strs.is_empty() {
-        (String::new(), Vec::new())
+        .collect::<Vec<_>>();
+
+    let bounds = bounds.iter().filter(|bound| {
+        !skip.contains(&bound.alias) && (!filter_callback_modified || !bound.callback_modified)
+    });
+
+    let type_names = lifetimes
+        .iter()
+        .cloned()
+        .chain(bounds.clone().map(|b| b.type_parameter_definition(r#async)))
+        .collect::<Vec<_>>();
+
+    let type_names = if type_names.is_empty() {
+        String::new()
     } else {
-        let bounds = bounds
-            .iter_lifetimes()
-            .filter(|s| !skip_lifetimes.contains(s))
-            .map(|s| format!("'{}", s))
-            .chain(
-                bounds
-                    .iter()
-                    .filter(|bound| {
-                        !skip.contains(&bound.alias)
-                            && (!filter_callback_modified || !bound.callback_modified)
-                    })
-                    .map(|b| b.alias.to_string()),
-            )
-            .collect::<Vec<_>>();
-        (format!("<{}>", strs.join(", ")), bounds)
-    }
+        format!("<{}>", type_names.join(", "))
+    };
+
+    let bounds = lifetimes
+        .into_iter()
+        .chain(bounds.map(|b| b.alias.to_string()))
+        .collect::<Vec<_>>();
+
+    (type_names, bounds)
 }
 
 pub fn body_chunk(env: &Env, analysis: &analysis::functions::Info) -> Chunk {