From 33e0002de468e5fd6014d214962673af20d28f3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Fri, 29 Nov 2024 10:38:22 +0100 Subject: [PATCH 1/4] Fix clippy 1.83 warning --- src/nameutil.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/nameutil.rs b/src/nameutil.rs index 09b2c85ef..0529a2164 100644 --- a/src/nameutil.rs +++ b/src/nameutil.rs @@ -2,27 +2,19 @@ use std::{borrow::Cow, collections::HashMap, path::*, sync::OnceLock}; use crate::case::*; -static mut CRATE_NAME_OVERRIDES: Option> = None; +static CRATE_NAME_OVERRIDES: OnceLock> = OnceLock::new(); pub(crate) fn set_crate_name_overrides(overrides: HashMap) { - unsafe { - assert!( - CRATE_NAME_OVERRIDES.is_none(), - "Crate name overrides already set" - ); - CRATE_NAME_OVERRIDES = Some(overrides); - } + assert!( + CRATE_NAME_OVERRIDES.set(overrides).is_ok(), + "Crate name overrides already set" + ); } fn get_crate_name_override(crate_name: &str) -> Option { - unsafe { - if let Some(ref overrides) = CRATE_NAME_OVERRIDES { - if let Some(crate_name) = overrides.get(crate_name) { - return Some(crate_name.clone()); - } - } - None - } + CRATE_NAME_OVERRIDES + .get() + .and_then(|overrides| overrides.get(crate_name).cloned()) } pub fn split_namespace_name(name: &str) -> (Option<&str>, &str) { From ae29aefbe96ac4ea5bebf6be561aaaf1d443f08f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Sun, 1 Dec 2024 15:43:40 +0100 Subject: [PATCH 2/4] Rework RustType in preparation for more Into> in arg position Option and references in types were mostly handled in `try_build()` which prevented callers from composing alternative types. This commit redistributes responsabilities between `try_build()` & `try_build_param()` so they actually target their specific use cases: standalone type or function argument (also known as parameter in the code base). These changes are needed to introduce new `BoundType`s in order to generate additional `impl Into>` in argument position. Also: * Update call sites to use the appropriate `try_build()` or `try_build_param()` with required proprties depending on the intention. * Factorised the Unknown Conversion detection, though I suspect it's not really useful and already caught by checking the Unimplemented Type set. * Fixed trampoline `Option` return value incorrectly generated as `Option` because the `is_gstring()` helper function would not detect the wrapped `GString`. Seen in GES for Project's missing-uri signal. * Add missing parameter config handling for a corner case in return position. * Reworded a couple of identifiers so as to help me better understand the intention. Hopefully, that could also help others. * This incidentally fixed the generation argument references and `Option` in commented out callbacks. While this doesn't improve code in use, it improves the accuracy of generated signatures. * Removed one layer of error reported in some commented function signatures. Some commented out function signatures contained multiple error reports for a single argument, e.g. /*Unknown conversion*//*Unimplemented*/, which added verbosity whithout providing any meaningful insight. Only one is reported now. The last two cause diff in commented methods comparer to previous versions. This was considered not a problem since this will not produce more diffs in subsequent re-generations. --- src/analysis/bounds.rs | 37 ++- src/analysis/class_builder.rs | 4 +- src/analysis/ffi_type.rs | 8 +- src/analysis/functions.rs | 15 +- src/analysis/properties.rs | 4 +- src/analysis/ref_mode.rs | 37 ++- src/analysis/return_value.rs | 2 +- src/analysis/rust_type.rs | 476 +++++++++++++++----------------- src/analysis/trampolines.rs | 47 +--- src/codegen/bound.rs | 2 +- src/codegen/child_properties.rs | 15 +- src/codegen/function.rs | 6 +- src/codegen/mod.rs | 1 - src/codegen/object.rs | 4 +- src/codegen/parameter.rs | 34 +-- src/codegen/properties.rs | 15 +- src/codegen/property_body.rs | 40 ++- src/codegen/ref_mode.rs | 11 - src/codegen/return_value.rs | 50 ++-- src/codegen/sys/ffi_type.rs | 12 +- src/codegen/sys/fields.rs | 6 +- src/codegen/trampoline.rs | 6 +- 22 files changed, 370 insertions(+), 462 deletions(-) delete mode 100644 src/codegen/ref_mode.rs diff --git a/src/analysis/bounds.rs b/src/analysis/bounds.rs index 30d1339f3..5e746a7d3 100644 --- a/src/analysis/bounds.rs +++ b/src/analysis/bounds.rs @@ -13,7 +13,7 @@ use crate::{ consts::TYPE_PARAMETERS_START, env::Env, library::{Basic, Class, Concurrency, Function, ParameterDirection, Type, TypeId}, - traits::IntoString, + traits::*, }; #[derive(Clone, Eq, Debug, PartialEq)] @@ -86,13 +86,24 @@ impl Bounds { concurrency: Concurrency, configured_functions: &[&config::functions::Function], ) -> (Option, Option) { - let type_name = RustType::builder(env, par.typ) - .ref_mode(RefMode::ByRefFake) - .try_build(); - if type_name.is_err() { + let nullable = configured_functions + .matched_parameters(&par.name) + .iter() + .find_map(|p| p.nullable) + .unwrap_or(par.nullable); + let Ok(rust_type) = RustType::builder(env, par.typ) + .direction(par.direction) + .nullable(nullable) + .ref_mode(if par.move_ { + RefMode::None + } else { + par.ref_mode + }) + .try_build() + else { return (None, None); - } - let mut type_string = type_name.into_string(); + }; + let mut type_string = rust_type.into_string(); let mut callback_info = None; let mut ret = None; let mut need_is_into_check = false; @@ -134,7 +145,7 @@ impl Bounds { RustType::builder(env, function.ret.typ) .direction(function.ret.direction) .nullable(nullable) - .try_build() + .try_build_param() .into_string(), ); } @@ -342,19 +353,15 @@ fn find_out_parameters( // function but that a) happens afterwards and b) is not accessible // from here either. let nullable = configured_functions + .matched_parameters(¶m.name) .iter() - .find_map(|f| { - f.parameters - .iter() - .filter(|p| p.ident.is_match(¶m.name)) - .find_map(|p| p.nullable) - }) + .find_map(|p| p.nullable) .unwrap_or(param.nullable); RustType::builder(env, param.typ) .direction(param.direction) .nullable(nullable) - .try_build() + .try_build_param() .into_string() }) .collect() diff --git a/src/analysis/class_builder.rs b/src/analysis/class_builder.rs index f7ab0870e..19380445e 100644 --- a/src/analysis/class_builder.rs +++ b/src/analysis/class_builder.rs @@ -121,7 +121,7 @@ fn analyze_property( } } - let (get_out_ref_mode, set_in_ref_mode, nullable) = get_property_ref_modes(env, prop); + let (get_out_ref_mode, set_in_ref_mode, _) = get_property_ref_modes(env, prop); let mut bounds = Bounds::default(); if let Some(bound) = Bounds::type_for(env, prop.typ) { @@ -136,7 +136,7 @@ fn analyze_property( is_get: false, func_name: String::new(), func_name_alias: None, - nullable, + nullable: library::Nullable(false), // no Options for builder setters here get_out_ref_mode, set_in_ref_mode, set_bound: None, diff --git a/src/analysis/ffi_type.rs b/src/analysis/ffi_type.rs index 60926257a..2967272ff 100644 --- a/src/analysis/ffi_type.rs +++ b/src/analysis/ffi_type.rs @@ -122,7 +122,7 @@ fn ffi_inner(env: &Env, tid: TypeId, inner: &str) -> Result { typ.get_name() ); warn_main!(tid, "{}", msg); - return Err(TypeError::Mismatch(msg)); + return Err(TypeError::mismatch(msg)); } } else { warn_main!(tid, "Type `{}` missing c_type", typ.get_name()); @@ -156,7 +156,7 @@ fn ffi_inner(env: &Env, tid: TypeId, inner: &str) -> Result { env.library.type_(tid).get_name() ); warn_main!(tid, "{}", msg); - Err(TypeError::Mismatch(msg)) + Err(TypeError::mismatch(msg)) } } else { fix_name(env, tid, inner) @@ -168,7 +168,7 @@ fn ffi_inner(env: &Env, tid: TypeId, inner: &str) -> Result { inner ); warn_main!(tid, "{}", msg); - Err(TypeError::Mismatch(msg)) + Err(TypeError::mismatch(msg)) } } } @@ -201,7 +201,7 @@ fn fix_name(env: &Env, type_id: TypeId, name: &str) -> Result { .type_status_sys(&type_id.full_name(&env.library)) .ignored() { - Err(TypeError::Ignored(name_with_prefix)) + Err(TypeError::ignored(name_with_prefix)) } else { Ok(name_with_prefix.into()) } diff --git a/src/analysis/functions.rs b/src/analysis/functions.rs index e3fbe1944..84a5bb7ca 100644 --- a/src/analysis/functions.rs +++ b/src/analysis/functions.rs @@ -370,7 +370,8 @@ fn analyze_callbacks( if let Ok(rust_type) = RustType::builder(env, par.typ) .direction(par.direction) .try_from_glib(&par.try_from_glib) - .try_build() + .for_callback(true) + .try_build_param() { used_types.extend(rust_type.into_used_types()); } @@ -752,7 +753,7 @@ fn analyze_function( if let Ok(rust_type) = RustType::builder(env, par.typ) .direction(par.direction) .try_from_glib(&par.try_from_glib) - .try_build() + .try_build_param() { if !rust_type.as_str().ends_with("GString") || par.c_type == "gchar***" { used_types.extend(rust_type.into_used_types()); @@ -787,6 +788,7 @@ fn analyze_function( && *env.library.type_(par.typ) == Type::Basic(library::Basic::Pointer)) && RustType::builder(env, par.typ) .direction(par.direction) + .ref_mode(par.ref_mode) .scope(par.scope) .try_from_glib(&par.try_from_glib) .try_build_param() @@ -862,7 +864,7 @@ fn analyze_function( for out in &trampoline.output_params { if let Ok(rust_type) = RustType::builder(env, out.lib_par.typ) .direction(ParameterDirection::Out) - .try_build() + .try_build_param() { used_types.extend(rust_type.into_used_types()); } @@ -870,7 +872,7 @@ fn analyze_function( if let Some(ref out) = trampoline.ffi_ret { if let Ok(rust_type) = RustType::builder(env, out.lib_par.typ) .direction(ParameterDirection::Return) - .try_build() + .try_build_param() { used_types.extend(rust_type.into_used_types()); } @@ -1178,14 +1180,15 @@ fn analyze_callback( .direction(p.direction) .nullable(p.nullable) .try_from_glib(&p.try_from_glib) - .try_build() + .for_callback(true) + .try_build_param() { imports_to_add.extend(rust_type.into_used_types()); } } if let Ok(rust_type) = RustType::builder(env, func.ret.typ) .direction(ParameterDirection::Return) - .try_build() + .try_build_param() { if !rust_type.as_str().ends_with("GString") && !rust_type.as_str().ends_with("GAsyncResult") diff --git a/src/analysis/properties.rs b/src/analysis/properties.rs index 95fbe2bc5..69cc1878d 100644 --- a/src/analysis/properties.rs +++ b/src/analysis/properties.rs @@ -238,7 +238,7 @@ fn analyze_property( { if let Ok(rust_type) = RustType::builder(env, prop.typ) .direction(library::ParameterDirection::Out) - .try_build() + .try_build_param() { imports.add_used_types(rust_type.used_types()); } @@ -280,7 +280,7 @@ fn analyze_property( { if let Ok(rust_type) = RustType::builder(env, prop.typ) .direction(library::ParameterDirection::In) - .try_build() + .try_build_param() { imports.add_used_types(rust_type.used_types()); } diff --git a/src/analysis/ref_mode.rs b/src/analysis/ref_mode.rs index 4b87f5c07..73a319f6e 100644 --- a/src/analysis/ref_mode.rs +++ b/src/analysis/ref_mode.rs @@ -1,4 +1,7 @@ -use std::str::FromStr; +use std::{ + fmt::{self, Write}, + str::FromStr, +}; use super::{c_type::is_mut_ptr, record_type::RecordType}; use crate::{config::gobjects::GObject, env, library}; @@ -99,6 +102,7 @@ impl RefMode { Self::ByRefMut if !is_mut_ptr(&par.c_type) => Self::ByRef, Self::ByRefMut if immutable => Self::ByRefImmut, Self::ByRef if self_in_trait && !is_mut_ptr(&par.c_type) => Self::ByRefConst, + Self::None if par.direction.is_out() && !*par.nullable => Self::ByRefMut, ref_mode => ref_mode, } } @@ -106,11 +110,32 @@ impl RefMode { pub fn is_ref(self) -> bool { match self { Self::None => false, - Self::ByRef => true, - Self::ByRefMut => true, - Self::ByRefImmut => true, - Self::ByRefConst => true, - Self::ByRefFake => true, + Self::ByRef + | Self::ByRefMut + | Self::ByRefImmut + | Self::ByRefConst + | Self::ByRefFake => true, + } + } + + pub fn is_immutable(self) -> bool { + match self { + Self::None | Self::ByRefMut => false, + Self::ByRef | Self::ByRefImmut | Self::ByRefConst | Self::ByRefFake => true, + } + } + + pub fn is_none(self) -> bool { + matches!(self, Self::None) + } +} + +impl fmt::Display for RefMode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + RefMode::None | RefMode::ByRefFake => f.write_str(""), + RefMode::ByRef | RefMode::ByRefImmut | RefMode::ByRefConst => f.write_char('&'), + RefMode::ByRefMut => f.write_str("&mut "), } } } diff --git a/src/analysis/return_value.rs b/src/analysis/return_value.rs index fb9207ea2..7ed2193de 100644 --- a/src/analysis/return_value.rs +++ b/src/analysis/return_value.rs @@ -123,7 +123,7 @@ pub fn analyze( if let Ok(rust_type) = RustType::builder(env, typ) .direction(par.lib_par.direction) .try_from_glib(&par.try_from_glib) - .try_build() + .try_build_param() { used_types.extend(rust_type.into_used_types()); } diff --git a/src/analysis/rust_type.rs b/src/analysis/rust_type.rs index 400f9d9ef..f219457c3 100644 --- a/src/analysis/rust_type.rs +++ b/src/analysis/rust_type.rs @@ -1,4 +1,4 @@ -use std::{borrow::Borrow, result}; +use std::{borrow::Borrow, fmt, result}; use super::conversion_type::ConversionType; use crate::{ @@ -12,9 +12,30 @@ use crate::{ #[derive(Clone, Debug, PartialEq, Eq)] pub enum TypeError { - Ignored(String), - Mismatch(String), - Unimplemented(String), + Ignored(RustType), + Mismatch(RustType), + Unimplemented(RustType), + UnknownConversion(RustType), +} + +impl TypeError { + pub fn ignored(r: impl Into) -> Self { + Self::Ignored(r.into()) + } + pub fn mismatch(r: impl Into) -> Self { + Self::Mismatch(r.into()) + } + pub fn unimplemented(r: impl Into) -> Self { + Self::Unimplemented(r.into()) + } + pub fn message(&self) -> &'static str { + match self { + Self::Ignored(_) => "Ignored", + Self::Mismatch(_) => "Mismatch", + Self::Unimplemented(_) => "Unimplemented", + Self::UnknownConversion(_) => "Unknown conversion", + } + } } /// A `RustType` definition and its associated types to be `use`d. @@ -22,6 +43,7 @@ pub enum TypeError { pub struct RustType { inner: String, used_types: Vec, + nullable_as_option: bool, } impl RustType { @@ -40,13 +62,7 @@ impl RustType { RustType { inner: rust_type.to_string(), used_types: vec![rust_type.to_string()], - } - } - - fn new_with_uses(rust_type: &impl ToString, uses: &[impl ToString]) -> Self { - RustType { - inner: rust_type.to_string(), - used_types: uses.iter().map(ToString::to_string).collect(), + nullable_as_option: true, } } @@ -69,7 +85,7 @@ impl RustType { ); if env.type_status(&type_id.full_name(&env.library)).ignored() { - return Err(TypeError::Ignored(type_name)); + return Err(TypeError::ignored(type_name)); } } @@ -81,6 +97,7 @@ impl RustType { RustType { inner: type_name.clone(), used_types: vec![type_name], + nullable_as_option: true, } }) } @@ -93,6 +110,7 @@ impl RustType { Self::check(env, type_id, &type_name).map(|type_name| RustType { inner: type_name.clone(), used_types: vec![type_name], + nullable_as_option: true, }) } @@ -108,35 +126,24 @@ impl RustType { self.inner.as_str() } - #[inline] pub fn alter_type(mut self, op: impl FnOnce(String) -> String) -> Self { self.inner = op(self.inner); self } +} - #[inline] - fn format_parameter(self, direction: ParameterDirection) -> Self { - if direction.is_out() { - self.alter_type(|type_| format!("&mut {type_}")) - } else { - self - } - } - - #[inline] - fn apply_ref_mode(self, ref_mode: RefMode) -> Self { - match ref_mode.for_rust_type() { - "" => self, - ref_mode => self.alter_type(|typ_| format!("{ref_mode}{typ_}")), - } +impl fmt::Display for RustType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.inner) } } -impl From for RustType { +impl> From for RustType { fn from(rust_type: T) -> Self { RustType { - inner: rust_type.to_string(), + inner: rust_type.as_ref().to_string(), used_types: Vec::new(), + nullable_as_option: true, } } } @@ -149,11 +156,11 @@ impl IntoString for RustType { pub type Result = result::Result; -fn into_inner(res: Result) -> String { +fn unwrap_rust_type(res: Result) -> RustType { use self::TypeError::*; match res { - Ok(rust_type) => rust_type.into_string(), - Err(Ignored(s) | Mismatch(s) | Unimplemented(s)) => s, + Ok(r) => r, + Err(Ignored(r) | Mismatch(r) | Unimplemented(r) | UnknownConversion(r)) => r, } } @@ -161,10 +168,12 @@ impl IntoString for Result { fn into_string(self) -> String { use self::TypeError::*; match self { - Ok(s) => s.into_string(), - Err(Ignored(s)) => format!("/*Ignored*/{s}"), - Err(Mismatch(s)) => format!("/*Metadata mismatch*/{s}"), - Err(Unimplemented(s)) => format!("/*Unimplemented*/{s}"), + Ok(r) => r.into_string(), + Err(ref err) => match err { + Ignored(r) | Mismatch(r) | Unimplemented(r) | UnknownConversion(r) => { + format!("/*{}*/{r}", err.message()) + } + }, } } } @@ -173,10 +182,11 @@ impl MapAny for Result { fn map_any RustType>(self, op: F) -> Result { use self::TypeError::*; match self { - Ok(rust_type) => Ok(op(rust_type)), - Err(Ignored(s)) => Err(Ignored(op(s.into()).into_string())), - Err(Mismatch(s)) => Err(Mismatch(op(s.into()).into_string())), - Err(Unimplemented(s)) => Err(Unimplemented(op(s.into()).into_string())), + Ok(r) => Ok(op(r)), + Err(Ignored(r)) => Err(Ignored(op(r))), + Err(Mismatch(r)) => Err(Mismatch(op(r))), + Err(Unimplemented(r)) => Err(Unimplemented(op(r))), + Err(UnknownConversion(r)) => Err(UnknownConversion(op(r))), } } } @@ -191,6 +201,7 @@ pub struct RustTypeBuilder<'env> { concurrency: library::Concurrency, try_from_glib: TryFromGlib, callback_parameters_config: CallbackParameters, + is_for_callback: bool, } impl<'env> RustTypeBuilder<'env> { @@ -205,6 +216,7 @@ impl<'env> RustTypeBuilder<'env> { concurrency: library::Concurrency::None, try_from_glib: TryFromGlib::default(), callback_parameters_config: Vec::new(), + is_for_callback: false, } } @@ -246,14 +258,18 @@ impl<'env> RustTypeBuilder<'env> { self } + pub fn for_callback(mut self, is_for_callback: bool) -> Self { + self.is_for_callback = is_for_callback; + self + } + pub fn try_build(self) -> Result { use crate::library::{Basic::*, Type::*}; 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_ { + match *type_ { Basic(fund) => { match fund { None => err("()"), @@ -291,21 +307,21 @@ impl<'env> RustTypeBuilder<'env> { UniChar => ok("char"), Utf8 => { - if self.ref_mode.is_ref() { + if self.ref_mode.is_immutable() { ok("str") } else { ok_and_use(&use_glib_type(self.env, "GString")) } } Filename => { - if self.ref_mode.is_ref() { + if self.ref_mode.is_immutable() { ok_and_use("std::path::Path") } else { ok_and_use("std::path::PathBuf") } } OsString => { - if self.ref_mode.is_ref() { + if self.ref_mode.is_immutable() { ok_and_use("std::ffi::OsStr") } else { ok_and_use("std::ffi::OsString") @@ -317,9 +333,17 @@ impl<'env> RustTypeBuilder<'env> { Unsupported => err("Unsupported"), _ => err(&format!("Basic: {fund:?}")), } + .map_any(|mut r| { + r.nullable_as_option = !( + // passed to ffi as pointer to the stack allocated type + self.direction == ParameterDirection::Out + && ConversionType::of(self.env, self.type_id) == ConversionType::Direct + ); + r + }) } Alias(ref alias) => { - RustType::try_new_and_use(self.env, self.type_id).and_then(|alias_rust_type| { + RustType::try_new_and_use(self.env, self.type_id).and_then(|mut r| { RustType::builder(self.env, alias.typ) .direction(self.direction) .nullable(self.nullable) @@ -327,12 +351,17 @@ impl<'env> RustTypeBuilder<'env> { .scope(self.scope) .concurrency(self.concurrency) .try_from_glib(&self.try_from_glib) + .callback_parameters_config(self.callback_parameters_config.as_ref()) + .for_callback(self.is_for_callback) .try_build() - .map_any(|_| alias_rust_type) + .map_any(|alias_r| { + r.nullable_as_option = alias_r.nullable_as_option; + r + }) }) } Record(library::Record { ref c_type, .. }) if c_type == "GVariantType" => { - let type_name = if self.ref_mode.is_ref() { + let type_name = if self.ref_mode.is_immutable() { "VariantTy" } else { "VariantType" @@ -346,7 +375,7 @@ impl<'env> RustTypeBuilder<'env> { .type_status(&self.type_id.full_name(&self.env.library)) .ignored() { - Err(TypeError::Ignored(rust_type.into_string())) + Err(TypeError::ignored(rust_type)) } else { Ok(rust_type) } @@ -355,7 +384,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) { @@ -376,14 +404,14 @@ impl<'env> RustTypeBuilder<'env> { .scope(self.scope) .concurrency(self.concurrency) .try_build() - .map_any(|rust_type| { - rust_type.alter_type(|typ| { - if self.ref_mode.is_ref() { - format!("[{typ}]") - } else { - format!("Vec<{typ}>") - } - }) + .map_any(|mut r| { + r.nullable_as_option = false; // null is empty + r.inner = if self.ref_mode.is_immutable() { + format!("[{}{}]", inner_ref_mode, r.inner) + } else { + format!("Vec<{}>", r.inner) + }; + r }) } CArray(inner_tid) @@ -409,18 +437,21 @@ impl<'env> RustTypeBuilder<'env> { }; if let Some(s) = array_type { - skip_option = true; - if self.ref_mode.is_ref() { - Ok(format!("[{s}]").into()) + if self.ref_mode.is_immutable() { + ok(&format!("[{s}]")) } else { - Ok(format!("Vec<{s}>").into()) + ok(&format!("Vec<{s}>")) } } else { - Err(TypeError::Unimplemented(type_.get_name())) + Err(TypeError::unimplemented(type_.get_name())) } } else { - Err(TypeError::Unimplemented(type_.get_name())) + Err(TypeError::unimplemented(type_.get_name())) } + .map_any(|mut r| { + r.nullable_as_option = false; // null is empty + r + }) } Custom(library::Custom { ref name, .. }) => { RustType::try_new_and_use_with_name(self.env, self.type_id, name) @@ -447,50 +478,36 @@ impl<'env> RustTypeBuilder<'env> { } else if full_name == "GLib.DestroyNotify" { return Ok(format!("Fn(){concurrency} + 'static").into()); } + + let mut has_param_err = false; let mut params = Vec::with_capacity(f.parameters.len()); - let mut err = false; for p in &f.parameters { if p.closure.is_some() { continue; } - let nullable = self .callback_parameters_config .iter() .find(|cp| cp.ident.is_match(&p.name)) .and_then(|c| c.nullable) .unwrap_or(p.nullable); - let p_res = RustType::builder(self.env, p.typ) + let ref_mode = if p.typ.is_basic_type(self.env) { + RefMode::None + } else { + RefMode::ByRef + }; + let param = RustType::builder(self.env, p.typ) .direction(p.direction) .nullable(nullable) - .try_build(); - match p_res { - Ok(p_rust_type) => { - let is_basic = p.typ.is_basic_type(self.env); - let y = RustType::try_new(self.env, p.typ) - .unwrap_or_else(|_| RustType::default()); - params.push(format!( - "{}{}", - if is_basic || *nullable { "" } else { "&" }, - if !is_gstring(y.as_str()) { - if !is_basic && *nullable { - p_rust_type.into_string().replace("Option<", "Option<&") - } else { - p_rust_type.into_string() - } - } else if *nullable { - "Option<&str>".to_owned() - } else { - "&str".to_owned() - } - )); - } - e => { - err = true; - params.push(e.into_string()); - } - } + .ref_mode(ref_mode) + .for_callback(true) + .scope(self.scope) + .try_build_param(); + has_param_err |= param.is_err(); + params.push(param.into_string()); } + let params = params.join(", "); + let closure_kind = if self.scope.is_call() { "FnMut" } else if self.scope.is_async() { @@ -498,200 +515,141 @@ impl<'env> RustTypeBuilder<'env> { } else { "Fn" }; - let ret_res = RustType::builder(self.env, f.ret.typ) - .direction(f.ret.direction) - .nullable(f.ret.nullable) - .try_build(); - let ret = match ret_res { - Ok(ret_rust_type) => { - let y = RustType::try_new(self.env, f.ret.typ) - .unwrap_or_else(|_| RustType::default()); - format!( - "{}({}) -> {}{}", - closure_kind, - params.join(", "), - if !is_gstring(y.as_str()) { - ret_rust_type.as_str() - } else if *f.ret.nullable { - "Option" - } else { - "String" - }, - concurrency - ) - } - Err(TypeError::Unimplemented(ref x)) if x == "()" => { - format!("{}({}){}", closure_kind, params.join(", "), concurrency) - } - e => { - err = true; - format!( - "{}({}) -> {}{}", - closure_kind, - params.join(", "), - e.into_string(), - concurrency - ) - } - }; - if err { - return Err(TypeError::Unimplemented(ret)); - } - Ok(if *self.nullable { - if self.scope.is_call() { - format!("Option<&mut dyn ({ret})>") - } else { - format!("Option>") - } + let res = if f.ret.c_type == "void" { + Ok(format!("{closure_kind}({params}){concurrency}").into()) } else { - format!( - "{}{}", - ret, - if self.scope.is_call() { - "" - } else { - " + 'static" - } - ) - } - .into()) - } - _ => Err(TypeError::Unimplemented(type_.get_name())), - }; - - match self - .try_from_glib - .or_type_defaults(self.env, self.type_id) - .borrow() - { - TryFromGlib::Option => { - rust_type = rust_type.map_any(|rust_type| { - rust_type - .alter_type(|typ_| { - let mut opt = format!("Option<{typ_}>"); - if self.direction == ParameterDirection::In { - opt = format!("impl Into<{opt}>"); - } - - opt + RustType::builder(self.env, f.ret.typ) + .direction(f.ret.direction) + .nullable(f.ret.nullable) + .for_callback(true) + .try_build_param() + .map_any(|mut r| { + r.inner = format!("{closure_kind}({params}) -> {r}{concurrency}"); + r }) - .apply_ref_mode(self.ref_mode) - }); - } - TryFromGlib::Result { ok_type, err_type } => { - if self.direction == ParameterDirection::In { - rust_type = rust_type.map_any(|rust_type| { - RustType::new_with_uses( - &format!("impl Into<{}>", &rust_type.as_str()), - &[&rust_type.as_str()], - ) - }); - } else { - rust_type = rust_type.map_any(|_| { - RustType::new_with_uses( - &format!("Result<{}, {}>", &ok_type, &err_type), - &[ok_type, err_type], - ) - }); } - } - TryFromGlib::ResultInfallible { ok_type } => { - let new_rust_type = RustType::new_and_use(ok_type).apply_ref_mode(self.ref_mode); - rust_type = rust_type.map_any(|_| new_rust_type); - } - _ => { - rust_type = rust_type.map_any(|rust_type| rust_type.apply_ref_mode(self.ref_mode)); - } - } + .map_any(|mut func| { + // Handle nullability here as it affects the type e.g. for bounds + func.nullable_as_option = false; + match (*self.nullable, self.scope.is_call()) { + (false, true) => (), + (false, false) => func.inner = format!("{func} + 'static"), + (true, true) => func.inner = format!("Option<&mut dyn ({func})>"), + (true, false) => func.inner = format!("Option>"), + } + func + }); - if *self.nullable && !skip_option { - match ConversionType::of(self.env, self.type_id) { - ConversionType::Pointer | ConversionType::Scalar => { - rust_type = rust_type.map_any(|rust_type| { - rust_type.alter_type(|typ_| format!("Option<{typ_}>")) - }); + if has_param_err { + return Err(TypeError::unimplemented(unwrap_rust_type(res))); } - _ => (), + + res } + _ => Err(TypeError::unimplemented(type_.get_name())), } - - rust_type } pub fn try_build_param(self) -> Result { use crate::library::Type::*; let type_ = self.env.library.type_(self.type_id); - assert!( self.direction != ParameterDirection::None, "undefined direction for parameter with type {type_:?}" ); - - let rust_type = RustType::builder(self.env, self.type_id) + let res = RustType::builder(self.env, self.type_id) .direction(self.direction) .nullable(self.nullable) .ref_mode(self.ref_mode) .scope(self.scope) + .concurrency(self.concurrency) .try_from_glib(&self.try_from_glib) + .callback_parameters_config(self.callback_parameters_config.as_ref()) + .for_callback(self.is_for_callback) .try_build(); + match type_ { - Basic(library::Basic::Utf8 | library::Basic::OsString | library::Basic::Filename) - if (self.direction == ParameterDirection::InOut - || (self.direction == ParameterDirection::Out - && self.ref_mode == RefMode::ByRefMut)) => - { - Err(TypeError::Unimplemented(into_inner(rust_type))) + Basic(_) | Enumeration(_) | Union(_) | Bitfield(_) | Custom(_) | Class(_) + | Interface(_) | Record(_) | Function(_) | Alias(_) => (), + CArray(_) | List(_) | SList(_) | PtrArray(_) => { + if self.direction == ParameterDirection::InOut { + return Err(TypeError::unimplemented(unwrap_rust_type(res))); + } } - Basic(_) => rust_type.map_any(|rust_type| rust_type.format_parameter(self.direction)), - - Alias(alias) => rust_type - .and_then(|rust_type| { - RustType::builder(self.env, alias.typ) - .direction(self.direction) - .nullable(self.nullable) - .ref_mode(self.ref_mode) - .scope(self.scope) - .try_from_glib(&self.try_from_glib) - .try_build_param() - .map_any(|_| rust_type) - }) - .map_any(|rust_type| rust_type.format_parameter(self.direction)), - - Enumeration(..) | Union(..) | Bitfield(..) => { - rust_type.map_any(|rust_type| rust_type.format_parameter(self.direction)) + Array(..) | FixedArray(..) | HashTable(..) => { + return Err(TypeError::unimplemented(unwrap_rust_type(res))); } + } - Record(..) => { - if self.direction == ParameterDirection::InOut { - Err(TypeError::Unimplemented(into_inner(rust_type))) - } else { - rust_type - } + if let Ok(ref r) = res { + if ConversionType::of(self.env, self.type_id) == ConversionType::Unknown { + return Err(TypeError::UnknownConversion(r.clone())); } + if self.direction == ParameterDirection::InOut && self.ref_mode.is_none() { + return Err(TypeError::Ignored(r.clone())); + } + } - Class(..) | Interface(..) => match self.direction { - ParameterDirection::In | ParameterDirection::Out | ParameterDirection::Return => { - rust_type + res.map_any(|mut r| { + let ref_ = self.ref_mode.to_string(); + match self + .try_from_glib + .or_type_defaults(self.env, self.type_id) + .borrow() + { + TryFromGlib::Option => { + if self.direction == ParameterDirection::In && !self.is_for_callback { + r.inner = format!("impl Into>"); + } else { + r.inner = format!("Option<{r}>"); + } } - _ => Err(TypeError::Unimplemented(into_inner(rust_type))), - }, - - List(..) | SList(..) => match self.direction { - ParameterDirection::In | ParameterDirection::Return => rust_type, - _ => Err(TypeError::Unimplemented(into_inner(rust_type))), - }, - CArray(..) | PtrArray(..) => match self.direction { - ParameterDirection::In | ParameterDirection::Out | ParameterDirection::Return => { - rust_type + TryFromGlib::OptionMandatory => { + if self.direction == ParameterDirection::Return { + r.inner = r.inner.to_string(); + } else { + r.inner = format!("{ref_}{r}"); + } } - _ => Err(TypeError::Unimplemented(into_inner(rust_type))), - }, - Function(ref func) if func.name == "AsyncReadyCallback" => { - Ok("AsyncReadyCallback".into()) + TryFromGlib::Result { ok_type, err_type } => { + if self.direction == ParameterDirection::In && !self.is_for_callback { + r.used_types.push(r.inner.to_string()); + r.inner = format!("impl Into<{}>", r.inner); + } else { + r.used_types.retain(|t| *t != r.inner); + r.used_types + .extend([ok_type, err_type].iter().map(ToString::to_string)); + r.inner = format!("Result<{ok_type}, {err_type}>"); + } + } + TryFromGlib::ResultInfallible { ok_type } => { + r.used_types.push(ok_type.to_string()); + r.inner = ok_type.to_string(); + } + _ if r.nullable_as_option && *self.nullable => { + r.inner = if self.direction == ParameterDirection::Return { + if self.is_for_callback && is_gstring(&r.inner) { + "Option".to_string() + } else { + format!("Option<{r}>") + } + } else if self.is_for_callback && is_gstring(&r.inner) { + "Option<&str>".to_string() + } else { + format!("Option<{ref_}{r}>") + }; + } + _ if self.is_for_callback && is_gstring(&r.inner) => { + r.inner = if self.direction == ParameterDirection::Return { + "String".to_string() + } else { + "&str".to_string() + }; + } + _ => r.inner = format!("{ref_}{r}"), } - Function(_) => rust_type, - Custom(..) => rust_type.map(|rust_type| rust_type.format_parameter(self.direction)), - _ => Err(TypeError::Unimplemented(type_.get_name())), - } + + r + }) } } diff --git a/src/analysis/trampolines.rs b/src/analysis/trampolines.rs index af7b1fb59..1df019f12 100644 --- a/src/analysis/trampolines.rs +++ b/src/analysis/trampolines.rs @@ -4,8 +4,7 @@ use super::{ bounds::{BoundType, Bounds}, conversion_type::ConversionType, ffi_type::used_ffi_type, - ref_mode::RefMode, - rust_type::RustType, + rust_type::{RustType, TypeError}, trampoline_parameters::{self, Parameters}, }; use crate::{ @@ -80,9 +79,7 @@ pub fn analyze( let mut bounds: Bounds = Default::default(); if in_trait || fundamental_type { - let type_name = RustType::builder(env, type_tid) - .ref_mode(RefMode::ByRefFake) - .try_build(); + let type_name = RustType::builder(env, type_tid).try_build(); if fundamental_type { bounds.add_parameter( "this", @@ -124,32 +121,12 @@ pub fn analyze( trampoline_parameters::analyze(env, &signal.parameters, type_tid, configured_signals, None) }; - if in_trait || fundamental_type { - let type_name = RustType::builder(env, type_tid) - .ref_mode(RefMode::ByRefFake) - .try_build(); - if fundamental_type { - bounds.add_parameter( - "this", - &type_name.into_string(), - BoundType::AsRef(None), - false, - ); - } else { - bounds.add_parameter( - "this", - &type_name.into_string(), - BoundType::IsA(None), - false, - ); - } - } - for par in ¶meters.rust_parameters { if let Ok(rust_type) = RustType::builder(env, par.typ) .direction(par.direction) .try_from_glib(&par.try_from_glib) - .try_build() + .for_callback(true) + .try_build_param() { used_types.extend(rust_type.into_used_types()); } @@ -165,7 +142,8 @@ pub fn analyze( if signal.ret.typ != Default::default() { if let Ok(rust_type) = RustType::builder(env, signal.ret.typ) .direction(library::ParameterDirection::Out) - .try_build() + .for_callback(true) + .try_build_param() { // No GString used_types.extend(rust_type.into_used_types()); @@ -235,7 +213,6 @@ fn closure_errors(env: &Env, signal: &library::Signal) -> Vec { } pub fn type_error(env: &Env, par: &library::Parameter) -> Option<&'static str> { - use super::rust_type::TypeError::*; if par.direction == library::ParameterDirection::Out { Some("Out") } else if par.direction == library::ParameterDirection::InOut { @@ -245,11 +222,11 @@ pub fn type_error(env: &Env, par: &library::Parameter) -> Option<&'static str> { } else if ConversionType::of(env, par.typ) == ConversionType::Unknown { Some("Unknown conversion") } else { - match RustType::try_new(env, par.typ) { - Err(Ignored(_)) => Some("Ignored"), - Err(Mismatch(_)) => Some("Mismatch"), - Err(Unimplemented(_)) => Some("Unimplemented"), - Ok(_) => None, - } + RustType::builder(env, par.typ) + .direction(par.direction) + .try_build_param() + .as_ref() + .map_err(TypeError::message) + .err() } } diff --git a/src/codegen/bound.rs b/src/codegen/bound.rs index 7a5d998aa..7e28ffe3a 100644 --- a/src/codegen/bound.rs +++ b/src/codegen/bound.rs @@ -21,7 +21,7 @@ impl Bound { nullable: Nullable, r#async: bool, ) -> String { - let ref_str = ref_mode.for_rust_type(); + let ref_str = ref_mode.to_string(); // Generate `impl Trait` if this bound does not have an alias let trait_bound = match self.type_parameter_reference() { diff --git a/src/codegen/child_properties.rs b/src/codegen/child_properties.rs index 4163bb814..38f2dd927 100644 --- a/src/codegen/child_properties.rs +++ b/src/codegen/child_properties.rs @@ -120,18 +120,11 @@ fn declaration(env: &Env, prop: &ChildProperty, is_get: bool) -> String { } fn body(env: &Env, prop: &ChildProperty, in_trait: bool, is_get: bool) -> Chunk { - let mut builder = property_body::Builder::new_for_child_property(env); - builder + property_body::Builder::new_for_child_property(env) .name(&prop.name) .in_trait(in_trait) .var_name(&prop.prop_name) - .is_get(is_get); - - if let Ok(type_) = RustType::try_new(env, prop.typ) { - builder.type_(type_.as_str()); - } else { - builder.type_("/*Unknown type*/"); - } - - builder.generate() + .for_get(is_get) + .type_(&RustType::try_new(env, prop.typ).into_string()) + .generate() } diff --git a/src/codegen/function.rs b/src/codegen/function.rs index f0195a68b..fab500b2f 100644 --- a/src/codegen/function.rs +++ b/src/codegen/function.rs @@ -407,8 +407,6 @@ pub fn body_chunk_futures( ) -> StdResult { use std::fmt::Write; - use crate::analysis::ref_mode::RefMode; - let async_future = analysis.async_future.as_ref().unwrap(); let mut body = String::new(); @@ -447,7 +445,7 @@ pub fn body_chunk_futures( )?; } else if is_str { writeln!(body, "let {} = String::from({});", par.name, par.name)?; - } else if c_par.ref_mode != RefMode::None { + } else if !c_par.ref_mode.is_none() { writeln!(body, "let {} = {}.clone();", par.name, par.name)?; } } @@ -487,7 +485,7 @@ pub fn body_chunk_futures( "\t\t{}.as_ref().map(::std::borrow::Borrow::borrow),", par.name )?; - } else if c_par.ref_mode != RefMode::None { + } else if !c_par.ref_mode.is_none() { writeln!(body, "\t\t&{},", par.name)?; } else { writeln!(body, "\t\t{},", par.name)?; diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 9adf2c1b0..a3752cf41 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -32,7 +32,6 @@ mod properties; mod property_body; mod record; mod records; -mod ref_mode; mod return_value; mod signal; mod signal_body; diff --git a/src/codegen/object.rs b/src/codegen/object.rs index 6f64db1de..3f7bf3b0d 100644 --- a/src/codegen/object.rs +++ b/src/codegen/object.rs @@ -371,7 +371,7 @@ fn generate_builder(w: &mut dyn Write, env: &Env, analysis: &analysis::object::I let param_type = RustType::builder(env, property.typ) .direction(direction) .ref_mode(property.set_in_ref_mode) - .try_build(); + .try_build_param(); let comment_prefix = if param_type.is_err() { "//" } else { "" }; let mut param_type_str = param_type.into_string(); let (param_type_override, bounds, conversion) = match param_type_str.as_str() { @@ -382,7 +382,7 @@ fn generate_builder(w: &mut dyn Write, env: &Env, analysis: &analysis::object::I ), "&[&str]" => ( Some(format!("impl Into<{glib_crate_name}::StrV>")), - String::from(""), + String::new(), ".into()", ), _ if !property.bounds.is_empty() => { diff --git a/src/codegen/parameter.rs b/src/codegen/parameter.rs index efdada344..0e4c50923 100644 --- a/src/codegen/parameter.rs +++ b/src/codegen/parameter.rs @@ -1,7 +1,6 @@ use crate::{ analysis::{ - bounds::Bounds, conversion_type::ConversionType, function_parameters::CParameter, - ref_mode::RefMode, rust_type::RustType, + bounds::Bounds, function_parameters::CParameter, ref_mode::RefMode, rust_type::RustType, }, env::Env, traits::*, @@ -19,26 +18,19 @@ impl ToParameter for CParameter { self.ref_mode }; if self.instance_parameter { - format!("{}self", ref_mode.for_rust_type()) + format!("{ref_mode}self") } else { - let type_str = match bounds.get_parameter_bound(&self.name) { - Some(bound) => { - bound.full_type_parameter_reference(ref_mode, self.nullable, r#async) - } - None => { - let type_name = RustType::builder(env, self.typ) - .direction(self.direction) - .nullable(self.nullable) - .ref_mode(ref_mode) - .scope(self.scope) - .try_from_glib(&self.try_from_glib) - .try_build_param() - .into_string(); - match ConversionType::of(env, self.typ) { - ConversionType::Unknown => format!("/*Unknown conversion*/{type_name}"), - _ => type_name, - } - } + let type_str = if let Some(bound) = bounds.get_parameter_bound(&self.name) { + bound.full_type_parameter_reference(ref_mode, self.nullable, r#async) + } else { + RustType::builder(env, self.typ) + .direction(self.direction) + .nullable(self.nullable) + .ref_mode(ref_mode) + .scope(self.scope) + .try_from_glib(&self.try_from_glib) + .try_build_param() + .into_string() }; format!("{}: {}", self.name, type_str) } diff --git a/src/codegen/properties.rs b/src/codegen/properties.rs index fee68a7d9..163a1ed1d 100644 --- a/src/codegen/properties.rs +++ b/src/codegen/properties.rs @@ -117,18 +117,11 @@ fn declaration(env: &Env, prop: &Property) -> String { } fn body(env: &Env, prop: &Property, in_trait: bool) -> Chunk { - let mut builder = property_body::Builder::new(env); - builder + property_body::Builder::new(env) .name(&prop.name) .in_trait(in_trait) .var_name(&prop.var_name) - .is_get(prop.is_get); - - if let Ok(type_) = RustType::try_new(env, prop.typ) { - builder.type_(type_.as_str()); - } else { - builder.type_("/*Unknown type*/"); - } - - builder.generate() + .for_get(prop.is_get) + .type_(&RustType::try_new(env, prop.typ).into_string()) + .generate() } diff --git a/src/codegen/property_body.rs b/src/codegen/property_body.rs index 3a58741bd..fdd709bc0 100644 --- a/src/codegen/property_body.rs +++ b/src/codegen/property_body.rs @@ -1,67 +1,59 @@ use crate::{chunk::Chunk, env::Env, nameutil::use_gtk_type}; +#[derive(Debug, Default)] pub struct Builder<'a> { name: String, in_trait: bool, var_name: String, - is_get: bool, + for_get: bool, is_child_property: bool, type_: String, - env: &'a Env, + env: Option<&'a Env>, } impl<'a> Builder<'a> { pub fn new(env: &'a Env) -> Self { Self { - env, - name: Default::default(), - in_trait: Default::default(), - var_name: Default::default(), - is_get: Default::default(), - is_child_property: Default::default(), - type_: Default::default(), + env: Some(env), + ..Default::default() } } pub fn new_for_child_property(env: &'a Env) -> Self { Self { is_child_property: true, - env, - name: Default::default(), - in_trait: Default::default(), - var_name: Default::default(), - is_get: Default::default(), - type_: Default::default(), + env: Some(env), + ..Default::default() } } - pub fn name(&mut self, name: &str) -> &mut Self { + pub fn name(mut self, name: &str) -> Self { self.name = name.into(); self } - pub fn in_trait(&mut self, value: bool) -> &mut Self { + pub fn in_trait(mut self, value: bool) -> Self { self.in_trait = value; self } - pub fn var_name(&mut self, name: &str) -> &mut Self { + pub fn var_name(mut self, name: &str) -> Self { self.var_name = name.into(); self } - pub fn is_get(&mut self, value: bool) -> &mut Self { - self.is_get = value; + pub fn for_get(mut self, value: bool) -> Self { + self.for_get = value; self } - pub fn type_(&mut self, type_: &str) -> &mut Self { + pub fn type_(mut self, type_: &str) -> Self { self.type_ = type_.into(); self } pub fn generate(&self) -> Chunk { - let chunks = if self.is_get { + let chunks = if self.for_get { self.chunks_for_get() } else { self.chunks_for_set() @@ -79,7 +71,7 @@ impl<'a> Builder<'a> { vec![Chunk::Custom(format!( "{}::child_property({}, &item.clone().upcast(),\"{}\")", - use_gtk_type(self.env, "prelude::ContainerExtManual"), + use_gtk_type(self.env.unwrap(), "prelude::ContainerExtManual"), self_, self.name, ))] @@ -107,7 +99,7 @@ impl<'a> Builder<'a> { vec![Chunk::Custom(format!( "{}::child_set_property({}, &item.clone().upcast(),\"{}\", &{})", - use_gtk_type(self.env, "prelude::ContainerExtManual"), + use_gtk_type(self.env.unwrap(), "prelude::ContainerExtManual"), self_, self.name, self.var_name diff --git a/src/codegen/ref_mode.rs b/src/codegen/ref_mode.rs deleted file mode 100644 index 17e79d302..000000000 --- a/src/codegen/ref_mode.rs +++ /dev/null @@ -1,11 +0,0 @@ -use crate::analysis::ref_mode::RefMode; - -impl RefMode { - pub(crate) fn for_rust_type(self) -> &'static str { - match self { - RefMode::None | RefMode::ByRefFake => "", - RefMode::ByRef | RefMode::ByRefImmut | RefMode::ByRefConst => "&", - RefMode::ByRefMut => "&mut ", - } - } -} diff --git a/src/codegen/return_value.rs b/src/codegen/return_value.rs index 42f1112ee..9092c4898 100644 --- a/src/codegen/return_value.rs +++ b/src/codegen/return_value.rs @@ -2,12 +2,11 @@ use std::cmp; use crate::{ analysis::{ - self, conversion_type::ConversionType, namespaces, out_parameters::Mode, - rust_type::RustType, try_from_glib::TryFromGlib, + self, namespaces, out_parameters::Mode, rust_type::RustType, try_from_glib::TryFromGlib, }, env::Env, - library::{self, ParameterDirection, TypeId}, - nameutil::{is_gstring, mangle_keywords, use_glib_type}, + library::{self, TypeId}, + nameutil::{mangle_keywords, use_glib_type}, traits::*, }; @@ -27,26 +26,16 @@ impl ToReturnValue for library::Parameter { try_from_glib: &TryFromGlib, is_trampoline: bool, ) -> Option { - let mut name = RustType::builder(env, self.typ) + let name = RustType::builder(env, self.typ) .direction(self.direction) .nullable(self.nullable) .scope(self.scope) .try_from_glib(try_from_glib) + .for_callback(is_trampoline) .try_build_param() .into_string(); - if is_trampoline - && self.direction == library::ParameterDirection::Return - && is_gstring(&name) - { - name = "String".to_owned(); - } - let type_str = match ConversionType::of(env, self.typ) { - ConversionType::Unknown => format!("/*Unknown conversion*/{name}"), - // TODO: records as in gtk_container_get_path_for_child - _ => name, - }; - Some(type_str) + Some(name) } } @@ -249,24 +238,17 @@ pub fn out_parameters_as_return(env: &Env, analysis: &analysis::functions::Info) if pos > skip { return_str.push_str(", "); } - let s = out_parameter_as_return(out, env); - return_str.push_str(&s); + // TODO: upcasts? + return_str.push_str( + &RustType::builder(env, out.lib_par.typ) + .direction(out.lib_par.direction) + .nullable(out.lib_par.nullable) + .scope(out.lib_par.scope) + .try_from_glib(&out.try_from_glib) + .try_build_param() + .into_string(), + ); } return_str.push_str(&suffix); return_str } - -fn out_parameter_as_return(out: &analysis::Parameter, env: &Env) -> String { - // TODO: upcasts? - let name = RustType::builder(env, out.lib_par.typ) - .direction(ParameterDirection::Return) - .nullable(out.lib_par.nullable) - .scope(out.lib_par.scope) - .try_from_glib(&out.try_from_glib) - .try_build_param() - .into_string(); - match ConversionType::of(env, out.lib_par.typ) { - ConversionType::Unknown => format!("/*Unknown conversion*/{name}"), - _ => name, - } -} diff --git a/src/codegen/sys/ffi_type.rs b/src/codegen/sys/ffi_type.rs index 58168f941..742a7f7ae 100644 --- a/src/codegen/sys/ffi_type.rs +++ b/src/codegen/sys/ffi_type.rs @@ -108,14 +108,14 @@ fn ffi_inner(env: &Env, tid: library::TypeId, mut inner: String) -> Result { match inner.as_str() { "void" => "c_void", // TODO: try use time:Tm - "tm" => return Err(TypeError::Unimplemented(inner)), + "tm" => return Err(TypeError::unimplemented(inner)), _ => &*inner, } } IntPtr => "intptr_t", UIntPtr => "uintptr_t", Bool => "bool", - Unsupported => return Err(TypeError::Unimplemented(inner)), + Unsupported => return Err(TypeError::unimplemented(inner)), VarArgs => panic!("Should not reach here"), }; Ok(inner.into()) @@ -130,7 +130,7 @@ fn ffi_inner(env: &Env, tid: library::TypeId, mut inner: String) -> Result { typ.get_name() ); warn!("{}", msg); - return Err(TypeError::Mismatch(msg)); + return Err(TypeError::mismatch(msg)); } } else { warn!("Type `{}` missing c_type", typ.get_name()); @@ -173,7 +173,7 @@ fn ffi_inner(env: &Env, tid: library::TypeId, mut inner: String) -> Result { env.library.type_(tid).get_name() ); warn!("{}", msg); - Err(TypeError::Mismatch(msg)) + Err(TypeError::mismatch(msg)) } } else { fix_name(env, tid, &inner) @@ -185,7 +185,7 @@ fn ffi_inner(env: &Env, tid: library::TypeId, mut inner: String) -> Result { inner ); warn!("{}", msg); - Err(TypeError::Mismatch(msg)) + Err(TypeError::mismatch(msg)) } } }; @@ -227,7 +227,7 @@ fn fix_name(env: &Env, type_id: library::TypeId, name: &str) -> Result { .type_status_sys(&type_id.full_name(&env.library)) .ignored() { - Err(TypeError::Ignored(name_with_prefix)) + Err(TypeError::ignored(name_with_prefix)) } else { Ok(name_with_prefix.into()) } diff --git a/src/codegen/sys/fields.rs b/src/codegen/sys/fields.rs index 0927ae7f5..9608132e3 100644 --- a/src/codegen/sys/fields.rs +++ b/src/codegen/sys/fields.rs @@ -140,7 +140,7 @@ fn analyze_fields( fn field_ffi_type(env: &Env, field: &Field) -> Result { if field.is_incomplete(&env.library) { - return Err(TypeError::Ignored(format!( + return Err(TypeError::ignored(format!( "field {} has incomplete type", &field.name ))); @@ -151,12 +151,12 @@ fn field_ffi_type(env: &Env, field: &Field) -> Result { let (failure, signature) = function_signature(env, func, true); let signature = format!("Option"); if failure { - Err(TypeError::Unimplemented(signature)) + Err(TypeError::unimplemented(signature)) } else { Ok(signature.into()) } } else { - Err(TypeError::Ignored(format!( + Err(TypeError::ignored(format!( "field {} has empty c:type", &field.name ))) diff --git a/src/codegen/trampoline.rs b/src/codegen/trampoline.rs index b339ef074..72488b47e 100644 --- a/src/codegen/trampoline.rs +++ b/src/codegen/trampoline.rs @@ -104,7 +104,7 @@ fn func_parameters( for (pos, par) in analysis.parameters.rust_parameters.iter().enumerate() { if pos == 0 { if let Some(replace_self_bound) = &replace_self_bound { - param_str.push_str(par.ref_mode.for_rust_type()); + param_str.push_str(&par.ref_mode.to_string()); param_str.push_str(replace_self_bound.as_ref()); continue; } @@ -115,8 +115,7 @@ fn func_parameters( } } - let s = func_parameter(env, par, &analysis.bounds); - param_str.push_str(&s); + param_str.push_str(&func_parameter(env, par, &analysis.bounds)); } param_str @@ -139,6 +138,7 @@ fn func_parameter(env: &Env, par: &RustParameter, bounds: &Bounds) -> String { .direction(par.direction) .nullable(par.nullable) .ref_mode(ref_mode) + .for_callback(true) .try_build_param() .into_string(), } From 45aad8f6aff98d4a6d93d8741068310291299951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Sun, 1 Dec 2024 17:07:45 +0100 Subject: [PATCH 3/4] Generalize Bound & to_glib_extra in more contexts This commit paves the way for the introduction of additional bounds in more contexts by generalizing the use of `Bound` and `to_glib_extra`. `to_glib_extra`s allow prepending a processing before calling the actual conversion to the glib type. For instance, this can be used to convert a value into an `Option` when an `impl Into>` argument is used. This commit adds support for `to_glib_extra` for callback arguments and for property setters. Properties were using a specific `PropertyBound` type which added code duplication and couldn't take advantage of the regular `Bound` which can take advantage of `to_glib_extra`. --- src/analysis/bounds.rs | 233 +++++++++++++++++----------- src/analysis/child_properties.rs | 21 +-- src/analysis/class_builder.rs | 1 - src/analysis/function_parameters.rs | 35 ++--- src/analysis/functions.rs | 49 ++++-- src/analysis/properties.rs | 20 ++- src/analysis/trampolines.rs | 14 +- src/chunk/chunk.rs | 2 +- src/codegen/bound.rs | 22 +-- src/codegen/function.rs | 13 +- src/codegen/object.rs | 2 +- src/codegen/properties.rs | 12 +- src/codegen/property_body.rs | 20 ++- src/codegen/translate_to_glib.rs | 5 +- src/writer/to_code.rs | 2 +- 15 files changed, 259 insertions(+), 192 deletions(-) diff --git a/src/analysis/bounds.rs b/src/analysis/bounds.rs index 5e746a7d3..bbebfb550 100644 --- a/src/analysis/bounds.rs +++ b/src/analysis/bounds.rs @@ -12,30 +12,41 @@ use crate::{ config, consts::TYPE_PARAMETERS_START, env::Env, - library::{Basic, Class, Concurrency, Function, ParameterDirection, Type, TypeId}, + library::{Basic, Class, Concurrency, Function, Nullable, ParameterDirection, Type, TypeId}, traits::*, }; -#[derive(Clone, Eq, Debug, PartialEq)] +#[derive(Clone, Copy, Eq, Debug, PartialEq)] pub enum BoundType { NoWrapper, - // lifetime - IsA(Option), - // lifetime <- shouldn't be used but just in case... - AsRef(Option), + IsA, + AsRef, } impl BoundType { pub fn need_isa(&self) -> bool { - matches!(*self, Self::IsA(_)) + matches!(*self, Self::IsA) } - // TODO: This is just a heuristic for now, based on what we do in codegen! - // Theoretically the surrounding function should determine whether it needs to - // reuse an alias (ie. to use in `call_func::`) or not. - // In the latter case an `impl` is generated instead of a type name/alias. - pub fn has_alias(&self) -> bool { - matches!(*self, Self::NoWrapper) + pub fn get_to_glib_extra( + &self, + nullable: bool, + instance_parameter: bool, + move_: bool, + ) -> String { + use BoundType::*; + match *self { + AsRef if move_ && nullable => ".map(|p| p.as_ref().clone().upcast())", + AsRef if nullable => ".as_ref().map(|p| p.as_ref())", + AsRef if move_ => ".upcast()", + AsRef => ".as_ref()", + IsA if move_ && nullable => ".map(|p| p.upcast())", + IsA if nullable && !instance_parameter => ".map(|p| p.as_ref())", + IsA if move_ => ".upcast()", + IsA => ".as_ref()", + _ => "", + } + .to_string() } } @@ -43,7 +54,7 @@ impl BoundType { pub struct Bound { pub bound_type: BoundType, pub parameter_name: String, - /// Bound does not have an alias when `param: impl type_str` is used + pub lt: Option, pub alias: Option, pub type_str: String, pub callback_modified: bool, @@ -110,8 +121,7 @@ impl Bounds { if !par.instance_parameter && par.direction != ParameterDirection::Out { if let Some(bound_type) = Bounds::type_for(env, par.typ) { - ret = Some(Bounds::get_to_glib_extra( - &bound_type, + ret = Some(bound_type.get_to_glib_extra( *par.nullable, par.instance_parameter, par.move_, @@ -204,57 +214,90 @@ impl Bounds { } } else if par.instance_parameter { if let Some(bound_type) = Bounds::type_for(env, par.typ) { - ret = Some(Bounds::get_to_glib_extra( - &bound_type, - *par.nullable, - true, - par.move_, - )); + ret = Some(bound_type.get_to_glib_extra(*par.nullable, true, par.move_)); } } (ret, callback_info) } + pub fn add_for_property_setter( + &mut self, + env: &Env, + type_id: TypeId, + name: &str, + ref_mode: RefMode, + nullable: Nullable, + ) -> bool { + let type_str = RustType::builder(env, type_id) + .nullable(nullable) + .direction(ParameterDirection::In) + .ref_mode(ref_mode) + .try_build() + .into_string(); + + let Some(bound_type) = Bounds::type_for(env, type_id) else { + return false; + }; + + let alias = match bound_type { + BoundType::NoWrapper | BoundType::IsA => { + Some(self.unused.pop_front().expect("No free type aliases!")) + } + _ => None, + }; + + self.push(Bound { + bound_type, + parameter_name: name.to_string(), + lt: None, + alias, + type_str, + callback_modified: false, + }); + + true + } + pub fn type_for(env: &Env, type_id: TypeId) -> Option { use self::BoundType::*; match env.library.type_(type_id) { - Type::Basic(Basic::Filename | Basic::OsString) => Some(AsRef(None)), + Type::Basic(Basic::Filename | Basic::OsString) => Some(AsRef), Type::Class(Class { is_fundamental: true, .. - }) => Some(AsRef(None)), + }) => Some(AsRef), Type::Class(Class { final_type: true, .. }) => None, Type::Class(Class { final_type: false, .. - }) => Some(IsA(None)), - Type::Interface(..) => Some(IsA(None)), + }) => Some(IsA), + Type::Interface(..) => Some(IsA), Type::List(_) | Type::SList(_) | Type::CArray(_) => None, Type::Function(_) => Some(NoWrapper), _ => None, } } - pub fn get_to_glib_extra( - bound_type: &BoundType, - nullable: bool, + pub fn get_to_glib_extra_for( + env: &Env, + type_id: TypeId, + ref_mode: RefMode, + nullable: Nullable, instance_parameter: bool, - move_: bool, - ) -> String { - use self::BoundType::*; - match bound_type { - AsRef(_) if move_ && nullable => ".map(|p| p.as_ref().clone().upcast())".to_owned(), - AsRef(_) if nullable => ".as_ref().map(|p| p.as_ref())".to_owned(), - AsRef(_) if move_ => ".upcast()".to_owned(), - AsRef(_) => ".as_ref()".to_owned(), - IsA(_) if move_ && nullable => ".map(|p| p.upcast())".to_owned(), - IsA(_) if nullable && !instance_parameter => ".map(|p| p.as_ref())".to_owned(), - IsA(_) if move_ => ".upcast()".to_owned(), - IsA(_) => ".as_ref()".to_owned(), - _ => String::new(), + ) -> Option { + let res = Self::type_for(env, type_id)?.get_to_glib_extra( + *nullable, + instance_parameter, + ref_mode.is_none(), + ); + + if res.is_empty() { + return None; } + + Some(res) } pub fn add_parameter( @@ -270,18 +313,50 @@ impl Bounds { if self.used.iter().any(|n| n.parameter_name == name) { return; } - let alias = bound_type - .has_alias() - .then(|| self.unused.pop_front().expect("No free type aliases!")); + let alias = match bound_type { + BoundType::NoWrapper => { + // TODO: This is just a heuristic for now, based on what we do in codegen! + // Theoretically the surrounding function should determine whether it needs to + // reuse an alias (ie. to use in `call_func::`) or not. + // In the latter case an `impl` is generated instead of a type name/alias. + Some(self.unused.pop_front().expect("No free type aliases!")) + } + _ => None, + }; self.used.push(Bound { bound_type, parameter_name: name.to_owned(), + lt: None, alias, type_str: type_str.to_owned(), callback_modified: false, }); } + pub fn push(&mut self, bound: Bound) { + if let Some(lt) = bound.lt { + if !self.lifetimes.contains(<) { + self.lifetimes.push(lt); + self.lifetimes.sort(); + } + } + + if let Some(alias) = bound.alias { + if self + .used + .iter() + .any(|b| b.alias.map_or(false, |a| a == alias)) + { + panic!("Pushing a bound with an alias already in use"); + } + + self.unused.retain(|u| *u != alias); + } + + self.used.push(bound); + self.used.sort_by_key(|u| u.alias); + } + pub fn get_parameter_bound(&self, name: &str) -> Option<&Bound> { self.iter().find(move |n| n.parameter_name == name) } @@ -292,8 +367,8 @@ impl Bounds { for used in &self.used { match used.bound_type { NoWrapper => (), - IsA(_) => imports.add("glib::prelude::*"), - AsRef(_) => imports.add_used_type(&used.type_str), + IsA => imports.add("glib::prelude::*"), + AsRef => imports.add_used_type(&used.type_str), } } } @@ -311,28 +386,6 @@ impl Bounds { } } -#[derive(Clone, Debug)] -pub struct PropertyBound { - pub alias: char, - pub type_str: String, -} - -impl PropertyBound { - pub fn get(env: &Env, type_id: TypeId) -> Option { - let type_ = env.type_(type_id); - if type_.is_final_type() { - return None; - } - Some(Self { - alias: TYPE_PARAMETERS_START, - type_str: RustType::builder(env, type_id) - .ref_mode(RefMode::ByRefFake) - .try_build() - .into_string(), - }) - } -} - fn find_out_parameters( env: &Env, function: &Function, @@ -399,24 +452,24 @@ mod tests { #[test] fn get_new_all() { let mut bounds: Bounds = Default::default(); - let typ = BoundType::IsA(None); - bounds.add_parameter("a", "", typ.clone(), false); + let typ = BoundType::IsA; + bounds.add_parameter("a", "", typ, false); assert_eq!(bounds.iter().len(), 1); // Don't add second time - bounds.add_parameter("a", "", typ.clone(), false); + bounds.add_parameter("a", "", typ, 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); + bounds.add_parameter("b", "", typ, false); + bounds.add_parameter("c", "", typ, false); + bounds.add_parameter("d", "", typ, false); + bounds.add_parameter("e", "", typ, false); + bounds.add_parameter("f", "", typ, false); + bounds.add_parameter("g", "", typ, false); + bounds.add_parameter("h", "", typ, false); assert_eq!(bounds.iter().len(), 8); - bounds.add_parameter("h", "", typ.clone(), false); + bounds.add_parameter("h", "", typ, false); assert_eq!(bounds.iter().len(), 8); - bounds.add_parameter("i", "", typ.clone(), false); - bounds.add_parameter("j", "", typ.clone(), false); + bounds.add_parameter("i", "", typ, false); + bounds.add_parameter("j", "", typ, false); bounds.add_parameter("k", "", typ, false); } @@ -427,7 +480,7 @@ mod tests { 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); + bounds.add_parameter(c.to_string().as_str(), "", typ, false); } } @@ -435,8 +488,8 @@ mod tests { fn get_parameter_bound() { let mut bounds: Bounds = Default::default(); let typ = BoundType::NoWrapper; - bounds.add_parameter("a", "", typ.clone(), false); - bounds.add_parameter("b", "", typ.clone(), false); + bounds.add_parameter("a", "", typ, false); + bounds.add_parameter("b", "", typ, false); let bound = bounds.get_parameter_bound("a").unwrap(); // `NoWrapper `bounds are expected to have an alias: assert_eq!(bound.alias, Some('P')); @@ -450,17 +503,17 @@ mod tests { #[test] fn impl_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); + let typ = BoundType::IsA; + bounds.add_parameter("a", "", typ, false); + bounds.add_parameter("b", "", typ, false); let bound = bounds.get_parameter_bound("a").unwrap(); // `IsA` is simplified to an inline `foo: impl IsA` and // lacks an alias/type-parameter: assert_eq!(bound.alias, None); assert_eq!(bound.bound_type, typ); - let typ = BoundType::AsRef(None); - bounds.add_parameter("c", "", typ.clone(), false); + let typ = BoundType::AsRef; + bounds.add_parameter("c", "", typ, false); let bound = bounds.get_parameter_bound("c").unwrap(); // Same `impl AsRef` simplification as `IsA`: assert_eq!(bound.alias, None); diff --git a/src/analysis/child_properties.rs b/src/analysis/child_properties.rs index 9c6ef572c..e19eb80da 100644 --- a/src/analysis/child_properties.rs +++ b/src/analysis/child_properties.rs @@ -1,12 +1,7 @@ use log::error; use crate::{ - analysis::{ - bounds::{Bound, Bounds}, - imports::Imports, - ref_mode::RefMode, - rust_type::RustType, - }, + analysis::{bounds::Bounds, imports::Imports, ref_mode::RefMode, rust_type::RustType}, config, consts::TYPE_PARAMETERS_START, env::Env, @@ -120,19 +115,7 @@ fn analyze_property( let mut bounds_str = String::new(); let dir = ParameterDirection::In; - let set_params = if let Some(bound) = Bounds::type_for(env, typ) { - let r_type = RustType::builder(env, typ) - .ref_mode(RefMode::ByRefFake) - .try_build() - .into_string(); - - let _bound = Bound { - bound_type: bound, - parameter_name: TYPE_PARAMETERS_START.to_string(), - alias: Some(TYPE_PARAMETERS_START.to_owned()), - type_str: r_type, - callback_modified: false, - }; + let set_params = if Bounds::type_for(env, typ).is_some() { // TODO: bounds_str push?!?! bounds_str.push_str("TODO"); format!("{prop_name}: {TYPE_PARAMETERS_START}") diff --git a/src/analysis/class_builder.rs b/src/analysis/class_builder.rs index 19380445e..a22601648 100644 --- a/src/analysis/class_builder.rs +++ b/src/analysis/class_builder.rs @@ -139,7 +139,6 @@ fn analyze_property( nullable: library::Nullable(false), // no Options for builder setters here get_out_ref_mode, set_in_ref_mode, - set_bound: None, bounds, version: prop_version, deprecated_version: prop.deprecated_version, diff --git a/src/analysis/function_parameters.rs b/src/analysis/function_parameters.rs index 88fcc1397..95a97b80f 100644 --- a/src/analysis/function_parameters.rs +++ b/src/analysis/function_parameters.rs @@ -84,7 +84,7 @@ pub enum TransformationType { ToGlibScalar { name: String, nullable: library::Nullable, - needs_into: bool, + to_glib_extra: String, }, ToGlibPointer { name: String, @@ -127,8 +127,12 @@ impl TransformationType { } pub fn set_to_glib_extra(&mut self, to_glib_extra_: &str) { - if let Self::ToGlibPointer { to_glib_extra, .. } = self { - *to_glib_extra = to_glib_extra_.to_owned(); + match self { + Self::ToGlibScalar { to_glib_extra, .. } + | Self::ToGlibPointer { to_glib_extra, .. } => { + *to_glib_extra = to_glib_extra_.to_owned(); + } + _ => (), } } } @@ -287,8 +291,7 @@ pub fn analyze( let mut array_name = nameutil::mangle_keywords(&array_par.name); if let Some(bound_type) = Bounds::type_for(env, array_par.typ) { array_name = (array_name.into_owned() - + &Bounds::get_to_glib_extra( - &bound_type, + + &bound_type.get_to_glib_extra( *array_par.nullable, array_par.instance_parameter, move_, @@ -356,37 +359,31 @@ pub fn analyze( TransformationType::ToGlibScalar { name, nullable, - needs_into: false, + to_glib_extra: String::new(), } } } ConversionType::Scalar => TransformationType::ToGlibScalar { name, nullable, - needs_into: false, + to_glib_extra: String::new(), }, ConversionType::Option => { - let needs_into = match try_from_glib { - TryFromGlib::Option => par.direction == library::ParameterDirection::In, - TryFromGlib::OptionMandatory => false, - other => unreachable!("{:?} inconsistent / conversion type", other), - }; + let needs_into = par.direction == library::ParameterDirection::In + && matches!(try_from_glib, TryFromGlib::Option); TransformationType::ToGlibScalar { name, nullable: Nullable(false), - needs_into, + to_glib_extra: if needs_into { ".into()" } else { "" }.to_string(), } } ConversionType::Result { .. } => { - let needs_into = match try_from_glib { - TryFromGlib::Result { .. } => par.direction == library::ParameterDirection::In, - TryFromGlib::ResultInfallible { .. } => false, - other => unreachable!("{:?} inconsistent / conversion type", other), - }; + let needs_into = par.direction == library::ParameterDirection::In + && matches!(try_from_glib, TryFromGlib::Result { .. }); TransformationType::ToGlibScalar { name, nullable: Nullable(false), - needs_into, + to_glib_extra: if needs_into { ".into()" } else { "" }.to_string(), } } ConversionType::Pointer => TransformationType::ToGlibPointer { diff --git a/src/analysis/functions.rs b/src/analysis/functions.rs index 84a5bb7ca..ce7414b80 100644 --- a/src/analysis/functions.rs +++ b/src/analysis/functions.rs @@ -355,6 +355,7 @@ fn analyze_callbacks( None => &func.name, }; let mut destructors_to_update = Vec::new(); + let mut ind_c = 0; for pos in 0..parameters.c_parameters.len() { // If it is a user data parameter, we ignore it. if cross_user_data_check.values().any(|p| *p == pos) || user_data_indexes.contains(&pos) @@ -376,25 +377,33 @@ fn analyze_callbacks( used_types.extend(rust_type.into_used_types()); } let rust_type = env.library.type_(par.typ); - let callback_info = if !*par.nullable || !rust_type.is_function() { - let (to_glib_extra, callback_info) = bounds.add_for_parameter( - env, - func, - par, - false, - concurrency, - configured_functions, - ); - if let Some(to_glib_extra) = to_glib_extra { - if par.c_type != "GDestroyNotify" { - to_glib_extras.insert(pos, to_glib_extra); - } - } - callback_info + let (to_glib_extra, callback_info) = if !*par.nullable || !rust_type.is_function() { + bounds.add_for_parameter(env, func, par, false, concurrency, configured_functions) } else { - None + ( + Bounds::get_to_glib_extra_for( + env, + par.typ, + if par.move_ { + RefMode::None + } else { + par.ref_mode + }, + par.nullable, + par.instance_parameter, + ), + None, + ) }; + if let Some(to_glib_extra) = to_glib_extra { + if par.c_type != "GDestroyNotify" { + to_glib_extras.insert(ind_c, to_glib_extra); + } + } + + ind_c += 1; + if rust_type.is_function() { if par.c_type != "GDestroyNotify" { let callback_parameters_config = configured_functions.iter().find_map(|f| { @@ -536,6 +545,14 @@ fn analyze_callbacks( false, in_trait, ); + + for transformation in &mut parameters.transformations { + if let Some(to_glib_extra) = to_glib_extras.get(&transformation.ind_c) { + transformation + .transformation_type + .set_to_glib_extra(to_glib_extra); + } + } } else { warn_main!( type_tid, diff --git a/src/analysis/properties.rs b/src/analysis/properties.rs index 69cc1878d..2e25cbd6f 100644 --- a/src/analysis/properties.rs +++ b/src/analysis/properties.rs @@ -2,7 +2,7 @@ use log::warn; use crate::{ analysis::{ - bounds::{Bounds, PropertyBound}, + bounds::{Bound, Bounds}, imports::Imports, ref_mode::RefMode, rust_type::RustType, @@ -29,11 +29,16 @@ pub struct Property { pub get_out_ref_mode: RefMode, pub set_in_ref_mode: RefMode, pub bounds: Bounds, - pub set_bound: Option, pub version: Option, pub deprecated_version: Option, } +impl Property { + pub fn set_bound(&self) -> Option<&Bound> { + self.bounds.iter().next() + } +} + pub fn analyze( env: &Env, props: &[library::Property], @@ -267,7 +272,6 @@ fn analyze_property( nullable, get_out_ref_mode, set_in_ref_mode, - set_bound: None, bounds: Bounds::default(), version: getter_version, deprecated_version: prop.deprecated_version, @@ -287,8 +291,11 @@ fn analyze_property( if type_string.is_ok() { imports.add("glib::prelude::*"); } - let set_bound = PropertyBound::get(env, prop.typ); - if type_string.is_ok() && set_bound.is_some() { + let mut bounds = Bounds::default(); + let set_has_bound = + bounds.add_for_property_setter(env, prop.typ, &prop.name, set_in_ref_mode, nullable); + if type_string.is_ok() && set_has_bound { + // FIXME this might actually depend on the bound imports.add("glib::prelude::*"); if !*nullable { // TODO: support non-nullable setter if found any @@ -320,8 +327,7 @@ fn analyze_property( nullable, get_out_ref_mode, set_in_ref_mode, - set_bound, - bounds: Bounds::default(), + bounds, version: setter_version, deprecated_version: prop.deprecated_version, }) diff --git a/src/analysis/trampolines.rs b/src/analysis/trampolines.rs index 1df019f12..386f7567a 100644 --- a/src/analysis/trampolines.rs +++ b/src/analysis/trampolines.rs @@ -81,19 +81,9 @@ pub fn analyze( if in_trait || fundamental_type { let type_name = RustType::builder(env, type_tid).try_build(); if fundamental_type { - bounds.add_parameter( - "this", - &type_name.into_string(), - BoundType::AsRef(None), - false, - ); + bounds.add_parameter("this", &type_name.into_string(), BoundType::AsRef, false); } else { - bounds.add_parameter( - "this", - &type_name.into_string(), - BoundType::IsA(None), - false, - ); + bounds.add_parameter("this", &type_name.into_string(), BoundType::IsA, false); } } diff --git a/src/chunk/chunk.rs b/src/chunk/chunk.rs index a3225db3d..4919360df 100644 --- a/src/chunk/chunk.rs +++ b/src/chunk/chunk.rs @@ -67,10 +67,10 @@ pub enum Chunk { Name(String), ExternCFunc { name: String, + bounds: String, parameters: Vec, body: Box, return_value: Option, - bounds: String, }, Cast { name: String, diff --git a/src/codegen/bound.rs b/src/codegen/bound.rs index 7e28ffe3a..0cf41fa68 100644 --- a/src/codegen/bound.rs +++ b/src/codegen/bound.rs @@ -32,9 +32,9 @@ impl Bound { // Combining a ref mode and lifetime requires parentheses for disambiguation match self.bound_type { - BoundType::IsA(lifetime) => { + BoundType::IsA => { // TODO: This is fragile - let has_lifetime = r#async || lifetime.is_some(); + let has_lifetime = r#async || self.lt.is_some(); if !ref_str.is_empty() && has_lifetime { format!("({trait_bound})") @@ -48,14 +48,14 @@ impl Bound { }; match self.bound_type { - BoundType::IsA(_) if *nullable => { + BoundType::IsA if *nullable => { format!("Option<{ref_str}{trait_bound}>") } - BoundType::IsA(_) => format!("{ref_str}{trait_bound}"), - BoundType::AsRef(_) if *nullable => { + BoundType::IsA => format!("{ref_str}{trait_bound}"), + BoundType::AsRef if *nullable => { format!("Option<{trait_bound}>") } - BoundType::NoWrapper | BoundType::AsRef(_) => trait_bound, + BoundType::NoWrapper | BoundType::AsRef => trait_bound, } } @@ -71,21 +71,21 @@ impl Bound { pub(super) fn trait_bound(&self, r#async: bool) -> String { match self.bound_type { BoundType::NoWrapper => self.type_str.clone(), - BoundType::IsA(lifetime) => { + BoundType::IsA => { if r#async { - assert!(lifetime.is_none(), "Async overwrites lifetime"); + assert!(self.lt.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}"))) + .or_else(|| self.lt.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), + BoundType::AsRef if self.lt.is_some() => panic!("AsRef cannot have a lifetime"), + BoundType::AsRef => format!("AsRef<{}>", self.type_str), } } } diff --git a/src/codegen/function.rs b/src/codegen/function.rs index fab500b2f..93b37429d 100644 --- a/src/codegen/function.rs +++ b/src/codegen/function.rs @@ -302,7 +302,7 @@ pub fn bounds( // TODO: False or true? .filter(|bound| bound.alias.is_some_and(|alias| skip.contains(&alias))) .filter_map(|bound| match bound.bound_type { - IsA(lifetime) | AsRef(lifetime) => lifetime, + IsA | AsRef => bound.lt, _ => None, }) .collect::>(); @@ -438,10 +438,17 @@ pub fn body_chunk_futures( if is_slice { writeln!(body, "let {} = {}.to_vec();", par.name, par.name)?; } else if *c_par.nullable { + let to_glib_extra = analysis + .bounds + .get_parameter_bound(&c_par.name) + .map_or_else(String::new, |b| { + b.bound_type + .get_to_glib_extra(*c_par.nullable, c_par.instance_parameter, true) + }); writeln!( body, - "let {} = {}.map(ToOwned::to_owned);", - par.name, par.name + "let {} = {}{}.map(ToOwned::to_owned);", + par.name, par.name, to_glib_extra, )?; } else if is_str { writeln!(body, "let {} = String::from({});", par.name, par.name)?; diff --git a/src/codegen/object.rs b/src/codegen/object.rs index 3f7bf3b0d..10c36aea1 100644 --- a/src/codegen/object.rs +++ b/src/codegen/object.rs @@ -392,7 +392,7 @@ fn generate_builder(w: &mut dyn Write, env: &Env, analysis: &analysis::object::I bound.full_type_parameter_reference(RefMode::ByRef, Nullable(false), false) }); let conversion = param_bound.and_then(|bound| match bound.bound_type { - BoundType::AsRef(_) => Some(".as_ref().clone()"), + BoundType::AsRef => Some(".as_ref().clone()"), _ => None, }); (alias, bounds, conversion.unwrap_or(".clone().upcast()")) diff --git a/src/codegen/properties.rs b/src/codegen/properties.rs index 163a1ed1d..1e01679bb 100644 --- a/src/codegen/properties.rs +++ b/src/codegen/properties.rs @@ -84,9 +84,12 @@ fn declaration(env: &Env, prop: &Property) -> String { let set_param = if prop.is_get { bound = String::new(); String::new() - } else if let Some(ref set_bound) = prop.set_bound { - bound = format!("<{}: IsA<{}>>", set_bound.alias, set_bound.type_str); - format!(", {}: Option<&{}>", prop.var_name, set_bound.alias) + } else if let Some(set_bound) = prop.set_bound() { + let alias = set_bound + .alias + .expect("Property only supports IsA bound type with an alias"); + bound = format!("<{}: IsA<{}>>", alias, set_bound.type_str); + format!(", {}: Option<&{}>", prop.var_name, alias) } else { bound = String::new(); let dir = library::ParameterDirection::In; @@ -117,10 +120,11 @@ fn declaration(env: &Env, prop: &Property) -> String { } fn body(env: &Env, prop: &Property, in_trait: bool) -> Chunk { - property_body::Builder::new(env) + property_body::Builder::new(env, prop.set_bound()) .name(&prop.name) .in_trait(in_trait) .var_name(&prop.var_name) + .nullable(*prop.nullable) .for_get(prop.is_get) .type_(&RustType::try_new(env, prop.typ).into_string()) .generate() diff --git a/src/codegen/property_body.rs b/src/codegen/property_body.rs index fdd709bc0..96c802883 100644 --- a/src/codegen/property_body.rs +++ b/src/codegen/property_body.rs @@ -1,4 +1,4 @@ -use crate::{chunk::Chunk, env::Env, nameutil::use_gtk_type}; +use crate::{analysis::bounds::Bound, chunk::Chunk, env::Env, nameutil::use_gtk_type}; #[derive(Debug, Default)] pub struct Builder<'a> { @@ -6,15 +6,18 @@ pub struct Builder<'a> { in_trait: bool, var_name: String, for_get: bool, + nullable: bool, is_child_property: bool, type_: String, + set_bound: Option<&'a Bound>, env: Option<&'a Env>, } impl<'a> Builder<'a> { - pub fn new(env: &'a Env) -> Self { + pub fn new(env: &'a Env, set_bound: Option<&'a Bound>) -> Self { Self { env: Some(env), + set_bound, ..Default::default() } } @@ -47,6 +50,11 @@ impl<'a> Builder<'a> { self } + pub fn nullable(mut self, nullable: bool) -> Self { + self.nullable = nullable; + self + } + pub fn type_(mut self, type_: &str) -> Self { self.type_ = type_.into(); self @@ -111,9 +119,13 @@ impl<'a> Builder<'a> { "self" }; + let to_glib_extra = self.set_bound.as_ref().map_or_else(String::new, |b| { + b.bound_type.get_to_glib_extra(self.nullable, false, false) + }); + vec![Chunk::Custom(format!( - "ObjectExt::set_property({},\"{}\", {})", - self_, self.name, self.var_name + "ObjectExt::set_property({self_},\"{}\", {}{to_glib_extra})", + self.name, self.var_name, ))] } } diff --git a/src/codegen/translate_to_glib.rs b/src/codegen/translate_to_glib.rs index 2d3b4afaf..aafaa178c 100644 --- a/src/codegen/translate_to_glib.rs +++ b/src/codegen/translate_to_glib.rs @@ -14,11 +14,10 @@ impl TranslateToGlib for TransformationType { ToGlibDirect { ref name } => name.clone(), ToGlibScalar { ref name, - needs_into, + ref to_glib_extra, .. } => { - let pre_into = if needs_into { ".into()" } else { "" }; - format!("{}{}{}", name, pre_into, ".into_glib()") + format!("{name}{to_glib_extra}.into_glib()") } ToGlibPointer { ref name, diff --git a/src/writer/to_code.rs b/src/writer/to_code.rs index 038813f20..4b8608215 100644 --- a/src/writer/to_code.rs +++ b/src/writer/to_code.rs @@ -170,10 +170,10 @@ impl ToCode for Chunk { Name(ref name) => vec![name.clone()], ExternCFunc { ref name, + ref bounds, ref parameters, ref body, ref return_value, - ref bounds, } => { let prefix = format!(r#"unsafe extern "C" fn {name}{bounds}("#); let suffix = ")".to_string(); From 24f0d6639caa6dfb17ce2c92f67499bc7ba3a7af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Thu, 5 Dec 2024 22:40:50 +0100 Subject: [PATCH 4/4] Generate `Into>` in argument position where applicable This commit adds support for generating `Into>`, `Into>` and `Into>` in argument position. The existing `analysis::Bound` type was updated to support new bounds for these variants: 1. Owned value This is straightforward, just need a `to_glib_extra` `.into()`: ```rust impl AudioDecoder { fn finish_frame(&self, buf: impl Into>) -> Result<...> { [...] ffi::gst_audio_decoder_finish_frame( [...] buf.into().into_glib_ptr(), ) [...] } } ``` 2. Concrete types by reference Same, but needs a lifetime: ```rust impl TextOverlay { fn set_text<'a>(&self, text: impl Into>) { [...] ffi::ges_text_overlay_set_text() [...] text.into().to_glib_none().0, )) [...] } } ``` 3. Trait bound by reference Needs a lifetime and a generic parameter and a longer `to_glib_extra`: ```rust impl Pipeline { fn use_clock<'a, P: IsA>(&self, clock: impl Into>) { [...] ffi::gst_pipeline_use_clock( [...] clock.into().as_ref().map(|p| p.as_ref()).to_glib_none().0, )) [...] } } ``` Other Changes: These changes revealed a bug in trampoline `user_data` generic parameters handling: these parameters can be grouped, in which case the grouped callbacks are passed as a tuple in `user_data`. The actual callback types are then required to recover the callbacks from the tuple. The way it was implemented, all the callback generic parameters (bounds) from the outter function were considered as taking part in the `user_data`, regardless of the actual grouping. From the code bases on which I tested this, this had no consequences since callbacks for a particular function were all grouped anyway. However, with the new bounds implemented by this commit, functions with callbacks can now use a lifetime, which may not be part of the callback signatures, in which case it should not be included as part of a callback group. This is now fixed. I took the liberty to add details and remane a couple of identifiers to ease my understanding of what this code was achieving. --- src/analysis/bounds.rs | 439 ++++++++++++++++------------ src/analysis/child_properties.rs | 52 ++-- src/analysis/class_builder.rs | 9 +- src/analysis/function_parameters.rs | 18 +- src/analysis/functions.rs | 2 + src/analysis/ref_mode.rs | 23 ++ src/analysis/trampolines.rs | 7 +- src/chunk/chunk.rs | 2 +- src/codegen/bound.rs | 137 +++++---- src/codegen/bounds.rs | 46 +++ src/codegen/function.rs | 94 +----- src/codegen/function_body_chunk.rs | 124 +++++--- src/codegen/mod.rs | 1 + src/codegen/object.rs | 46 +-- src/codegen/properties.rs | 23 +- src/writer/to_code.rs | 4 +- 16 files changed, 587 insertions(+), 440 deletions(-) create mode 100644 src/codegen/bounds.rs diff --git a/src/analysis/bounds.rs b/src/analysis/bounds.rs index bbebfb550..570c1a02e 100644 --- a/src/analysis/bounds.rs +++ b/src/analysis/bounds.rs @@ -9,10 +9,13 @@ use crate::{ ref_mode::RefMode, rust_type::RustType, }, - config, + config::{self, parameter_matchable::ParameterMatchable}, consts::TYPE_PARAMETERS_START, env::Env, - library::{Basic, Class, Concurrency, Function, Nullable, ParameterDirection, Type, TypeId}, + library::{ + Basic, Class, Concurrency, Function, Nullable, ParameterDirection, ParameterScope, Type, + TypeId, + }, traits::*, }; @@ -21,13 +24,12 @@ pub enum BoundType { NoWrapper, IsA, AsRef, + IntoOption, + IntoOptionRef, + IntoOptionIsA, } impl BoundType { - pub fn need_isa(&self) -> bool { - matches!(*self, Self::IsA) - } - pub fn get_to_glib_extra( &self, nullable: bool, @@ -36,6 +38,7 @@ impl BoundType { ) -> String { use BoundType::*; match *self { + NoWrapper => "", AsRef if move_ && nullable => ".map(|p| p.as_ref().clone().upcast())", AsRef if nullable => ".as_ref().map(|p| p.as_ref())", AsRef if move_ => ".upcast()", @@ -44,7 +47,9 @@ impl BoundType { IsA if nullable && !instance_parameter => ".map(|p| p.as_ref())", IsA if move_ => ".upcast()", IsA => ".as_ref()", - _ => "", + IntoOption | IntoOptionRef { .. } => ".into()", + IntoOptionIsA if move_ => ".into()", + IntoOptionIsA => ".into().as_ref().map(|p| p.as_ref())", } .to_string() } @@ -63,8 +68,8 @@ pub struct Bound { #[derive(Clone, Debug)] pub struct Bounds { unused: VecDeque, - used: Vec, - lifetimes: Vec, + pub used: Vec, + pub lifetimes: Vec, } impl Default for Bounds { @@ -97,124 +102,144 @@ impl Bounds { concurrency: Concurrency, configured_functions: &[&config::functions::Function], ) -> (Option, Option) { + if par.direction == ParameterDirection::Out { + return (None, None); + } + let nullable = configured_functions .matched_parameters(&par.name) .iter() .find_map(|p| p.nullable) .unwrap_or(par.nullable); + let ref_mode = if par.move_ { + RefMode::None + } else { + par.ref_mode + }; let Ok(rust_type) = RustType::builder(env, par.typ) .direction(par.direction) .nullable(nullable) - .ref_mode(if par.move_ { - RefMode::None - } else { - par.ref_mode - }) + .ref_mode(ref_mode) .try_build() else { return (None, None); }; + + if par.instance_parameter { + let ret = Bounds::get_to_glib_extra_for( + env, + par.typ, + ref_mode, + nullable, + par.direction, + par.instance_parameter, + par.scope, + ); + + return (ret, None); + } + let mut type_string = rust_type.into_string(); - let mut callback_info = None; let mut ret = None; - let mut need_is_into_check = false; - - if !par.instance_parameter && par.direction != ParameterDirection::Out { - if let Some(bound_type) = Bounds::type_for(env, par.typ) { - ret = Some(bound_type.get_to_glib_extra( - *par.nullable, - par.instance_parameter, - par.move_, - )); - if r#async && (par.name == "callback" || par.name.ends_with("_callback")) { - let func_name = func.c_identifier.as_ref().unwrap(); - let finish_func_name = if let Some(finish_func_name) = &func.finish_func { - finish_func_name.to_string() - } else { - finish_function_name(func_name) - }; - if let Some(function) = find_function(env, &finish_func_name) { - // FIXME: This should work completely based on the analysis of the finish() - // function but that a) happens afterwards and b) is - // not accessible from here either. - let mut out_parameters = - find_out_parameters(env, function, configured_functions); - if use_function_return_for_result( - env, - function.ret.typ, - &func.name, - configured_functions, - ) { - let nullable = configured_functions - .iter() - .find_map(|f| f.ret.nullable) - .unwrap_or(function.ret.nullable); - - out_parameters.insert( - 0, - RustType::builder(env, function.ret.typ) - .direction(function.ret.direction) - .nullable(nullable) - .try_build_param() - .into_string(), - ); - } - let parameters = format_out_parameters(&out_parameters); - let error_type = find_error_type(env, function); - if let Some(ref error) = error_type { - type_string = - format!("FnOnce(Result<{parameters}, {error}>) + 'static"); - } else { - type_string = format!("FnOnce({parameters}) + 'static"); - } - let bound_name = *self.unused.front().unwrap(); - callback_info = Some(CallbackInfo { - callback_type: type_string.clone(), - success_parameters: parameters, - error_parameters: error_type, - bound_name, - }); + let mut callback_info = None; + + if let Some(mut bound_type) = Bounds::type_for( + env, + par.typ, + ref_mode, + nullable, + par.direction, + par.instance_parameter, + par.scope, + ) { + ret = Some(bound_type.get_to_glib_extra( + *par.nullable, + par.instance_parameter, + par.move_, + )); + + if r#async && (par.name == "callback" || par.name.ends_with("_callback")) { + bound_type = BoundType::NoWrapper; + + let func_name = func.c_identifier.as_ref().unwrap(); + let finish_func_name = if let Some(finish_func_name) = &func.finish_func { + finish_func_name.to_string() + } else { + finish_function_name(func_name) + }; + if let Some(function) = find_function(env, &finish_func_name) { + // FIXME: This should work completely based on the analysis of the finish() + // function but that a) happens afterwards and b) is + // not accessible from here either. + let mut out_parameters = + find_out_parameters(env, function, configured_functions); + if use_function_return_for_result( + env, + function.ret.typ, + &func.name, + configured_functions, + ) { + let nullable = configured_functions + .iter() + .find_map(|f| f.ret.nullable) + .unwrap_or(function.ret.nullable); + + out_parameters.insert( + 0, + RustType::builder(env, function.ret.typ) + .direction(function.ret.direction) + .nullable(nullable) + .try_build_param() + .into_string(), + ); } - } else if par.c_type == "GDestroyNotify" || env.library.type_(par.typ).is_function() - { - need_is_into_check = par.c_type != "GDestroyNotify"; - if let Type::Function(_) = env.library.type_(par.typ) { - let callback_parameters_config = - configured_functions.iter().find_map(|f| { - f.parameters - .iter() - .find(|p| p.ident.is_match(&par.name)) - .map(|p| &p.callback_parameters) - }); - - let mut rust_ty = RustType::builder(env, par.typ) - .direction(par.direction) - .scope(par.scope) - .concurrency(concurrency); - if let Some(callback_parameters_config) = callback_parameters_config { - rust_ty = - rust_ty.callback_parameters_config(callback_parameters_config); - } - type_string = rust_ty - .try_from_glib(&par.try_from_glib) - .try_build() - .into_string(); - let bound_name = *self.unused.front().unwrap(); - callback_info = Some(CallbackInfo { - callback_type: type_string.clone(), - success_parameters: String::new(), - error_parameters: None, - bound_name, - }); + let parameters = format_out_parameters(&out_parameters); + let error_type = find_error_type(env, function); + if let Some(ref error) = error_type { + type_string = format!("FnOnce(Result<{parameters}, {error}>) + 'static"); + } else { + type_string = format!("FnOnce({parameters}) + 'static"); } + let bound_name = *self.unused.front().unwrap(); + callback_info = Some(CallbackInfo { + callback_type: type_string.clone(), + success_parameters: parameters, + error_parameters: error_type, + bound_name, + }); } - 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.c_type == "GDestroyNotify" || env.library.type_(par.typ).is_function() { + if let Type::Function(_) = env.library.type_(par.typ) { + let callback_parameters_config = configured_functions.iter().find_map(|f| { + f.parameters + .iter() + .find(|p| p.ident.is_match(&par.name)) + .map(|p| &p.callback_parameters) + }); + + let mut rust_ty = RustType::builder(env, par.typ) + .direction(par.direction) + .scope(par.scope) + .concurrency(concurrency); + if let Some(callback_parameters_config) = callback_parameters_config { + rust_ty = rust_ty.callback_parameters_config(callback_parameters_config); + } + type_string = rust_ty + .try_from_glib(&par.try_from_glib) + .try_build() + .into_string(); + let bound_name = *self.unused.front().unwrap(); + callback_info = Some(CallbackInfo { + callback_type: type_string.clone(), + success_parameters: String::new(), + error_parameters: None, + bound_name, + }); } } - } else if par.instance_parameter { - if let Some(bound_type) = Bounds::type_for(env, par.typ) { - ret = Some(bound_type.get_to_glib_extra(*par.nullable, true, par.move_)); + + if par.c_type != "GDestroyNotify" { + self.add_for(&par.name, &type_string, bound_type); } } @@ -229,6 +254,18 @@ impl Bounds { ref_mode: RefMode, nullable: Nullable, ) -> bool { + let Some(bound_type) = Bounds::type_for( + env, + type_id, + ref_mode, + nullable, + ParameterDirection::In, + false, + ParameterScope::None, + ) else { + return false; + }; + let type_str = RustType::builder(env, type_id) .nullable(nullable) .direction(ParameterDirection::In) @@ -236,47 +273,66 @@ impl Bounds { .try_build() .into_string(); - let Some(bound_type) = Bounds::type_for(env, type_id) else { - return false; - }; - - let alias = match bound_type { - BoundType::NoWrapper | BoundType::IsA => { - Some(self.unused.pop_front().expect("No free type aliases!")) - } - _ => None, - }; - - self.push(Bound { - bound_type, - parameter_name: name.to_string(), - lt: None, - alias, - type_str, - callback_modified: false, - }); + self.add_for(name, &type_str, bound_type); true } - pub fn type_for(env: &Env, type_id: TypeId) -> Option { - use self::BoundType::*; + fn type_for( + env: &Env, + type_id: TypeId, + ref_mode: RefMode, + nullable: Nullable, + direction: ParameterDirection, + instance_parameter: bool, + scope: ParameterScope, + ) -> Option { match env.library.type_(type_id) { - Type::Basic(Basic::Filename | Basic::OsString) => Some(AsRef), + Type::Basic(Basic::Filename | Basic::OsString) => Some(BoundType::AsRef), Type::Class(Class { is_fundamental: true, .. - }) => Some(AsRef), + }) => Some(BoundType::AsRef), Type::Class(Class { final_type: true, .. - }) => None, + }) => { + if *nullable + && direction == ParameterDirection::In + && !ref_mode.is_none() + && !instance_parameter + { + Some(BoundType::IntoOptionRef) + } else { + None + } + } Type::Class(Class { final_type: false, .. - }) => Some(IsA), - Type::Interface(..) => Some(IsA), + }) + | Type::Interface(..) => { + if *nullable + && direction == ParameterDirection::In + && !ref_mode.is_none() + && !instance_parameter + { + Some(BoundType::IntoOptionIsA) + } else { + Some(BoundType::IsA) + } + } Type::List(_) | Type::SList(_) | Type::CArray(_) => None, - Type::Function(_) => Some(NoWrapper), - _ => None, + Type::Function(_) => Some(BoundType::NoWrapper), + _ => { + if *nullable && direction == ParameterDirection::In && scope.is_none() { + if !ref_mode.is_none() { + Some(BoundType::IntoOptionRef) + } else { + Some(BoundType::IntoOption) + } + } else { + None + } + } } } @@ -285,13 +341,21 @@ impl Bounds { type_id: TypeId, ref_mode: RefMode, nullable: Nullable, + direction: ParameterDirection, instance_parameter: bool, + scope: ParameterScope, ) -> Option { - let res = Self::type_for(env, type_id)?.get_to_glib_extra( - *nullable, + let bound_type = Self::type_for( + env, + type_id, + ref_mode, + nullable, + direction, instance_parameter, - ref_mode.is_none(), - ); + scope, + )?; + + let res = bound_type.get_to_glib_extra(*nullable, instance_parameter, ref_mode.is_none()); if res.is_empty() { return None; @@ -300,33 +364,35 @@ impl Bounds { Some(res) } - pub fn add_parameter( - &mut self, - name: &str, - type_str: &str, - mut bound_type: BoundType, - r#async: bool, - ) { - if r#async && name == "callback" { - bound_type = BoundType::NoWrapper; - } + pub fn add_for(&mut self, name: &str, type_str: &str, bound_type: BoundType) { if self.used.iter().any(|n| n.parameter_name == name) { return; } - let alias = match bound_type { + + let mut lt = None; + let mut alias = None; + match bound_type { BoundType::NoWrapper => { // TODO: This is just a heuristic for now, based on what we do in codegen! // Theoretically the surrounding function should determine whether it needs to // reuse an alias (ie. to use in `call_func::`) or not. // In the latter case an `impl` is generated instead of a type name/alias. - Some(self.unused.pop_front().expect("No free type aliases!")) + alias = Some(self.unused.pop_front().expect("No free type aliases!")); } - _ => None, - }; + BoundType::IntoOptionRef => { + lt = Some(self.get_lifetime()); + } + BoundType::IntoOptionIsA => { + lt = Some(self.get_lifetime()); + alias = Some(self.unused.pop_front().expect("No free type aliases!")); + } + _ => (), + } + self.used.push(Bound { bound_type, parameter_name: name.to_owned(), - lt: None, + lt, alias, type_str: type_str.to_owned(), callback_modified: false, @@ -357,6 +423,16 @@ impl Bounds { self.used.sort_by_key(|u| u.alias); } + fn get_lifetime(&mut self) -> char { + // In practice, we always need at most one lifetime so far + let lt = 'a'; + if self.lifetimes.is_empty() { + self.lifetimes.push(lt); + } + + lt + } + pub fn get_parameter_bound(&self, name: &str) -> Option<&Bound> { self.iter().find(move |n| n.parameter_name == name) } @@ -366,9 +442,9 @@ impl Bounds { use self::BoundType::*; for used in &self.used { match used.bound_type { - NoWrapper => (), - IsA => imports.add("glib::prelude::*"), - AsRef => imports.add_used_type(&used.type_str), + NoWrapper { .. } | IntoOption | IntoOptionRef { .. } => (), + IsA { .. } | IntoOptionIsA { .. } => imports.add("glib::prelude::*"), + AsRef { .. } => imports.add_used_type(&used.type_str), } } } @@ -381,8 +457,8 @@ impl Bounds { self.used.iter() } - pub fn iter_lifetimes(&self) -> Iter<'_, char> { - self.lifetimes.iter() + pub fn iter_aliases(&self) -> impl Iterator + '_ { + self.used.iter().flat_map(|u| u.alias) } } @@ -448,29 +524,30 @@ fn find_error_type(env: &Env, function: &Function) -> Option { #[cfg(test)] mod tests { use super::*; + use std::mem::discriminant; #[test] fn get_new_all() { let mut bounds: Bounds = Default::default(); let typ = BoundType::IsA; - bounds.add_parameter("a", "", typ, false); + bounds.add_for("a", "", typ); assert_eq!(bounds.iter().len(), 1); // Don't add second time - bounds.add_parameter("a", "", typ, false); + bounds.add_for("a", "", typ); assert_eq!(bounds.iter().len(), 1); - bounds.add_parameter("b", "", typ, false); - bounds.add_parameter("c", "", typ, false); - bounds.add_parameter("d", "", typ, false); - bounds.add_parameter("e", "", typ, false); - bounds.add_parameter("f", "", typ, false); - bounds.add_parameter("g", "", typ, false); - bounds.add_parameter("h", "", typ, false); + bounds.add_for("b", "", typ); + bounds.add_for("c", "", typ); + bounds.add_for("d", "", typ); + bounds.add_for("e", "", typ); + bounds.add_for("f", "", typ); + bounds.add_for("g", "", typ); + bounds.add_for("h", "", typ); assert_eq!(bounds.iter().len(), 8); - bounds.add_parameter("h", "", typ, false); + bounds.add_for("h", "", typ); assert_eq!(bounds.iter().len(), 8); - bounds.add_parameter("i", "", typ, false); - bounds.add_parameter("j", "", typ, false); - bounds.add_parameter("k", "", typ, false); + bounds.add_for("i", "", typ); + bounds.add_for("j", "", typ); + bounds.add_for("k", "", typ); } #[test] @@ -480,7 +557,7 @@ mod tests { 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, false); + bounds.add_for(c.to_string().as_str(), "", typ); } } @@ -488,15 +565,15 @@ mod tests { fn get_parameter_bound() { let mut bounds: Bounds = Default::default(); let typ = BoundType::NoWrapper; - bounds.add_parameter("a", "", typ, false); - bounds.add_parameter("b", "", typ, false); + bounds.add_for("a", "", typ); + bounds.add_for("b", "", typ); let bound = bounds.get_parameter_bound("a").unwrap(); // `NoWrapper `bounds are expected to have an alias: assert_eq!(bound.alias, Some('P')); - assert_eq!(bound.bound_type, typ); + assert_eq!(discriminant(&bound.bound_type), discriminant(&typ)); let bound = bounds.get_parameter_bound("b").unwrap(); assert_eq!(bound.alias, Some('Q')); - assert_eq!(bound.bound_type, typ); + assert_eq!(discriminant(&bound.bound_type), discriminant(&typ)); assert_eq!(bounds.get_parameter_bound("c"), None); } @@ -504,8 +581,8 @@ mod tests { fn impl_bound() { let mut bounds: Bounds = Default::default(); let typ = BoundType::IsA; - bounds.add_parameter("a", "", typ, false); - bounds.add_parameter("b", "", typ, false); + bounds.add_for("a", "", typ); + bounds.add_for("b", "", typ); let bound = bounds.get_parameter_bound("a").unwrap(); // `IsA` is simplified to an inline `foo: impl IsA` and // lacks an alias/type-parameter: @@ -513,7 +590,7 @@ mod tests { assert_eq!(bound.bound_type, typ); let typ = BoundType::AsRef; - bounds.add_parameter("c", "", typ, false); + bounds.add_for("c", "", typ); let bound = bounds.get_parameter_bound("c").unwrap(); // Same `impl AsRef` simplification as `IsA`: assert_eq!(bound.alias, None); diff --git a/src/analysis/child_properties.rs b/src/analysis/child_properties.rs index e19eb80da..762405561 100644 --- a/src/analysis/child_properties.rs +++ b/src/analysis/child_properties.rs @@ -113,33 +113,35 @@ fn analyze_property( } let nullable = library::Nullable(set_in_ref_mode.is_ref()); + let mut bounds = Bounds::default(); let mut bounds_str = String::new(); let dir = ParameterDirection::In; - let set_params = if Bounds::type_for(env, typ).is_some() { - // TODO: bounds_str push?!?! - bounds_str.push_str("TODO"); - format!("{prop_name}: {TYPE_PARAMETERS_START}") - // let mut bounds = Bounds::default(); - // bounds.add_parameter("P", &r_type, bound, false); - // let (s_bounds, _) = function::bounds(&bounds, &[], false); - // // Because the bounds won't necessarily be added into the final - // function, we // only keep the "inner" part to make - // the string computation easier. So // `` becomes - // `T: X`. bounds_str.push_str(&s_bounds[1..s_bounds. - // len() - 1]); format!("{}: {}", prop_name, - // bounds.iter().last().unwrap().alias) - } else { - format!( - "{}: {}", - prop_name, - RustType::builder(env, typ) - .direction(dir) - .nullable(nullable) - .ref_mode(set_in_ref_mode) - .try_build_param() - .into_string() - ) - }; + let set_params = + if bounds.add_for_property_setter(env, typ, &prop.name, set_in_ref_mode, nullable) { + // TODO: bounds_str push?!?! + bounds_str.push_str("TODO"); + format!("{prop_name}: {TYPE_PARAMETERS_START}") + // let mut bounds = Bounds::default(); + // bounds.add_parameter("P", &r_type, bound, false); + // let (s_bounds, _) = function::bounds(&bounds, &[], false); + // // Because the bounds won't necessarily be added into the final + // function, we // only keep the "inner" part to make + // the string computation easier. So // `` becomes + // `T: X`. bounds_str.push_str(&s_bounds[1..s_bounds. + // len() - 1]); format!("{}: {}", prop_name, + // bounds.iter().last().unwrap().alias) + } else { + format!( + "{}: {}", + prop_name, + RustType::builder(env, typ) + .direction(dir) + .nullable(nullable) + .ref_mode(set_in_ref_mode) + .try_build_param() + .into_string() + ) + }; Some(ChildProperty { name, diff --git a/src/analysis/class_builder.rs b/src/analysis/class_builder.rs index a22601648..a253cfd06 100644 --- a/src/analysis/class_builder.rs +++ b/src/analysis/class_builder.rs @@ -121,12 +121,13 @@ fn analyze_property( } } - let (get_out_ref_mode, set_in_ref_mode, _) = get_property_ref_modes(env, prop); + let (get_out_ref_mode, set_in_ref_mode, nullable) = get_property_ref_modes(env, prop); let mut bounds = Bounds::default(); - if let Some(bound) = Bounds::type_for(env, prop.typ) { + let set_has_bound = + bounds.add_for_property_setter(env, prop.typ, &prop.name, set_in_ref_mode, nullable); + if set_has_bound { imports.add("glib::prelude::*"); - bounds.add_parameter(&prop.name, &rust_type_res.into_string(), bound, false); } Some(Property { @@ -136,7 +137,7 @@ fn analyze_property( is_get: false, func_name: String::new(), func_name_alias: None, - nullable: library::Nullable(false), // no Options for builder setters here + nullable, get_out_ref_mode, set_in_ref_mode, bounds, diff --git a/src/analysis/function_parameters.rs b/src/analysis/function_parameters.rs index 95a97b80f..5ccdc9386 100644 --- a/src/analysis/function_parameters.rs +++ b/src/analysis/function_parameters.rs @@ -289,14 +289,16 @@ pub fn analyze( } if let Some(array_par) = array_par { let mut array_name = nameutil::mangle_keywords(&array_par.name); - if let Some(bound_type) = Bounds::type_for(env, array_par.typ) { - array_name = (array_name.into_owned() - + &bound_type.get_to_glib_extra( - *array_par.nullable, - array_par.instance_parameter, - move_, - )) - .into(); + if let Some(to_glib_extra) = Bounds::get_to_glib_extra_for( + env, + array_par.typ, + if move_ { RefMode::None } else { RefMode::ByRef }, + Nullable(false), + array_par.direction, + array_par.instance_parameter, + array_par.scope, + ) { + array_name = (array_name.into_owned() + &to_glib_extra).into(); } add_rust_parameter = false; diff --git a/src/analysis/functions.rs b/src/analysis/functions.rs index ce7414b80..83a595e4b 100644 --- a/src/analysis/functions.rs +++ b/src/analysis/functions.rs @@ -390,7 +390,9 @@ fn analyze_callbacks( par.ref_mode }, par.nullable, + par.direction, par.instance_parameter, + par.scope, ), None, ) diff --git a/src/analysis/ref_mode.rs b/src/analysis/ref_mode.rs index 73a319f6e..4eb3e5c06 100644 --- a/src/analysis/ref_mode.rs +++ b/src/analysis/ref_mode.rs @@ -128,6 +128,29 @@ impl RefMode { pub fn is_none(self) -> bool { matches!(self, Self::None) } + + pub fn to_string_with_maybe_lt(self, lt: Option) -> String { + match self { + RefMode::None | RefMode::ByRefFake => { + assert!(lt.is_none(), "incompatible ref mode {self:?} with lifetime"); + String::new() + } + RefMode::ByRef | RefMode::ByRefImmut | RefMode::ByRefConst => { + if let Some(lt) = lt { + format!("&'{lt}") + } else { + "&".to_string() + } + } + RefMode::ByRefMut => { + if let Some(lt) = lt { + format!("&'{lt} mut ") + } else { + "&mut ".to_string() + } + } + } + } } impl fmt::Display for RefMode { diff --git a/src/analysis/trampolines.rs b/src/analysis/trampolines.rs index 386f7567a..5c965b2b7 100644 --- a/src/analysis/trampolines.rs +++ b/src/analysis/trampolines.rs @@ -37,8 +37,7 @@ pub struct Trampoline { pub user_data_index: usize, pub destroy_index: usize, pub nullable: library::Nullable, - /// This field is used to give the type name when generating the "IsA" - /// part. + /// This field is used to give the type name when generating the `IsA` part. pub type_name: String, } @@ -81,9 +80,9 @@ pub fn analyze( if in_trait || fundamental_type { let type_name = RustType::builder(env, type_tid).try_build(); if fundamental_type { - bounds.add_parameter("this", &type_name.into_string(), BoundType::AsRef, false); + bounds.add_for("this", &type_name.into_string(), BoundType::AsRef); } else { - bounds.add_parameter("this", &type_name.into_string(), BoundType::IsA, false); + bounds.add_for("this", &type_name.into_string(), BoundType::IsA); } } diff --git a/src/chunk/chunk.rs b/src/chunk/chunk.rs index 4919360df..969f35908 100644 --- a/src/chunk/chunk.rs +++ b/src/chunk/chunk.rs @@ -67,7 +67,7 @@ pub enum Chunk { Name(String), ExternCFunc { name: String, - bounds: String, + generic_params: String, parameters: Vec, body: Box, return_value: Option, diff --git a/src/codegen/bound.rs b/src/codegen/bound.rs index 0cf41fa68..6dbce8763 100644 --- a/src/codegen/bound.rs +++ b/src/codegen/bound.rs @@ -9,83 +9,108 @@ use crate::{ impl Bound { /// Returns the type parameter reference. /// Currently always returns the alias. - pub(super) fn type_parameter_reference(&self) -> Option { - self.alias + /// + /// This doesn't include the lifetime, which could be shared by other type + /// parameters. Use [Bounds::to_generic_params_str](crate::analysis::bounds::Bounds::to_generic_params_str) + /// to get the full generic parameter list, including lifetimes. + pub fn type_parameter_definition(&self, r#async: bool) -> Option { + use BoundType::*; + match self.bound_type { + NoWrapper => { + let alias = self.alias.expect("must be defined in this context"); + Some(format!("{alias}: {}", self.type_str)) + } + IsA if self.alias.is_some() => Some(format!( + "{}: IsA<{}>{}", + self.alias.unwrap(), + self.type_str, + if r#async { " + Clone + 'static" } else { "" }, + )), + IntoOptionIsA => { + let alias = self.alias.expect("must be defined in this context"); + Some(format!( + "{alias}: IsA<{}>{}", + self.type_str, + if r#async { " + Clone + 'static" } else { "" }, + )) + } + _ => None, + } } /// Returns the type parameter reference, with [`BoundType::IsA`] wrapped /// in `ref_mode` and `nullable` as appropriate. - pub(super) fn full_type_parameter_reference( + pub fn full_type_parameter_reference( &self, ref_mode: RefMode, nullable: Nullable, r#async: bool, ) -> String { - let ref_str = ref_mode.to_string(); + use BoundType::*; + match self.bound_type { + NoWrapper => self + .alias + .expect("must be defined in this context") + .to_string(), + IsA if self.alias.is_none() => { + let suffix = r#async + .then(|| " + Clone + 'static".to_string()) + .unwrap_or_default(); - // Generate `impl Trait` if this bound does not have an alias - let trait_bound = match self.type_parameter_reference() { - Some(t) => t.to_string(), - None => { - let trait_bound = self.trait_bound(r#async); - let trait_bound = format!("impl {trait_bound}"); + let mut trait_bound = format!("impl IsA<{}>{suffix}", self.type_str); - // Combining a ref mode and lifetime requires parentheses for disambiguation - match self.bound_type { - BoundType::IsA => { - // TODO: This is fragile - let has_lifetime = r#async || self.lt.is_some(); + let ref_str = ref_mode.to_string(); + if !ref_str.is_empty() && r#async { + trait_bound = format!("({trait_bound})"); + } - if !ref_str.is_empty() && has_lifetime { - format!("({trait_bound})") - } else { - trait_bound - } - } - _ => trait_bound, + if *nullable { + format!("Option<{ref_str}{trait_bound}>") + } else { + format!("{ref_str}{trait_bound}") } } - }; - - match self.bound_type { - BoundType::IsA if *nullable => { - format!("Option<{ref_str}{trait_bound}>") + IsA if self.alias.is_some() => { + let alias = self.alias.unwrap(); + let ref_str = ref_mode.to_string(); + if *nullable { + format!("Option<{ref_str} {alias}>") + } else { + format!("{ref_str} {alias}") + } } - BoundType::IsA => format!("{ref_str}{trait_bound}"), - BoundType::AsRef if *nullable => { - format!("Option<{trait_bound}>") + IsA => { + if *nullable { + format!("Option>", self.type_str) + } else { + format!("impl IsA<{}>", self.type_str) + } } - BoundType::NoWrapper | BoundType::AsRef => trait_bound, - } - } + AsRef => { + assert!(self.lt.is_none(), "AsRef cannot have a lifetime"); - /// Returns the type parameter definition for this bound, usually - /// of the form `T: SomeTrait` or `T: IsA`. - pub(super) fn type_parameter_definition(&self, r#async: bool) -> Option { - self.alias - .map(|alias| format!("{}: {}", alias, self.trait_bound(r#async))) - } - - /// Returns the trait bound, usually of the form `SomeTrait` - /// or `IsA`. - pub(super) fn trait_bound(&self, r#async: bool) -> String { - match self.bound_type { - BoundType::NoWrapper => self.type_str.clone(), - BoundType::IsA => { - if r#async { - assert!(self.lt.is_none(), "Async overwrites lifetime"); + if *nullable { + format!("Option>", self.type_str) + } else { + format!("impl AsRef<{}>", self.type_str) } - let is_a = format!("IsA<{}>", self.type_str); + } + IntoOption => { + format!("impl Into>", self.type_str) + } + IntoOptionRef => { + assert!(self.lt.is_some(), "must be defined in this context"); + let ref_str = ref_mode.to_string_with_maybe_lt(self.lt); - let lifetime = r#async - .then(|| " + Clone + 'static".to_string()) - .or_else(|| self.lt.map(|l| format!(" + '{l}"))) - .unwrap_or_default(); + format!("impl Into>", self.type_str) + } + IntoOptionIsA => { + assert!(self.lt.is_some(), "must be defined in this context"); + let ref_str = ref_mode.to_string_with_maybe_lt(self.lt); + let alias = self.alias.expect("must be defined in this context"); - format!("{is_a}{lifetime}") + format!("impl Into>") } - BoundType::AsRef if self.lt.is_some() => panic!("AsRef cannot have a lifetime"), - BoundType::AsRef => format!("AsRef<{}>", self.type_str), } } } diff --git a/src/codegen/bounds.rs b/src/codegen/bounds.rs new file mode 100644 index 000000000..e7c528bb0 --- /dev/null +++ b/src/codegen/bounds.rs @@ -0,0 +1,46 @@ +use crate::analysis::bounds::Bounds; + +impl Bounds { + pub fn to_generic_params_str(&self) -> String { + self.to_generic_params_str_(false) + } + + pub fn to_generic_params_str_async(&self) -> String { + self.to_generic_params_str_(true) + } + + fn to_generic_params_str_(&self, r#async: bool) -> String { + let mut res = String::new(); + + if self.lifetimes.is_empty() && self.used.iter().find_map(|b| b.alias).is_none() { + return res; + } + + res.push('<'); + let mut is_first = true; + + for lt in self.lifetimes.iter() { + if is_first { + is_first = false; + } else { + res.push_str(", "); + } + res.push('\''); + res.push(*lt); + } + + for bound in self.used.iter() { + if let Some(type_param_def) = bound.type_parameter_definition(r#async) { + if is_first { + is_first = false; + } else { + res.push_str(", "); + } + res.push_str(&type_param_def); + } + } + res.push('>'); + + res + } +} diff --git a/src/codegen/function.rs b/src/codegen/function.rs index 93b37429d..f1a05f87b 100644 --- a/src/codegen/function.rs +++ b/src/codegen/function.rs @@ -215,8 +215,6 @@ pub fn declaration(env: &Env, analysis: &analysis::functions::Info) -> String { }; let mut param_str = String::with_capacity(100); - let (bounds, _) = bounds(&analysis.bounds, &[], false, false); - for par in &analysis.parameters.rust_parameters { if !param_str.is_empty() { param_str.push_str(", "); @@ -229,7 +227,7 @@ pub fn declaration(env: &Env, analysis: &analysis::functions::Info) -> String { format!( "fn {}{}({}){}", analysis.codegen_name(), - bounds, + analysis.bounds.to_generic_params_str(), param_str, return_str, ) @@ -252,98 +250,34 @@ pub fn declaration_futures(env: &Env, analysis: &analysis::functions::Info) -> S let mut param_str = String::with_capacity(100); - let mut skipped = 0; - let mut skipped_bounds = vec![]; - for (pos, par) in analysis.parameters.rust_parameters.iter().enumerate() { + let mut bounds = Bounds::default(); + for par in analysis.parameters.rust_parameters.iter() { let c_par = &analysis.parameters.c_parameters[par.ind_c]; if c_par.name == "callback" || c_par.name == "cancellable" { - skipped += 1; - if let Some(alias) = analysis - .bounds - .get_parameter_bound(&c_par.name) - .and_then(|bound| bound.type_parameter_reference()) - { - skipped_bounds.push(alias); - } continue; } - if pos - skipped > 0 { - param_str.push_str(", "); + if let Some(bound) = analysis.bounds.get_parameter_bound(&c_par.name).cloned() { + bounds.push(bound); } let s = c_par.to_parameter(env, &analysis.bounds, true); + if !param_str.is_empty() { + param_str.push_str(", "); + } param_str.push_str(&s); } - let (bounds, _) = bounds(&analysis.bounds, skipped_bounds.as_ref(), true, false); - format!( "fn {}{}({}){}", - async_future.name, bounds, param_str, return_str, + async_future.name, + bounds.to_generic_params_str_async(), + param_str, + return_str, ) } -pub fn bounds( - bounds: &Bounds, - skip: &[char], - r#async: bool, - filter_callback_modified: bool, -) -> (String, Vec) { - use crate::analysis::bounds::BoundType::*; - - if bounds.is_empty() { - return (String::new(), Vec::new()); - } - - let skip_lifetimes = bounds - .iter() - // TODO: False or true? - .filter(|bound| bound.alias.is_some_and(|alias| skip.contains(&alias))) - .filter_map(|bound| match bound.bound_type { - IsA | AsRef => bound.lt, - _ => None, - }) - .collect::>(); - - let lifetimes = bounds - .iter_lifetimes() - .filter(|s| !skip_lifetimes.contains(s)) - .map(|s| format!("'{s}")) - .collect::>(); - - let bounds = bounds.iter().filter(|bound| { - bound.alias.map_or(true, |alias| !skip.contains(&alias)) - && (!filter_callback_modified || !bound.callback_modified) - }); - - let type_names = lifetimes - .iter() - .cloned() - .chain( - bounds - .clone() - .filter_map(|b| b.type_parameter_definition(r#async)), - ) - .collect::>(); - - let type_names = if type_names.is_empty() { - String::new() - } else { - format!("<{}>", type_names.join(", ")) - }; - - let bounds = lifetimes - .into_iter() - // TODO: enforce that this is only used on NoWrapper! - // TODO: Analyze if alias is used in function, otherwise set to None! - .chain(bounds.filter_map(|b| b.alias).map(|a| a.to_string())) - .collect::>(); - - (type_names, bounds) -} - pub fn body_chunk( env: &Env, analysis: &analysis::functions::Info, @@ -396,9 +330,7 @@ pub fn body_chunk( } } - let (bounds, bounds_names) = bounds(&analysis.bounds, &[], false, true); - - builder.generate(env, &bounds, &bounds_names.join(", ")) + builder.generate(env, &analysis.bounds) } pub fn body_chunk_futures( diff --git a/src/codegen/function_body_chunk.rs b/src/codegen/function_body_chunk.rs index 6bee059c9..37d376005 100644 --- a/src/codegen/function_body_chunk.rs +++ b/src/codegen/function_body_chunk.rs @@ -3,6 +3,7 @@ use std::collections::{hash_map::Entry, BTreeMap, HashMap}; use crate::{ analysis::{ self, + bounds::Bounds, conversion_type::ConversionType, function_parameters::{ CParameter as AnalysisCParameter, Transformation, TransformationType, @@ -70,12 +71,19 @@ pub struct Builder { // Key: user data index // Value: (global position used as id, type, callbacks) -type FuncParameters<'a> = BTreeMap>; +type UserDataById<'a> = BTreeMap>; -struct FuncParameter<'a> { +struct UserData<'a> { pos: usize, - full_type: Option<(String, String)>, + typ_: Option, callbacks: Vec<&'a Trampoline>, + generic_params: String, + qualified_aliases: String, +} + +struct UserDataType { + immutable: String, + mutable: String, } impl Builder { @@ -136,7 +144,7 @@ impl Builder { self.in_unsafe = in_unsafe; self } - pub fn generate(&self, env: &Env, bounds: &str, bounds_names: &str) -> Chunk { + pub fn generate(&self, env: &Env, outer_bounds: &Bounds) -> Chunk { let mut body = Vec::new(); let mut uninitialized_vars = if self.outs_as_return { @@ -145,28 +153,50 @@ impl Builder { Vec::new() }; - let mut group_by_user_data = FuncParameters::new(); + let mut user_data_by_id = UserDataById::new(); // We group arguments by callbacks. if !self.callbacks.is_empty() || !self.destroys.is_empty() { for (pos, callback) in self.callbacks.iter().enumerate() { let user_data_index = callback.user_data_index; - if group_by_user_data.contains_key(&user_data_index) { + if user_data_by_id.contains_key(&user_data_index) { continue; } + + let mut bounds = Bounds::default(); let calls = self .callbacks .iter() .filter(|c| c.user_data_index == user_data_index) + .inspect(|c| { + if let Some(bound) = outer_bounds.get_parameter_bound(&c.name).cloned() { + bounds.push(bound) + } + }) .collect::>(); - group_by_user_data.insert( + + let (generic_params, qualified_aliases) = if bounds.is_empty() { + (String::new(), String::new()) + } else { + let qualified_aliases = format!( + "::<{}>", + bounds + .iter_aliases() + .map(|a| a.to_string()) + .collect::>() + .join(", ") + ); + (bounds.to_generic_params_str(), qualified_aliases) + }; + + user_data_by_id.insert( user_data_index, - FuncParameter { + UserData { pos, - full_type: if calls.len() > 1 { + typ_: if calls.len() > 1 { if calls.iter().all(|c| c.scope.is_call()) { - Some(( - format!( + Some(UserDataType { + immutable: format!( "&({})", calls .iter() @@ -174,7 +204,7 @@ impl Builder { .collect::>() .join(", ") ), - format!( + mutable: format!( "&mut ({})", calls .iter() @@ -182,7 +212,7 @@ impl Builder { .collect::>() .join(", ") ), - )) + }) } else { let s = format!( "Box_<({})>", @@ -192,18 +222,23 @@ impl Builder { .collect::>() .join(", ") ); - Some((s.clone(), s)) + Some(UserDataType { + immutable: s.clone(), + mutable: s, + }) } } else { None }, callbacks: calls, + generic_params, + qualified_aliases, }, ); } } - let call = self.generate_call(&group_by_user_data); + let call = self.generate_call(&user_data_by_id); let call = self.generate_call_conversion(call, &mut uninitialized_vars); let ret = self.generate_out_return(&mut uninitialized_vars); let (call, ret) = self.apply_outs_mode(call, ret, &mut uninitialized_vars); @@ -221,7 +256,7 @@ impl Builder { if !self.callbacks.is_empty() || !self.destroys.is_empty() { // Key: user data index // Value: the current pos in the tuple for the given argument. - let mut poses = HashMap::with_capacity(group_by_user_data.len()); + let mut poses = HashMap::with_capacity(user_data_by_id.len()); for trampoline in &self.callbacks { *poses .entry(&trampoline.user_data_index) @@ -239,13 +274,11 @@ impl Builder { env, &mut chunks, trampoline, - &group_by_user_data[&user_data_index].full_type, + &user_data_by_id[&user_data_index], match pos { Entry::Occupied(ref x) => Some(*x.get()), _ => None, }, - bounds, - bounds_names, false, ); pos.and_modify(|x| { @@ -257,18 +290,17 @@ impl Builder { env, &mut chunks, destroy, - &group_by_user_data[&destroy.user_data_index].full_type, + &user_data_by_id[&destroy.user_data_index], None, // doesn't matter for destroy - bounds, - bounds_names, true, ); } - for FuncParameter { + for UserData { pos, - full_type, + typ_: user_data_type, callbacks: calls, - } in group_by_user_data.values() + .. + } in user_data_by_id.values() { if calls.len() > 1 { chunks.push(Chunk::Let { @@ -303,7 +335,10 @@ impl Builder { ) })), type_: Some(Box::new(Chunk::Custom( - full_type.clone().map(|x| x.0).unwrap(), + user_data_type + .as_ref() + .map(|x| x.immutable.clone()) + .unwrap(), ))), }); } else if !calls.is_empty() { @@ -384,14 +419,12 @@ impl Builder { env: &Env, chunks: &mut Vec, trampoline: &Trampoline, - full_type: &Option<(String, String)>, + user_data: &UserData, pos: Option, - bounds: &str, - bounds_names: &str, is_destroy: bool, ) { if !is_destroy { - if full_type.is_none() { + if user_data.typ_.is_none() { if trampoline.scope.is_call() { chunks.push(Chunk::Custom(format!( "let mut {0}_data: {1} = {0};", @@ -459,13 +492,13 @@ impl Builder { .last() .map_or_else(|| "Unknown".to_owned(), |p| p.name.clone()); - if let Some(full_type) = full_type { + if let Some(ref user_data_typ_) = user_data.typ_ { if is_destroy || trampoline.scope.is_async() { body.push(Chunk::Let { name: format!("{}callback", if is_destroy { "_" } else { "" }), is_mut: false, value: Box::new(Chunk::Custom(format!("Box_::from_raw({func} as *mut _)"))), - type_: Some(Box::new(Chunk::Custom(full_type.1.clone()))), + type_: Some(Box::new(Chunk::Custom(user_data_typ_.mutable.clone()))), }); } else { body.push(Chunk::Let { @@ -486,15 +519,15 @@ impl Builder { if !trampoline.scope.is_async() && !trampoline.scope.is_call() { format!( "&{}", - full_type - .1 + user_data_typ_ + .mutable .strip_prefix("Box_<") .unwrap() .strip_suffix(">") .unwrap() ) } else { - full_type.1.clone() + user_data_typ_.mutable.clone() }, ))), }); @@ -618,6 +651,7 @@ impl Builder { let extern_func = Chunk::ExternCFunc { name: format!("{}_func", trampoline.name), + generic_params: user_data.generic_params.clone(), parameters: trampoline .parameters .c_parameters @@ -650,31 +684,25 @@ impl Builder { } else { None }, - bounds: bounds.to_owned(), }; chunks.push(extern_func); - let bounds_str = if bounds_names.is_empty() { - String::new() - } else { - format!("::<{bounds_names}>") - }; if !is_destroy { if *trampoline.nullable { chunks.push(Chunk::Custom(format!( "let {0} = if {0}_data.is_some() {{ Some({0}_func{1} as _) }} else {{ None }};", - trampoline.name, bounds_str + trampoline.name, user_data.qualified_aliases, ))); } else { chunks.push(Chunk::Custom(format!( "let {0} = Some({0}_func{1} as _);", - trampoline.name, bounds_str + trampoline.name, user_data.qualified_aliases, ))); } } else { chunks.push(Chunk::Custom(format!( "let destroy_call{} = Some({}_func{} as _);", - trampoline.destroy_index, trampoline.name, bounds_str + trampoline.destroy_index, trampoline.name, user_data.qualified_aliases, ))); } } @@ -918,10 +946,10 @@ impl Builder { "{}<{}: {}>", trampoline.name, trampoline.bound_name, trampoline.callback_type ), + generic_params: String::new(), parameters, body: Box::new(Chunk::Chunks(body)), return_value: None, - bounds: String::new(), }); let chunk = Chunk::Let { name: "callback".to_string(), @@ -972,7 +1000,7 @@ impl Builder { } } - fn generate_call(&self, calls: &FuncParameters<'_>) -> Chunk { + fn generate_call(&self, calls: &UserDataById<'_>) -> Chunk { let params = self.generate_func_parameters(calls); Chunk::FfiCall { name: self.glib_name.clone(), @@ -992,7 +1020,7 @@ impl Builder { call: Box::new(call), } } - fn generate_func_parameters(&self, calls: &FuncParameters<'_>) -> Vec { + fn generate_func_parameters(&self, calls: &UserDataById<'_>) -> Vec { let mut params = Vec::new(); for trans in &self.transformations { if !trans.transformation_type.is_to_glib() { @@ -1010,7 +1038,7 @@ impl Builder { params.push(chunk); } let mut to_insert = Vec::new(); - for (user_data_index, FuncParameter { pos, callbacks, .. }) in calls.iter() { + for (user_data_index, UserData { pos, callbacks, .. }) in calls.iter() { let all_call = callbacks.iter().all(|c| c.scope.is_call()); to_insert.push(( *user_data_index, diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index a3752cf41..986755b47 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -16,6 +16,7 @@ use crate::{ mod alias; mod bound; +mod bounds; mod child_properties; mod constants; mod doc; diff --git a/src/codegen/object.rs b/src/codegen/object.rs index 10c36aea1..1546c568c 100644 --- a/src/codegen/object.rs +++ b/src/codegen/object.rs @@ -13,12 +13,11 @@ use super::{ }; use crate::{ analysis::{ - self, bounds::BoundType, object::has_builder_properties, record_type::RecordType, - ref_mode::RefMode, rust_type::RustType, safety_assertion_mode::SafetyAssertionMode, + self, object::has_builder_properties, record_type::RecordType, rust_type::RustType, + safety_assertion_mode::SafetyAssertionMode, }, env::Env, - library::{self, Nullable}, - nameutil, + library, nameutil, traits::IntoString, }; @@ -371,6 +370,7 @@ fn generate_builder(w: &mut dyn Write, env: &Env, analysis: &analysis::object::I let param_type = RustType::builder(env, property.typ) .direction(direction) .ref_mode(property.set_in_ref_mode) + .nullable(property.nullable) .try_build_param(); let comment_prefix = if param_type.is_err() { "//" } else { "" }; let mut param_type_str = param_type.into_string(); @@ -378,24 +378,32 @@ fn generate_builder(w: &mut dyn Write, env: &Env, analysis: &analysis::object::I "&str" => ( Some(format!("impl Into<{glib_crate_name}::GString>")), String::new(), - ".into()", + ".into()".to_string(), ), "&[&str]" => ( Some(format!("impl Into<{glib_crate_name}::StrV>")), String::new(), - ".into()", + ".into()".to_string(), ), - _ if !property.bounds.is_empty() => { - let (bounds, _) = function::bounds(&property.bounds, &[], false, false); - let param_bound = property.bounds.get_parameter_bound(&property.name); - let alias = param_bound.map(|bound| { - bound.full_type_parameter_reference(RefMode::ByRef, Nullable(false), false) - }); - let conversion = param_bound.and_then(|bound| match bound.bound_type { - BoundType::AsRef => Some(".as_ref().clone()"), - _ => None, - }); - (alias, bounds, conversion.unwrap_or(".clone().upcast()")) + _ if property.set_bound().is_some() => { + let set_bound = property.set_bound().unwrap(); + + let param_type_override = set_bound.full_type_parameter_reference( + property.set_in_ref_mode, + property.nullable, + false, + ); + let conversion = set_bound.bound_type.get_to_glib_extra( + *property.nullable, + false, + property.set_in_ref_mode.is_none(), + ); + + ( + Some(param_type_override), + property.bounds.to_generic_params_str(), + conversion, + ) } typ if typ.starts_with('&') => { let should_clone = @@ -415,9 +423,9 @@ fn generate_builder(w: &mut dyn Write, env: &Env, analysis: &analysis::object::I ".clone()" }; - (None, String::new(), should_clone) + (None, String::new(), should_clone.to_string()) } - _ => (None, String::new(), ""), + _ => (None, String::new(), "".to_string()), }; if let Some(param_type_override) = param_type_override { param_type_str = param_type_override.to_string(); diff --git a/src/codegen/properties.rs b/src/codegen/properties.rs index 1e01679bb..cde18417c 100644 --- a/src/codegen/properties.rs +++ b/src/codegen/properties.rs @@ -80,18 +80,19 @@ fn generate_prop_func( } fn declaration(env: &Env, prop: &Property) -> String { - let bound: String; + let generic_param: String; let set_param = if prop.is_get { - bound = String::new(); + generic_param = String::new(); String::new() } else if let Some(set_bound) = prop.set_bound() { - let alias = set_bound - .alias - .expect("Property only supports IsA bound type with an alias"); - bound = format!("<{}: IsA<{}>>", alias, set_bound.type_str); - format!(", {}: Option<&{}>", prop.var_name, alias) + generic_param = prop.bounds.to_generic_params_str(); + format!( + ", {}: {}", + prop.var_name, + set_bound.full_type_parameter_reference(prop.set_in_ref_mode, prop.nullable, false), + ) } else { - bound = String::new(); + generic_param = String::new(); let dir = library::ParameterDirection::In; let param_type = RustType::builder(env, prop.typ) .direction(dir) @@ -99,7 +100,7 @@ fn declaration(env: &Env, prop: &Property) -> String { .ref_mode(prop.set_in_ref_mode) .try_build_param() .into_string(); - format!(", {}: {}", prop.var_name, param_type) + format!(", {}: {param_type}", prop.var_name) }; let return_str = if prop.is_get { let dir = library::ParameterDirection::Return; @@ -114,8 +115,8 @@ fn declaration(env: &Env, prop: &Property) -> String { String::new() }; format!( - "fn {}{}(&self{}){}", - prop.func_name, bound, set_param, return_str + "fn {}{generic_param}(&self{set_param}){return_str}", + prop.func_name, ) } diff --git a/src/writer/to_code.rs b/src/writer/to_code.rs index 4b8608215..fd66e1e16 100644 --- a/src/writer/to_code.rs +++ b/src/writer/to_code.rs @@ -170,12 +170,12 @@ impl ToCode for Chunk { Name(ref name) => vec![name.clone()], ExternCFunc { ref name, - ref bounds, + ref generic_params, ref parameters, ref body, ref return_value, } => { - let prefix = format!(r#"unsafe extern "C" fn {name}{bounds}("#); + let prefix = format!(r#"unsafe extern "C" fn {name}{generic_params}("#); let suffix = ")".to_string(); let params: Vec<_> = parameters .iter()