Skip to content

Commit 24f0d66

Browse files
committed
Generate Into<Option<_>> in argument position where applicable
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.
1 parent 45aad8f commit 24f0d66

16 files changed

+587
-440
lines changed

src/analysis/bounds.rs

Lines changed: 258 additions & 181 deletions
Large diffs are not rendered by default.

src/analysis/child_properties.rs

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -113,33 +113,35 @@ fn analyze_property(
113113
}
114114
let nullable = library::Nullable(set_in_ref_mode.is_ref());
115115

116+
let mut bounds = Bounds::default();
116117
let mut bounds_str = String::new();
117118
let dir = ParameterDirection::In;
118-
let set_params = if Bounds::type_for(env, typ).is_some() {
119-
// TODO: bounds_str push?!?!
120-
bounds_str.push_str("TODO");
121-
format!("{prop_name}: {TYPE_PARAMETERS_START}")
122-
// let mut bounds = Bounds::default();
123-
// bounds.add_parameter("P", &r_type, bound, false);
124-
// let (s_bounds, _) = function::bounds(&bounds, &[], false);
125-
// // Because the bounds won't necessarily be added into the final
126-
// function, we // only keep the "inner" part to make
127-
// the string computation easier. So // `<T: X>` becomes
128-
// `T: X`. bounds_str.push_str(&s_bounds[1..s_bounds.
129-
// len() - 1]); format!("{}: {}", prop_name,
130-
// bounds.iter().last().unwrap().alias)
131-
} else {
132-
format!(
133-
"{}: {}",
134-
prop_name,
135-
RustType::builder(env, typ)
136-
.direction(dir)
137-
.nullable(nullable)
138-
.ref_mode(set_in_ref_mode)
139-
.try_build_param()
140-
.into_string()
141-
)
142-
};
119+
let set_params =
120+
if bounds.add_for_property_setter(env, typ, &prop.name, set_in_ref_mode, nullable) {
121+
// TODO: bounds_str push?!?!
122+
bounds_str.push_str("TODO");
123+
format!("{prop_name}: {TYPE_PARAMETERS_START}")
124+
// let mut bounds = Bounds::default();
125+
// bounds.add_parameter("P", &r_type, bound, false);
126+
// let (s_bounds, _) = function::bounds(&bounds, &[], false);
127+
// // Because the bounds won't necessarily be added into the final
128+
// function, we // only keep the "inner" part to make
129+
// the string computation easier. So // `<T: X>` becomes
130+
// `T: X`. bounds_str.push_str(&s_bounds[1..s_bounds.
131+
// len() - 1]); format!("{}: {}", prop_name,
132+
// bounds.iter().last().unwrap().alias)
133+
} else {
134+
format!(
135+
"{}: {}",
136+
prop_name,
137+
RustType::builder(env, typ)
138+
.direction(dir)
139+
.nullable(nullable)
140+
.ref_mode(set_in_ref_mode)
141+
.try_build_param()
142+
.into_string()
143+
)
144+
};
143145

