Skip to content

Commit

Permalink
Merge pull request #1411 from gtk-rs/bilelmoussaoui/gio-infaliable-fu…
Browse files Browse the repository at this point in the history
…ture

Backport: codegen: Handle _finish functions not taking a GError inout param
  • Loading branch information
sdroege committed Dec 9, 2022
2 parents e79806a + 80812bc commit 9296514
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 42 deletions.
30 changes: 18 additions & 12 deletions src/analysis/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl Default for Bounds {
pub struct CallbackInfo {
pub callback_type: String,
pub success_parameters: String,
pub error_parameters: String,
pub error_parameters: Option<String>,
pub bound_name: char,
}

Expand Down Expand Up @@ -134,8 +134,12 @@ impl Bounds {
}
let parameters = format_out_parameters(&out_parameters);
let error_type = find_error_type(env, function);
type_string =
format!("FnOnce(Result<{}, {}>) + 'static", parameters, error_type);
if let Some(ref error) = error_type {
type_string =
format!("FnOnce(Result<{}, {}>) + 'static", parameters, error);
} else {
type_string = format!("FnOnce({}) + 'static", parameters);
}
let bound_name = *self.unused.front().unwrap();
callback_info = Some(CallbackInfo {
callback_type: type_string.clone(),
Expand Down Expand Up @@ -172,7 +176,7 @@ impl Bounds {
callback_info = Some(CallbackInfo {
callback_type: type_string.clone(),
success_parameters: String::new(),
error_parameters: String::new(),
error_parameters: None,
bound_name,
});
}
Expand Down Expand Up @@ -355,19 +359,21 @@ fn format_out_parameters(parameters: &[String]) -> String {
}
}

fn find_error_type(env: &Env, function: &Function) -> String {
fn find_error_type(env: &Env, function: &Function) -> Option<String> {
let error_param = function
.parameters
.iter()
.find(|param| param.direction == ParameterDirection::Out && param.name == "error")
.expect("error type");
.find(|param| param.direction.is_out() && param.is_error)?;
if let Type::Record(_) = env.type_(error_param.typ) {
return RustType::builder(env, error_param.typ)
.direction(error_param.direction)
.try_build()
.into_string();
return Some(
RustType::builder(env, error_param.typ)
.direction(error_param.direction)
.try_build()
.into_string(),
);
} else {
None
}
panic!("cannot find error type")
}

#[cfg(test)]
Expand Down
12 changes: 4 additions & 8 deletions src/analysis/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use super::{namespaces::NsId, special_functions};
#[derive(Clone, Debug)]
pub struct AsyncTrampoline {
pub is_method: bool,
pub has_error_parameter: bool,
pub name: String,
pub finish_func_name: String,
pub callback_type: String,
Expand All @@ -55,7 +56,7 @@ pub struct AsyncFuture {
pub is_method: bool,
pub name: String,
pub success_parameters: String,
pub error_parameters: String,
pub error_parameters: Option<String>,
pub assertion: SafetyAssertionMode,
}

Expand Down Expand Up @@ -1004,19 +1005,13 @@ fn analyze_async(
*commented = true;
return false;
}
if !*commented && (success_parameters.is_empty() || error_parameters.is_empty()) {
if !*commented && success_parameters.is_empty() {
if success_parameters.is_empty() {
warn_main!(
type_tid,
"{}: missing success parameters for async future",
func.name
);
} else if error_parameters.is_empty() {
warn_main!(
type_tid,
"{}: missing error parameters for async future",
func.name
);
}
*commented = true;
return false;
Expand All @@ -1025,6 +1020,7 @@ fn analyze_async(

*trampoline = Some(AsyncTrampoline {
is_method,
has_error_parameter: error_parameters.is_some(),
name: format!("{}_trampoline", codegen_name),
finish_func_name: format!("{}::{}", env.main_sys_crate_name(), finish_func_name),
callback_type,
Expand Down
30 changes: 24 additions & 6 deletions src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,17 @@ pub fn declaration(env: &Env, analysis: &analysis::functions::Info) -> String {
pub fn declaration_futures(env: &Env, analysis: &analysis::functions::Info) -> String {
let async_future = analysis.async_future.as_ref().unwrap();

let return_str = format!(
" -> Pin<Box_<dyn std::future::Future<Output = Result<{}, {}>> + 'static>>",
async_future.success_parameters, async_future.error_parameters
);
let return_str = if let Some(ref error_parameters) = async_future.error_parameters {
format!(
" -> Pin<Box_<dyn std::future::Future<Output = Result<{}, {}>> + 'static>>",
async_future.success_parameters, error_parameters
)
} else {
format!(
" -> Pin<Box_<dyn std::future::Future<Output = {}> + 'static>>",
async_future.success_parameters
)
};

let mut param_str = String::with_capacity(100);

Expand Down Expand Up @@ -402,10 +409,21 @@ pub fn body_chunk_futures(

let mut body = String::new();

let future_name = analysis
.async_future
.as_ref()
.map(|a| {
if a.error_parameters.is_some() {
"GioFuture"
} else {
"GioInfallibleFuture"
}
})
.unwrap_or("GioFuture");
let gio_future_name = if env.config.library_name != "Gio" {
"gio::GioFuture"
format!("gio::{future_name}")
} else {
"crate::GioFuture"
format!("crate::{future_name}")
};
writeln!(body)?;

Expand Down
49 changes: 33 additions & 16 deletions src/codegen/function_body_chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -848,14 +848,25 @@ impl Builder {
);
}
}
// If the trampoline doesn't have a GError parameter
let has_error_parameter = self
.async_trampoline
.as_ref()
.map(|a| a.has_error_parameter)
.unwrap_or_default();

let result = Chunk::Tuple(result, TupleMode::WithUnit);
let mut body = vec![Chunk::Let {
name: "error".to_string(),
is_mut: true,
value: Box::new(Chunk::NullMutPtr),
type_: None,
}];

let mut body = if has_error_parameter {
vec![Chunk::Let {
name: "error".to_string(),
is_mut: true,
value: Box::new(Chunk::NullMutPtr),
type_: None,
}]
} else {
vec![]
};
let output_vars = trampoline
.output_params
.iter()
Expand All @@ -871,7 +882,11 @@ impl Builder {
body.extend(output_vars);

let ret_name = if trampoline.ffi_ret.is_some() {
"ret"
if has_error_parameter {
"ret"
} else {
"result" // Needed as in case of an error param we would have let result = if error.is_null() { Ok()} else { Err()};
}
} else {
"_"
};
Expand All @@ -885,15 +900,17 @@ impl Builder {
}),
type_: None,
});
body.push(Chunk::Let {
name: "result".to_string(),
is_mut: false,
value: Box::new(Chunk::ErrorResultReturn {
ret: None,
value: Box::new(result),
}),
type_: None,
});
if has_error_parameter {
body.push(Chunk::Let {
name: "result".to_string(),
is_mut: false,
value: Box::new(Chunk::ErrorResultReturn {
ret: None,
value: Box::new(result),
}),
type_: None,
});
}
body.push(Chunk::Let {
name: "callback".to_string(),
is_mut: false,
Expand Down

0 comments on commit 9296514

Please sign in to comment.