From 867386412bc765434818dd587cc31732592f8f37 Mon Sep 17 00:00:00 2001 From: Anders Musikka Date: Sat, 12 Oct 2024 16:35:49 +0200 Subject: [PATCH] Support boxed dyn trait in return position --- Cargo.lock | 6 +- README.md | 10 ++ savefile-abi/Cargo.toml | 6 +- savefile-derive/Cargo.toml | 2 +- savefile-derive/src/lib.rs | 5 +- savefile-derive/src/savefile_abi.rs | 158 +++++++++++++++++- savefile-test/Cargo.toml | 2 +- .../advanced_datatypes_test.rs | 34 +++- savefile/Cargo.toml | 6 +- savefile/src/lib.rs | 33 ++++ savefile/src/prelude.rs | 2 +- 11 files changed, 243 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 199f947..12699b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -588,7 +588,7 @@ dependencies = [ [[package]] name = "savefile" -version = "0.17.11" +version = "0.17.12" dependencies = [ "arrayvec", "bit-set 0.5.3", @@ -614,7 +614,7 @@ dependencies = [ [[package]] name = "savefile-abi" -version = "0.17.11" +version = "0.17.12" dependencies = [ "byteorder", "libloading", @@ -655,7 +655,7 @@ dependencies = [ [[package]] name = "savefile-derive" -version = "0.17.11" +version = "0.17.12" dependencies = [ "proc-macro-error2", "proc-macro2", diff --git a/README.md b/README.md index e35d6de..df4aef6 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,16 @@ See the docs for more information, including schema-versioning: https://docs.rs/ # Changelog +## 0.17.12 + +Make savefile-abi support Result containing boxed dyn traits in method call return position. +I.e, this signature is now possible with savefile-abi: +``` + fn return_boxed_closure_result(&self) -> Result u32>,()>; +``` +This is a special case, dyn traits are still not possible in arbitrary positions. This particular +case is important in order to be able to have fallible functions returning boxed dyn traits. + ## 0.17.11 Support Sync + Send-bounds for dyn Fn parameters, in savefile-abi. Also, reduce diff --git a/savefile-abi/Cargo.toml b/savefile-abi/Cargo.toml index 26745cf..a8d3a93 100644 --- a/savefile-abi/Cargo.toml +++ b/savefile-abi/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "savefile-abi" -version = "0.17.11" +version = "0.17.12" edition = "2021" authors = ["Anders Musikka "] documentation = "https://docs.rs/savefile-abi/" @@ -17,8 +17,8 @@ keywords = ["dylib", "dlopen", "ffi"] license = "MIT/Apache-2.0" [dependencies] -savefile = { path="../savefile", version = "=0.17.11" } -savefile-derive = { path="../savefile-derive", version = "=0.17.11" } +savefile = { path="../savefile", version = "=0.17.12" } +savefile-derive = { path="../savefile-derive", version = "=0.17.12" } byteorder = "1.4" libloading = "0.8" diff --git a/savefile-derive/Cargo.toml b/savefile-derive/Cargo.toml index b1eb08b..85a5862 100644 --- a/savefile-derive/Cargo.toml +++ b/savefile-derive/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "savefile-derive" -version = "0.17.11" +version = "0.17.12" authors = ["Anders Musikka "] repository = "https://github.com/avl/savefile" rust-version = "1.74" diff --git a/savefile-derive/src/lib.rs b/savefile-derive/src/lib.rs index c77c4e3..856cd6c 100644 --- a/savefile-derive/src/lib.rs +++ b/savefile-derive/src/lib.rs @@ -254,7 +254,7 @@ pub fn savefile_abi_exportable( let uses = quote_spanned! { defspan => extern crate savefile; extern crate savefile_abi; - use savefile::prelude::{Packed, Schema, SchemaPrimitive, WithSchema, WithSchemaContext, get_schema, Serializer, Serialize, Deserializer, Deserialize, SavefileError, deserialize_slice_as_vec, ReadBytesExt,LittleEndian,AbiMethodArgument, AbiMethod, AbiMethodInfo,AbiTraitDefinition}; + use savefile::prelude::{Packed, Schema, SchemaPrimitive, WithSchema, WithSchemaContext, get_schema, get_result_schema, Serializer, Serialize, Deserializer, Deserialize, SavefileError, deserialize_slice_as_vec, ReadBytesExt,LittleEndian,AbiMethodArgument, AbiMethod, AbiMethodInfo,AbiTraitDefinition}; use savefile_abi::{parse_return_value_impl,abi_result_receiver,abi_boxed_trait_receiver, FlexBuffer, AbiExportable, TraitObject, PackagedTraitObject, Owning, AbiErrorMsg, RawAbiCallResult, AbiConnection, AbiConnectionMethod, AbiProtocol, abi_entry_light}; use std::collections::HashMap; use std::mem::MaybeUninit; @@ -508,7 +508,7 @@ pub fn savefile_abi_exportable( #version } - fn call(trait_object: TraitObject, method_number: u16, effective_version:u32, compatibility_mask: u64, data: &[u8], abi_result: *mut (), receiver: unsafe extern "C" fn(outcome: *const RawAbiCallResult, result_receiver: *mut ()/*Result>*/)) -> Result<(),SavefileError> { + fn call(trait_object: TraitObject, method_number: u16, effective_version:u32, compatibility_mask: u64, data: &[u8], abi_result: *mut (), __savefile_internal_receiver: unsafe extern "C" fn(outcome: *const RawAbiCallResult, result_receiver: *mut ()/*Result>*/)) -> Result<(),SavefileError> { let mut cursor = Cursor::new(data); @@ -539,6 +539,7 @@ pub fn savefile_abi_exportable( let extra_definitions: Vec<_> = extra_definitions.values().map(|(_, x)| x).collect(); let expanded = quote! { #[allow(clippy::double_comparisons)] + #[allow(unused_variables)] #[allow(clippy::needless_late_init)] #[allow(clippy::not_unsafe_ptr_arg_deref)] #[allow(non_upper_case_globals)] diff --git a/savefile-derive/src/savefile_abi.rs b/savefile-derive/src/savefile_abi.rs index f17088b..924d544 100644 --- a/savefile-derive/src/savefile_abi.rs +++ b/savefile-derive/src/savefile_abi.rs @@ -134,12 +134,14 @@ fn emit_closure_helpers( return names; } +#[derive(Debug)] pub(crate) enum ArgType { PlainData(Type), - Reference(Box, bool /*ismut (only traits objects can be mut here)*/), + Reference(Box, bool /*ismut (only trait objects can be mut here)*/), Str(bool /*static*/), Boxed(Box), Slice(Box), + Result(Box, Box), Trait(Ident, bool /*ismut self*/), Fn( TokenStream, /*full closure definition (e.g "Fn(u32)->u16")*/ @@ -207,6 +209,14 @@ pub(crate) fn parse_box_type( true, is_mut_ref, ) { + ArgType::Result(_,_) => { + abort!( + first_gen_arg.span(), + "{}. Savefile does not support boxed results. Try boxing the contents of the result instead. I.e, instead of Box>, try Result,Box>. Type encountered: {}", + location, + typ.to_token_stream() + ) + } ArgType::Boxed(_) => { abort!( first_gen_arg.span(), @@ -335,7 +345,7 @@ fn parse_type( if is_mut_ref { abort!( typ.span(), - "{}: Mutable refernces are not supported by savefile-abi, except for FnMut-trait objects. {}", + "{}: Mutable references are not supported by savefile-abi, except for FnMut-trait objects. {}", location, typ.to_token_stream() ); @@ -400,7 +410,7 @@ fn parse_type( if bound.ident == "FnOnce" { abort!( bound.ident.span(), - "{}, FnOnce is not supported. Maybe you can use FnMut instead?", + "{}, FnOnce is presently not supported by savefile-abi. Maybe you can use FnMut instead?", location, ); } @@ -470,6 +480,62 @@ fn parse_type( is_reference, is_mut_ref, ); + } else if last_seg.ident == "Result" && is_return_value { + if path.path.segments.len() != 1 { + abort!(path.path.segments.span(), "Savefile does not support types named 'Result', unless they are the standard type Result, and it must be specified as 'Result', without any namespace"); + } + if is_reference { + abort!(last_seg.ident.span(), "Savefile does not presently support reference to Result in return position. Consider removing the '&'."); + } + + match &last_seg.arguments { + PathArguments::Parenthesized(_) | + PathArguments::None => { + abort!(last_seg.arguments.span(), "Savefile does not support types named 'Result', unless they are the standard type Result, and it must be specified as 'Result', with two type parameters within angle-brackets. Found not type arguments."); + } + PathArguments::AngleBracketed(params) => { + let argvec: Vec<_> = params.args.iter().collect(); + if argvec.len() != 2 { + abort!(last_seg.arguments.span(), "Savefile does not support types named 'Result', unless they are the standard type Result, and it must be specified as 'Result', with two type parameters within angle-brackets. Got {} type arguments", argvec.len()); + } + let mut argtypes = vec![]; + for arg in argvec { + match arg { + GenericArgument::Type(argtyp) => { + argtypes.push(Box::new( + parse_type( + version, + arg_name, + argtyp, + method_name, + is_return_value, + &mut *name_generator, + extra_definitions, + is_reference, + is_mut_ref, + ))); + } + GenericArgument::Lifetime(_) => { + abort!(arg.span(), "Savefile does not support lifetime specifications."); + } + GenericArgument::Const(_) => { + abort!(arg.span(), "Savefile does not support const in this location."); + } + GenericArgument::Binding(_) => { + abort!(arg.span(), "Savefile does not support the syntax expressed here."); + } + GenericArgument::Constraint(_) => { + abort!(arg.span(), "Savefile does not support constraints at this position."); + } + } + } + let mut i = argtypes.into_iter(); + let oktype = i.next().unwrap(); + + let errtype = i.next().unwrap(); + return ArgType::Result(oktype, errtype); + } + } } else { rawtype = typ; } @@ -528,7 +594,7 @@ impl ArgType { version: u32, arg_index: Option, arg_orig_name: &str, - arg_name: &TokenStream, + arg_name: &TokenStream, //always just 'arg_orig_name' nesting_level: u32, take_ownership: bool, extra_definitions: &mut HashMap, @@ -583,6 +649,7 @@ impl ArgType { ArgType::Str(_) => None, ArgType::Reference(..) => None, ArgType::Slice(_) => None, + ArgType::Result(_,_) => None, }; let (mutsymbol, read_raw_ptr) = if *is_mut { @@ -888,6 +955,85 @@ impl ArgType { known_size_align_of_pointer1: None, } } + ArgType::Result(ok_type, err_type) => { + let ok_instruction = ok_type.get_instruction( + version, + arg_index, + "okval", + "e!( okval ), + nesting_level + 1, + true, + extra_definitions, + ); + let err_instruction = err_type.get_instruction( + version, + arg_index, + "errval", + "e!( errval ), + nesting_level + 1, + true, + extra_definitions, + ); + + let TypeInstruction { + callee_trampoline_variable_deserializer1: ok_callee_trampoline_variable_deserializer1, + caller_arg_serializer1: ok_caller_arg_serializer1, + deserialized_type: ok_deserialized_type, + callee_trampoline_temp_variable_declaration1: ok_callee_trampoline_temp_variable_declaration1, + caller_arg_serializer_temp1: ok_caller_arg_serializer_temp1, + schema: ok_schema, + .. + } = ok_instruction; + + let TypeInstruction { + callee_trampoline_variable_deserializer1: err_callee_trampoline_variable_deserializer1, + caller_arg_serializer1: err_caller_arg_serializer1, + deserialized_type: err_deserialized_type, + callee_trampoline_temp_variable_declaration1: err_callee_trampoline_temp_variable_declaration1, + caller_arg_serializer_temp1: err_caller_arg_serializer_temp1, + schema: err_schema, + .. + } = err_instruction; + + TypeInstruction { + deserialized_type: quote! { Result<#ok_deserialized_type, #err_deserialized_type> }, + callee_trampoline_temp_variable_declaration1: quote! { + #ok_callee_trampoline_temp_variable_declaration1; + #err_callee_trampoline_temp_variable_declaration1; + }, + callee_trampoline_variable_deserializer1: quote! { + if deserializer.read_bool()? { + Ok(#ok_callee_trampoline_variable_deserializer1) + } else { + Err(#err_callee_trampoline_variable_deserializer1) + } + }, + caller_arg_serializer_temp1: quote! { + #ok_caller_arg_serializer_temp1; + #err_caller_arg_serializer_temp1; + }, + caller_arg_serializer1: quote! { + match #arg_name { + Ok(okval) => { + serializer.write_bool(true)?; + #ok_caller_arg_serializer1 + }, + Err(errval) => { + serializer.write_bool(false)?; + #err_caller_arg_serializer1 + } + } + }, + schema: quote!( + get_result_schema(#ok_schema, #err_schema) + ), + arg_type1: quote!( + Result<#ok_deserialized_type, #err_deserialized_type> + ), + known_size_align1: None, + known_size_align_of_pointer1: None, + } + } } } } @@ -1188,12 +1334,12 @@ pub(super) fn generate_method_definitions( { Ok(()) => { let outcome = RawAbiCallResult::Success {data: #data_as_ptr, len: #data_length}; - unsafe { receiver(&outcome as *const _, abi_result) } + unsafe { __savefile_internal_receiver(&outcome as *const _, abi_result) } } Err(err) => { let err_str = format!("{:?}", err); let outcome = RawAbiCallResult::AbiError(AbiErrorMsg{error_msg_utf8: err_str.as_ptr(), len: err_str.len()}); - unsafe { receiver(&outcome as *const _, abi_result) } + unsafe { __savefile_internal_receiver(&outcome as *const _, abi_result) } } } } diff --git a/savefile-test/Cargo.toml b/savefile-test/Cargo.toml index d31062a..5233e03 100644 --- a/savefile-test/Cargo.toml +++ b/savefile-test/Cargo.toml @@ -14,7 +14,7 @@ nightly=["savefile/nightly"] [dependencies] savefile = { path = "../savefile", features = ["size_sanity_checks", "encryption", "compression","bit-set","bit-vec","rustc-hash","serde_derive", "quickcheck", "nalgebra"]} -savefile-derive = { path = "../savefile-derive", version = "=0.17.11" } +savefile-derive = { path = "../savefile-derive", version = "=0.17.12" } savefile-abi = { path = "../savefile-abi" } bit-vec = "0.8" arrayvec="0.7" diff --git a/savefile-test/src/savefile_abi_test/advanced_datatypes_test.rs b/savefile-test/src/savefile_abi_test/advanced_datatypes_test.rs index 980400c..9c664a8 100644 --- a/savefile-test/src/savefile_abi_test/advanced_datatypes_test.rs +++ b/savefile-test/src/savefile_abi_test/advanced_datatypes_test.rs @@ -20,6 +20,9 @@ pub trait AdvancedTestInterface : Send{ fn many_callbacks(&mut self, x: &mut dyn FnMut(&dyn Fn(&dyn Fn() -> u32) -> u32) -> u32) -> u32; fn buf_callback(&mut self, cb: Box); + fn return_boxed_closure_result(&self, fail: bool) -> Result u32>,()>; + fn owned_boxed_closure_param(&self, owned: Boxu32>); + } struct SimpleImpl; @@ -68,6 +71,17 @@ impl AdvancedTestInterface for AdvancedTestInterfaceImpl { fn buf_callback(&mut self, cb: Box) { cb(&[1,2,3], "hello".to_string()) } + fn return_boxed_closure_result(&self, fail: bool) -> Result u32>,()> { + if fail { + Err(()) + } else { + Ok(Box::new(|| 42)) + } + } + + fn owned_boxed_closure_param(&self, owned: Box u32>) { + assert_eq!(owned(), 42); + } } struct TestUser(Box); @@ -88,6 +102,17 @@ fn abi_test_buf_send() { require_send(boxed); } +#[test] +fn test_trait_object_in_return_position() { + let boxed: Box = Box::new(AdvancedTestInterfaceImpl {}); + let conn = AbiConnection::from_boxed_trait(boxed).unwrap(); + + let ret = conn.return_boxed_closure_result(false); + assert_eq!(ret.unwrap()(), 42); + let ret = conn.return_boxed_closure_result(true); + let Err(()) = ret else {panic!("Expected Err")}; +} + #[test] fn abi_test_buf_callback() { let boxed: Box = Box::new(AdvancedTestInterfaceImpl {}); @@ -112,7 +137,7 @@ fn abi_test_slice() { } #[test] -fn test_trait_object_in_return_position() { +fn test_result_trait_object_in_return_position() { let boxed: Box = Box::new(AdvancedTestInterfaceImpl {}); let conn = AbiConnection::from_boxed_trait(boxed).unwrap(); @@ -121,6 +146,13 @@ fn test_trait_object_in_return_position() { assert_eq!(ret.do_call(42), 42); } #[test] +fn test_boxed_trait_object_in_arg_position() { + let boxed: Box = Box::new(AdvancedTestInterfaceImpl {}); + let conn = AbiConnection::from_boxed_trait(boxed).unwrap(); + + conn.owned_boxed_closure_param(Box::new(||42)); +} +#[test] fn test_return_boxed_closure() { let closure; let closure2; diff --git a/savefile/Cargo.toml b/savefile/Cargo.toml index 5e18e15..05eea38 100644 --- a/savefile/Cargo.toml +++ b/savefile/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "savefile" -version = "0.17.11" +version = "0.17.12" authors = ["Anders Musikka "] documentation = "https://docs.rs/savefile/" homepage = "https://github.com/avl/savefile/" @@ -63,13 +63,13 @@ bit-set08 = {package="bit-set", version = "0.8", optional = true} rustc-hash = {version = "1.1", optional = true} memoffset = "0.9" byteorder = "1.4" -savefile-derive = {path="../savefile-derive", version = "=0.17.11", optional = true } +savefile-derive = {path="../savefile-derive", version = "=0.17.12", optional = true } serde_derive = {version= "1.0", optional = true} serde = {version= "1.0", optional = true} quickcheck = {version= "1.0", optional = true} [dev-dependencies] -savefile-derive = { path="../savefile-derive", version = "=0.17.11" } +savefile-derive = { path="../savefile-derive", version = "=0.17.12" } [build-dependencies] rustc_version="0.2" diff --git a/savefile/src/lib.rs b/savefile/src/lib.rs index c94b3d3..fb480eb 100644 --- a/savefile/src/lib.rs +++ b/savefile/src/lib.rs @@ -2447,6 +2447,39 @@ pub fn get_schema(version: u32) -> Schema { T::schema(version, &mut WithSchemaContext::new()) } +/// Get the schema for a type Result, where OK and ERR +/// have the schemas given by the parameters. +pub fn get_result_schema(ok: Schema, err: Schema) -> Schema { + Schema::Enum(SchemaEnum { + dbg_name: "Result".to_string(), + size: None, + alignment: None, + variants: vec![ + Variant { + name: "Ok".to_string(), + discriminant: 0, + fields: vec![Field { + name: "ok".to_string(), + value: Box::new(ok), + offset: None, + }], + }, + Variant { + name: "Err".to_string(), + discriminant: 0, + fields: vec![Field { + name: "err".to_string(), + value: Box::new(err), + offset: None, + }], + }, + ], + discriminant_size: 1, + has_explicit_repr: false, + }) +} + + /// This trait must be implemented for all data structures you wish to be /// able to serialize. To actually serialize data: create a [Serializer], /// then call serialize on your data to save, giving the Serializer diff --git a/savefile/src/prelude.rs b/savefile/src/prelude.rs index 49443e1..ece4789 100644 --- a/savefile/src/prelude.rs +++ b/savefile/src/prelude.rs @@ -1,5 +1,5 @@ pub use { - super::deserialize_slice_as_vec, super::get_schema, super::introspect_item, super::load, super::load_file, + super::deserialize_slice_as_vec, super::get_schema, super::get_result_schema, super::introspect_item, super::load, super::load_file, super::load_file_noschema, super::load_from_mem, super::load_noschema, super::save, super::save_file, super::save_file_noschema, super::save_noschema, super::save_to_mem, super::AbiRemoved, super::Canary1, super::Deserialize, super::Deserializer, super::Field, super::Introspect, super::IntrospectItem,