144146
Some(ChildProperty {
145147
name,

src/analysis/class_builder.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,13 @@ fn analyze_property(
121121
}
122122
}
123123

124-
let (get_out_ref_mode, set_in_ref_mode, _) = get_property_ref_modes(env, prop);
124+
let (get_out_ref_mode, set_in_ref_mode, nullable) = get_property_ref_modes(env, prop);
125125

126126
let mut bounds = Bounds::default();
127-
if let Some(bound) = Bounds::type_for(env, prop.typ) {
127+
let set_has_bound =
128+
bounds.add_for_property_setter(env, prop.typ, &prop.name, set_in_ref_mode, nullable);
129+
if set_has_bound {
128130
imports.add("glib::prelude::*");
129-
bounds.add_parameter(&prop.name, &rust_type_res.into_string(), bound, false);
130131
}
131132

132133
Some(Property {
@@ -136,7 +137,7 @@ fn analyze_property(
136137
is_get: false,
137138
func_name: String::new(),
138139
func_name_alias: None,
139-
nullable: library::Nullable(false), // no Options for builder setters here
140+
nullable,
140141
get_out_ref_mode,
141142
set_in_ref_mode,
142143
bounds,

src/analysis/function_parameters.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -289,14 +289,16 @@ pub fn analyze(
289289
}
290290
if let Some(array_par) = array_par {
291291
let mut array_name = nameutil::mangle_keywords(&array_par.name);
292-
if let Some(bound_type) = Bounds::type_for(env, array_par.typ) {
293-
array_name = (array_name.into_owned()
294-
+ &bound_type.get_to_glib_extra(
295-
*array_par.nullable,
296-
array_par.instance_parameter,
297-
move_,
298-
))
299-
.into();
292+
if let Some(to_glib_extra) = Bounds::get_to_glib_extra_for(
293+
env,
294+
array_par.typ,
295+
if move_ { RefMode::None } else { RefMode::ByRef },
296+
Nullable(false),
297+
array_par.direction,
298+
array_par.instance_parameter,
299+
array_par.scope,
300+
) {
301+
array_name = (array_name.into_owned() + &to_glib_extra).into();
300302
}
301303

302304
add_rust_parameter = false;

src/analysis/functions.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,9 @@ fn analyze_callbacks(
390390
par.ref_mode
391391
},
392392
par.nullable,
393+
par.direction,
393394
par.instance_parameter,
395+
par.scope,
394396
),
395397
None,
396398
)

src/analysis/ref_mode.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,29 @@ impl RefMode {
128128
pub fn is_none(self) -> bool {
129129
matches!(self, Self::None)
130130
}
131+
132+
pub fn to_string_with_maybe_lt(self, lt: Option<char>) -> String {
133+
match self {
134+
RefMode::None | RefMode::ByRefFake => {
135+
assert!(lt.is_none(), "incompatible ref mode {self:?} with lifetime");
136+
String::new()
137+
}
138+
RefMode::ByRef | RefMode::ByRefImmut | RefMode::ByRefConst => {
139+
if let Some(lt) = lt {
140+
format!("&'{lt}")
141+
} else {
142+
"&".to_string()
143+
}
144+
}
145+
RefMode::ByRefMut => {
146+
if let Some(lt) = lt {
147+
format!("&'{lt} mut ")
148+
} else {
149+
"&mut ".to_string()
150+
}
151+
}
152+
}
153+
}
131154
}
132155

133156
impl fmt::Display for RefMode {

src/analysis/trampolines.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ pub struct Trampoline {
3737
pub user_data_index: usize,
3838
pub destroy_index: usize,
3939
pub nullable: library::Nullable,
40-
/// This field is used to give the type name when generating the "IsA<X>"
41-
/// part.
40+
/// This field is used to give the type name when generating the `IsA<X>` part.
4241
pub type_name: String,
4342
}
4443

@@ -81,9 +80,9 @@ pub fn analyze(
8180
if in_trait || fundamental_type {
8281
let type_name = RustType::builder(env, type_tid).try_build();
8382
if fundamental_type {
84-
bounds.add_parameter("this", &type_name.into_string(), BoundType::AsRef, false);
83+
bounds.add_for("this", &type_name.into_string(), BoundType::AsRef);
8584
} else {
86-
bounds.add_parameter("this", &type_name.into_string(), BoundType::IsA, false);
85+
bounds.add_for("this", &type_name.into_string(), BoundType::IsA);
8786
}
8887
}
8988

src/chunk/chunk.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ pub enum Chunk {
6767
Name(String),
6868
ExternCFunc {
6969
name: String,
70-
bounds: String,
70+
generic_params: String,
7171
parameters: Vec<Param>,
7272
body: Box<Chunk>,
7373
return_value: Option<String>,

src/codegen/bound.rs

Lines changed: 81 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -9,83 +9,108 @@ use crate::{
99
impl Bound {
1010
/// Returns the type parameter reference.
1111
/// Currently always returns the alias.
12-
pub(super) fn type_parameter_reference(&self) -> Option<char> {
13-
self.alias
12+
///
13+
/// This doesn't include the lifetime, which could be shared by other type
14+
/// parameters. Use [Bounds::to_generic_params_str](crate::analysis::bounds::Bounds::to_generic_params_str)
15+
/// to get the full generic parameter list, including lifetimes.
16+
pub fn type_parameter_definition(&self, r#async: bool) -> Option<String> {
17+
use BoundType::*;
18+
match self.bound_type {
19+
NoWrapper => {
20+
let alias = self.alias.expect("must be defined in this context");
21+
Some(format!("{alias}: {}", self.type_str))
22+
}
23+
IsA if self.alias.is_some() => Some(format!(
24+
"{}: IsA<{}>{}",
25+
self.alias.unwrap(),
26+
self.type_str,
27+
if r#async { " + Clone + 'static" } else { "" },
28+
)),
29+
IntoOptionIsA => {
30+
let alias = self.alias.expect("must be defined in this context");
31+
Some(format!(
32+
"{alias}: IsA<{}>{}",
33+
self.type_str,
34+
if r#async { " + Clone + 'static" } else { "" },
35+
))
36+
}
37+
_ => None,
38+
}
1439
}
1540

1641
/// Returns the type parameter reference, with [`BoundType::IsA`] wrapped
1742
/// in `ref_mode` and `nullable` as appropriate.
18-
pub(super) fn full_type_parameter_reference(
43+
pub fn full_type_parameter_reference(
1944
&self,
2045
ref_mode: RefMode,
2146
nullable: Nullable,
2247
r#async: bool,
2348
) -> String {
24-
let ref_str = ref_mode.to_string();
49+
use BoundType::*;
50+
match self.bound_type {
51+
NoWrapper => self
52+
.alias
53+
.expect("must be defined in this context")
54+
.to_string(),
55+
IsA if self.alias.is_none() => {
56+
let suffix = r#async
57+
.then(|| " + Clone + 'static".to_string())
58+
.unwrap_or_default();
2559

26-
// Generate `impl Trait` if this bound does not have an alias
27-
let trait_bound = match self.type_parameter_reference() {
28-
Some(t) => t.to_string(),
29-
None => {
30-
let trait_bound = self.trait_bound(r#async);
31-
let trait_bound = format!("impl {trait_bound}");
60+
let mut trait_bound = format!("impl IsA<{}>{suffix}", self.type_str);
3261

33-
// Combining a ref mode and lifetime requires parentheses for disambiguation
34-
match self.bound_type {
35-
BoundType::IsA => {
36-
// TODO: This is fragile
37-
let has_lifetime = r#async || self.lt.is_some();
62+
let ref_str = ref_mode.to_string();
63+
if !ref_str.is_empty() && r#async {
64+
trait_bound = format!("({trait_bound})");
65+
}
3866

39-
if !ref_str.is_empty() && has_lifetime {
40-
format!("({trait_bound})")
41-
} else {
42-
trait_bound
43-
}
44-
}
45-
_ => trait_bound,
67+
if *nullable {
68+
format!("Option<{ref_str}{trait_bound}>")
69+
} else {
70+
format!("{ref_str}{trait_bound}")
4671
}
4772
}
48-
};
49-
50-
match self.bound_type {
51-
BoundType::IsA if *nullable => {
52-
format!("Option<{ref_str}{trait_bound}>")
73+
IsA if self.alias.is_some() => {
74+
let alias = self.alias.unwrap();
75+
let ref_str = ref_mode.to_string();
76+
if *nullable {
77+
format!("Option<{ref_str} {alias}>")
78+
} else {
79+
format!("{ref_str} {alias}")
80+
}
5381
}
54-
BoundType::IsA => format!("{ref_str}{trait_bound}"),
55-
BoundType::AsRef if *nullable => {
56-
format!("Option<{trait_bound}>")
82+
IsA => {
83+
if *nullable {
84+
format!("Option<impl Isa<{}>>", self.type_str)
85+
} else {
86+
format!("impl IsA<{}>", self.type_str)
87+
}
5788
}
58-
BoundType::NoWrapper | BoundType::AsRef => trait_bound,
59-
}
60-
}
89+
AsRef => {
90+
assert!(self.lt.is_none(), "AsRef cannot have a lifetime");
6191

62-
/// Returns the type parameter definition for this bound, usually
63-
/// of the form `T: SomeTrait` or `T: IsA<Foo>`.
64-
pub(super) fn type_parameter_definition(&self, r#async: bool) -> Option<String> {
65-
self.alias
66-
.map(|alias| format!("{}: {}", alias, self.trait_bound(r#async)))
67-
}
68-
69-
/// Returns the trait bound, usually of the form `SomeTrait`
70-
/// or `IsA<Foo>`.
71-
pub(super) fn trait_bound(&self, r#async: bool) -> String {
72-
match self.bound_type {
73-
BoundType::NoWrapper => self.type_str.clone(),
74-
BoundType::IsA => {
75-
if r#async {
76-
assert!(self.lt.is_none(), "Async overwrites lifetime");
92+
if *nullable {
93+
format!("Option<impl AsRef<{}>>", self.type_str)
94+
} else {
95+
format!("impl AsRef<{}>", self.type_str)
7796
}
78-
let is_a = format!("IsA<{}>", self.type_str);
97+
}
98+
IntoOption => {
99+
format!("impl Into<Option<{}>>", self.type_str)
100+
}
101+
IntoOptionRef => {
102+
assert!(self.lt.is_some(), "must be defined in this context");
103+
let ref_str = ref_mode.to_string_with_maybe_lt(self.lt);
79104

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

85-
format!("{is_a}{lifetime}")
112+
format!("impl Into<Option<{ref_str} {alias}>>")
86113
}
87-
BoundType::AsRef if self.lt.is_some() => panic!("AsRef cannot have a lifetime"),
88-
BoundType::AsRef => format!("AsRef<{}>", self.type_str),
89114
}
90115
}
91116
}

src/codegen/bounds.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
use crate::analysis::bounds::Bounds;
2+
3+
impl Bounds {
4+
pub fn to_generic_params_str(&self) -> String {
5+
self.to_generic_params_str_(false)
6+
}
7+
8+
pub fn to_generic_params_str_async(&self) -> String {
9+
self.to_generic_params_str_(true)
10+
}
11+
12+
fn to_generic_params_str_(&self, r#async: bool) -> String {
13+
let mut res = String::new();
14+
15+
if self.lifetimes.is_empty() && self.used.iter().find_map(|b| b.alias).is_none() {
16+
return res;
17+
}
18+
19+
res.push('<');
20+
let mut is_first = true;
21+
22+
for lt in self.lifetimes.iter() {
23+
if is_first {
24+
is_first = false;
25+
} else {
26+
res.push_str(", ");
27+
}
28+
res.push('\'');
29+
res.push(*lt);
30+
}
31+
32+
for bound in self.used.iter() {
33+
if let Some(type_param_def) = bound.type_parameter_definition(r#async) {
34+
if is_first {
35+
is_first = false;
36+
} else {
37+
res.push_str(", ");
38+
}
39+
res.push_str(&type_param_def);
40+
}
41+
}
42+
res.push('>');
43+
44+
res
45+
}
46+
}

0 commit comments

Comments
 (0)