Skip to content

Commit

Permalink
cxx-qt-gen: add support for Result<T> as a return type
Browse files Browse the repository at this point in the history
Closes KDAB#404
  • Loading branch information
ahayzen-kdab committed Jul 19, 2023
1 parent 63b6b1f commit f7a9040
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 31 deletions.
18 changes: 7 additions & 11 deletions crates/cxx-qt-gen/src/generator/cpp/inherit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
use indoc::formatdoc;

use crate::{
generator::cpp::{fragment::CppFragment, qobject::GeneratedCppQObjectBlocks},
generator::{
cpp::{fragment::CppFragment, qobject::GeneratedCppQObjectBlocks},
utils::cpp::syn_type_to_cpp_return_type,
},
parser::{cxxqtdata::ParsedCxxMappings, inherit::ParsedInheritedMethod},
};

use syn::{Result, ReturnType};

use super::types::CppType;
use syn::Result;

pub fn generate(
inherited_methods: &[ParsedInheritedMethod],
Expand All @@ -21,12 +22,7 @@ pub fn generate(
let mut result = GeneratedCppQObjectBlocks::default();

for method in inherited_methods {
let return_type = if let ReturnType::Type(_, ty) = &method.method.sig.output {
CppType::from(ty, cxx_mappings)?.as_cxx_ty().to_owned()
} else {
"void".to_owned()
};

let return_type = syn_type_to_cpp_return_type(&method.method.sig.output, cxx_mappings)?;
let base_class = base_class.as_deref().unwrap_or("QObject");

result.methods.push(CppFragment::Header(formatdoc! {
Expand All @@ -39,7 +35,7 @@ pub fn generate(
mutability = if method.mutable { "" } else { " const" },
func_ident = method.ident.cpp,
wrapper_ident = method.wrapper_ident(),
return_type = return_type,
return_type = return_type.unwrap_or_else(|| "void".to_string()),
base_class = base_class
}));
}
Expand Down
14 changes: 6 additions & 8 deletions crates/cxx-qt-gen/src/generator/cpp/invokable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ use crate::{
types::CppType,
},
naming::{invokable::QInvokableName, qobject::QObjectName},
utils::cpp::syn_type_to_cpp_return_type,
},
parser::{
cxxqtdata::ParsedCxxMappings,
invokable::{ParsedQInvokable, ParsedQInvokableSpecifiers},
},
};
use indoc::formatdoc;
use syn::{spanned::Spanned, Error, FnArg, Pat, PatIdent, PatType, Result, ReturnType};
use syn::{spanned::Spanned, Error, FnArg, Pat, PatIdent, PatType, Result};

pub fn generate_cpp_invokables(
invokables: &Vec<ParsedQInvokable>,
Expand All @@ -30,11 +31,8 @@ pub fn generate_cpp_invokables(
let qobject_ident = qobject_idents.cpp_class.cpp.to_string();
for invokable in invokables {
let idents = QInvokableName::from(invokable);
let return_cxx_ty = if let ReturnType::Type(_, ty) = &invokable.method.sig.output {
Some(CppType::from(ty, cxx_mappings)?)
} else {
None
};
let return_cxx_ty =
syn_type_to_cpp_return_type(&invokable.method.sig.output, cxx_mappings)?;

let parameters: Vec<CppNamedType> = invokable
.method
Expand Down Expand Up @@ -92,7 +90,7 @@ pub fn generate_cpp_invokables(
header: format!(
"Q_INVOKABLE {is_virtual}{return_cxx_ty} {ident}({parameter_types}){is_const}{is_final}{is_override};",
return_cxx_ty = if let Some(return_cxx_ty) = &return_cxx_ty {
return_cxx_ty.as_cxx_ty()
return_cxx_ty
} else {
"void"
},
Expand Down Expand Up @@ -124,7 +122,7 @@ pub fn generate_cpp_invokables(
}}
"#,
return_cxx_ty = if let Some(return_cxx_ty) = &return_cxx_ty {
return_cxx_ty.as_cxx_ty()
return_cxx_ty
} else {
"void"
},
Expand Down
1 change: 1 addition & 0 deletions crates/cxx-qt-gen/src/generator/rust/invokable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub fn generate_rust_invokables(
.collect::<Vec<TokenStream>>();
quote! { self: #rust_struct, cpp: #cpp_struct, #(#parameters),* }
};

let return_type = &invokable.method.sig.output;
let has_return = if matches!(invokable.method.sig.output, ReturnType::Default) {
quote! {}
Expand Down
131 changes: 119 additions & 12 deletions crates/cxx-qt-gen/src/generator/utils/cpp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,47 @@ use syn::{
ReturnType, Type, TypeArray, TypeBareFn, TypePtr, TypeReference, TypeSlice,
};

/// For a given Rust return type attempt to generate a C++ string
///
/// Note that return types are allowed to have a Result<T>
pub(crate) fn syn_type_to_cpp_return_type(
return_ty: &ReturnType,
cxx_mapping: &ParsedCxxMappings,
) -> Result<Option<String>> {
if let ReturnType::Type(_, ty) = return_ty {
// If we are a Result<T> then we just become T for C++
if let Type::Path(ty_path) = &**ty {
if let Some(segment) = ty_path.path.segments.first() {
if segment.ident == "Result" {
let mut args = path_argument_to_string(&segment.arguments, cxx_mapping)?
.unwrap_or_default();
if args.len() != 1 {
return Err(Error::new(
return_ty.span(),
"Result must have one argument",
));
}

if let Some(arg) = args.pop() {
// Map void to None
if arg == "void" {
return Ok(None);
}

return Ok(Some(arg));
} else {
unreachable!("Args should be of length 1");
}
}
}
}

syn_type_to_cpp_type(ty, cxx_mapping).map(|v| if v == "void" { None } else { Some(v) })
} else {
Ok(None)
}
}

/// For a given Rust type attempt to generate a C++ string
///
/// This is similar to the parsing in CXX
Expand Down Expand Up @@ -106,12 +147,7 @@ pub(crate) fn syn_type_to_cpp_type(ty: &Type, cxx_mapping: &ParsedCxxMappings) -
)),
}
}
// TODO: consider Type::Tuple with an empty tuple mapping to void
//
// TODO: handling Result<T> is tricky, as we need Result<T> in the CXX bridge
// but potentially Result<T, E> on the method. Then we need to detect that we have
// Result<()> and notice that that is a void return, otherwise we try to convert
// void which fails in C++
Type::Tuple(tuple) if tuple.elems.is_empty() => Ok("void".to_string()),
_others => Err(Error::new(
ty.span(),
format!("Unsupported type: {_others:?}"),
Expand Down Expand Up @@ -162,13 +198,21 @@ fn path_segment_to_string(
let mut ident = segment.ident.to_string();

// If we are a Pin<T> then for C++ it becomes just T
let args = if ident == "Pin" {
ident = path_argument_to_string(&segment.arguments, cxx_mapping)?
.map_or_else(|| "".to_owned(), |values| values.join(", "));
let args = match ident.as_str() {
"Pin" => {
ident = path_argument_to_string(&segment.arguments, cxx_mapping)?
.map_or_else(|| "".to_owned(), |values| values.join(", "));

None
} else {
path_argument_to_string(&segment.arguments, cxx_mapping)?.map(|values| values.join(", "))
None
}
"Result" => {
return Err(Error::new(segment.span(), "Result is not supported"));
}
"Option" => {
return Err(Error::new(segment.span(), "Option is not supported"));
}
_others => path_argument_to_string(&segment.arguments, cxx_mapping)?
.map(|values| values.join(", ")),
};

// If there are template args check that we aren't a recognised A of A<B>
Expand Down Expand Up @@ -554,4 +598,67 @@ mod tests {
"::rust::Fn<void, ()>"
);
}

#[test]
fn test_syn_type_to_cpp_type_tuple_empty() {
let ty = parse_quote! { () };
assert_eq!(
syn_type_to_cpp_type(&ty, &ParsedCxxMappings::default()).unwrap(),
"void"
);
}

#[test]
fn test_syn_type_to_cpp_type_tuple_multiple() {
let ty = parse_quote! { (i32, ) };
assert!(syn_type_to_cpp_type(&ty, &ParsedCxxMappings::default()).is_err());
}

#[test]
fn test_syn_type_to_cpp_return_type_none() {
let ty = parse_quote! {};
assert_eq!(
syn_type_to_cpp_return_type(&ty, &ParsedCxxMappings::default()).unwrap(),
None
);
}

#[test]
fn test_syn_type_to_cpp_return_type_normal() {
let ty = parse_quote! { -> bool };
assert_eq!(
syn_type_to_cpp_return_type(&ty, &ParsedCxxMappings::default()).unwrap(),
Some("bool".to_string())
);
}

#[test]
fn test_syn_type_to_cpp_return_type_result_bool() {
let ty = parse_quote! { -> Result<bool> };
assert_eq!(
syn_type_to_cpp_return_type(&ty, &ParsedCxxMappings::default()).unwrap(),
Some("bool".to_string())
);
}

#[test]
fn test_syn_type_to_cpp_return_type_result_empty() {
let ty = parse_quote! { -> Result<> };
assert!(syn_type_to_cpp_return_type(&ty, &ParsedCxxMappings::default()).is_err());
}

#[test]
fn test_syn_type_to_cpp_return_type_result_multiple() {
let ty = parse_quote! { -> Result<A, B, C> };
assert!(syn_type_to_cpp_return_type(&ty, &ParsedCxxMappings::default()).is_err());
}

#[test]
fn test_syn_type_to_cpp_return_type_result_tuple() {
let ty = parse_quote! { -> Result<()> };
assert_eq!(
syn_type_to_cpp_return_type(&ty, &ParsedCxxMappings::default()).unwrap(),
None
);
}
}

0 comments on commit f7a9040

Please sign in to comment.