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.

This commit also moved the `Bound::alias` field to the elligible `BoundType`
variants and `BoundType` variants contain named fields instead of a tuple of
value:

* Only certain `BoundType`s actually used an alias.
* This make it clear whether the `char` refers to a lifetime or an alias.
  • Loading branch information
fengalin committed Dec 2, 2024
1 parent 7b437e8 commit d65f584
Show file tree
Hide file tree
Showing 16 changed files with 499 additions and 301 deletions.
278 changes: 204 additions & 74 deletions src/analysis/bounds.rs

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion src/analysis/child_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,17 @@ fn analyze_property(

let mut bounds_str = String::new();
let dir = ParameterDirection::In;
let set_params = if Bounds::type_for(env, typ).is_some() {
let set_params = if Bounds::type_for(
env,
typ,
set_in_ref_mode,
nullable,
dir,
false,
Default::default(),
)
.is_some()
{
// TODO: bounds_str push?!?!
bounds_str.push_str("TODO");
format!("{prop_name}: {TYPE_PARAMETERS_START}")
Expand Down
7 changes: 4 additions & 3 deletions src/analysis/class_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
},
config::{self, GObject},
env::Env,
library,
library::{self, Nullable},
traits::*,
};

Expand Down Expand Up @@ -124,9 +124,10 @@ fn analyze_property(
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) {
let set_has_bound =
bounds.add_for_property_setter(env, prop.typ, &prop.name, set_in_ref_mode, Nullable(false));
if set_has_bound {
imports.add("glib::prelude::*");
bounds.add_parameter(&prop.name, &rust_type_res.into_string(), bound, false);
}

Some(Property {
Expand Down
10 changes: 9 additions & 1 deletion src/analysis/function_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,15 @@ 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) {
if let Some(bound_type) = Bounds::type_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()
+ &bound_type.get_to_glib_extra(
*array_par.nullable,
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
8 changes: 4 additions & 4 deletions src/analysis/trampolines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,14 @@ pub fn analyze(
bounds.add_parameter(
"this",
&type_name.into_string(),
BoundType::AsRef(None),
BoundType::new_as_ref(),
false,
);
} else {
bounds.add_parameter(
"this",
&type_name.into_string(),
BoundType::IsA(None),
BoundType::new_is_a(),
false,
);
}
Expand Down Expand Up @@ -132,14 +132,14 @@ pub fn analyze(
bounds.add_parameter(
"this",
&type_name.into_string(),
BoundType::AsRef(None),
BoundType::new_as_ref(),
false,
);
} else {
bounds.add_parameter(
"this",
&type_name.into_string(),
BoundType::IsA(None),
BoundType::new_is_a(),
false,
);
}
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
124 changes: 66 additions & 58 deletions src/codegen/bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,83 +9,91 @@ 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::Bounds::to_generic_params_str)

Check warning on line 14 in src/codegen/bound.rs

View workflow job for this annotation

GitHub Actions / build-deploy

unresolved link to `crate::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 { alias: Some(alias) } => Some(format!("{alias}: {}", self.type_str)),
IsA { alias: Some(alias) } => Some(format!(
"{alias}: IsA<{}>{}",
self.type_str,
if r#async { " + Clone + 'static" } else { "" },
)),
IntoOptionIsA { alias, .. } => {
let alias = alias.expect("should 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();

// 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}");
use BoundType::*;
match self.bound_type {
NoWrapper { alias } => alias.unwrap().to_string(),
IsA { alias: None } => {
let suffix = r#async
.then(|| " + Clone + 'static".to_string())
.unwrap_or_default();

// Combining a ref mode and lifetime requires parentheses for disambiguation
match self.bound_type {
BoundType::IsA(lifetime) => {
// TODO: This is fragile
let has_lifetime = r#async || lifetime.is_some();
let mut trait_bound = format!("impl IsA<{}>{suffix}", self.type_str);

if !ref_str.is_empty() && has_lifetime {
format!("({trait_bound})")
} else {
trait_bound
}
}
_ => trait_bound,
let ref_str = ref_mode.to_string();
if !ref_str.is_empty() && r#async {
trait_bound = format!("({trait_bound})");
}
}
};

match self.bound_type {
BoundType::IsA(_) if *nullable => {
format!("Option<{ref_str}{trait_bound}>")
if *nullable {
format!("Option<{ref_str}{trait_bound}>")
} else {
format!("{ref_str}{trait_bound}")
}
}
BoundType::IsA(_) => format!("{ref_str}{trait_bound}"),
BoundType::AsRef(_) if *nullable => {
format!("Option<{trait_bound}>")
IsA { alias: Some(alias) } => {
let ref_str = ref_mode.to_string();
if *nullable {
format!("Option<{ref_str} {alias}>")
} else {
format!("{ref_str} {alias}")
}
}
BoundType::NoWrapper | BoundType::AsRef(_) => trait_bound,
}
}

/// 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(lifetime) => {
if r#async {
assert!(lifetime.is_none(), "Async overwrites lifetime");
AsRef => {
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 { lt } => {
assert!(lt.is_some(), "must be defined in this context");
let ref_str = ref_mode.to_string_with_maybe_lt(lt);

let lifetime = r#async
.then(|| " + Clone + 'static".to_string())
.or_else(|| lifetime.map(|l| format!(" + '{l}")))
.unwrap_or_default();
format!("impl Into<Option<{ref_str} {}>>", self.type_str)
}
IntoOptionIsA { lt, alias } => {
assert!(lt.is_some(), "must be defined in this context");
let ref_str = ref_mode.to_string_with_maybe_lt(lt);
let alias = alias.expect("must be defined in this context");

format!("{is_a}{lifetime}")
format!("impl Into<Option<{ref_str} {alias}>>")
}
BoundType::AsRef(Some(_ /* lifetime */)) => panic!("AsRef cannot have a lifetime"),
BoundType::AsRef(None) => 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 d65f584

Please sign in to comment.