Skip to content

Commit

Permalink
Generate Into<Option<_>> in argument position where applicable
Browse files Browse the repository at this point in the history
This commit adds support for generating `Into<Option<_>>`, `Into<Option<&_>>`
and `Into<Option<&IsA<_>>` 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<Option<gst::Buffer>>) -> 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<Option<&'a str>>) {
        [...]
        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<Clock>>(&self, clock: impl Into<Option<&'a P>>) {
        [...]
        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.
  • Loading branch information
fengalin committed Dec 5, 2024
1 parent 45aad8f commit 74a0115
Show file tree
Hide file tree
Showing 16 changed files with 588 additions and 440 deletions.
439 changes: 258 additions & 181 deletions src/analysis/bounds.rs

Large diffs are not rendered by default.

52 changes: 27 additions & 25 deletions src/analysis/child_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 // `<T: X>` 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 // `<T: X>` 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,
Expand Down
9 changes: 5 additions & 4 deletions src/analysis/class_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
18 changes: 10 additions & 8 deletions src/analysis/function_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/analysis/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,9 @@ fn analyze_callbacks(
par.ref_mode
},
par.nullable,
par.direction,
par.instance_parameter,
par.scope,
),
None,
)
Expand Down
23 changes: 23 additions & 0 deletions src/analysis/ref_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<char>) -> 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 {
Expand Down
7 changes: 3 additions & 4 deletions src/analysis/trampolines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<X>"
/// part.
/// This field is used to give the type name when generating the `IsA<X>` part.
pub type_name: String,
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/chunk/chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub enum Chunk {
Name(String),
ExternCFunc {
name: String,
bounds: String,
generic_params: String,
parameters: Vec<Param>,
body: Box<Chunk>,
return_value: Option<String>,
Expand Down
137 changes: 81 additions & 56 deletions src/codegen/bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<char> {
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<String> {
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<impl Isa<{}>>", 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<Foo>`.
pub(super) fn type_parameter_definition(&self, r#async: bool) -> Option<String> {
self.alias
.map(|alias| format!("{}: {}", alias, self.trait_bound(r#async)))
}

/// Returns the trait bound, usually of the form `SomeTrait`
/// or `IsA<Foo>`.
pub(super) fn trait_bound(&self, r#async: bool) -> String {
match self.bound_type {
BoundType::NoWrapper => self.type_str.clone(),
BoundType::IsA => {
if r#async {
assert!(self.lt.is_none(), "Async overwrites lifetime");
if *nullable {
format!("Option<impl AsRef<{}>>", self.type_str)
} else {
format!("impl AsRef<{}>", self.type_str)
}
let is_a = format!("IsA<{}>", self.type_str);
}
IntoOption => {
format!("impl Into<Option<{}>>", 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<Option<{ref_str} {}>>", 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<Option<{ref_str} {alias}>>")
}
BoundType::AsRef if self.lt.is_some() => panic!("AsRef cannot have a lifetime"),
BoundType::AsRef => format!("AsRef<{}>", self.type_str),
}
}
}
46 changes: 46 additions & 0 deletions src/codegen/bounds.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
Loading

0 comments on commit 74a0115

Please sign in to comment.