From f7a9040f2757774409002238a8b68d5f0550ca0f Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Mon, 29 May 2023 13:50:28 +0100 Subject: [PATCH] cxx-qt-gen: add support for Result as a return type Closes #404 --- .../cxx-qt-gen/src/generator/cpp/inherit.rs | 18 +-- .../cxx-qt-gen/src/generator/cpp/invokable.rs | 14 +- .../src/generator/rust/invokable.rs | 1 + crates/cxx-qt-gen/src/generator/utils/cpp.rs | 131 ++++++++++++++++-- 4 files changed, 133 insertions(+), 31 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs index 2e87db580..880ec4623 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -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], @@ -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! { @@ -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 })); } diff --git a/crates/cxx-qt-gen/src/generator/cpp/invokable.rs b/crates/cxx-qt-gen/src/generator/cpp/invokable.rs index d398e4c16..8c8c3677b 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/invokable.rs @@ -11,6 +11,7 @@ use crate::{ types::CppType, }, naming::{invokable::QInvokableName, qobject::QObjectName}, + utils::cpp::syn_type_to_cpp_return_type, }, parser::{ cxxqtdata::ParsedCxxMappings, @@ -18,7 +19,7 @@ use crate::{ }, }; 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, @@ -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 = invokable .method @@ -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" }, @@ -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" }, diff --git a/crates/cxx-qt-gen/src/generator/rust/invokable.rs b/crates/cxx-qt-gen/src/generator/rust/invokable.rs index bbb0461c9..95ac64d48 100644 --- a/crates/cxx-qt-gen/src/generator/rust/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/rust/invokable.rs @@ -52,6 +52,7 @@ pub fn generate_rust_invokables( .collect::>(); 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! {} diff --git a/crates/cxx-qt-gen/src/generator/utils/cpp.rs b/crates/cxx-qt-gen/src/generator/utils/cpp.rs index d3ad4d5c3..9831ede99 100644 --- a/crates/cxx-qt-gen/src/generator/utils/cpp.rs +++ b/crates/cxx-qt-gen/src/generator/utils/cpp.rs @@ -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 +pub(crate) fn syn_type_to_cpp_return_type( + return_ty: &ReturnType, + cxx_mapping: &ParsedCxxMappings, +) -> Result> { + if let ReturnType::Type(_, ty) = return_ty { + // If we are a Result 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 @@ -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 is tricky, as we need Result in the CXX bridge - // but potentially Result 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:?}"), @@ -162,13 +198,21 @@ fn path_segment_to_string( let mut ident = segment.ident.to_string(); // If we are a Pin 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 @@ -554,4 +598,67 @@ mod tests { "::rust::Fn" ); } + + #[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 }; + 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 }; + 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 + ); + } }