feat: recursion Support recursion in the serialization schema graph (#43

)

* feat: recursion Support recursion in the serialization schema graph

* fix: Remove compile_tests from release-plz config

* Enable github-releases again (experimental)

* wip

README.md

@@ -107,6 +107,18 @@ what the rust project calls it)
 encode the discriminant. Set to 1 for enums which will never have more than 256 fields. Set to 2 for bigger
 enums. If you ever need an enum to have more than 65536 fields, set it to 4.
 
+1.5: The WithSchema::schema function now takes a context object. You can just pass this through for
+most data types. Only smart pointers, containers, Box etc need ot use this, to guard against 
+recursion. See documentation of WithSchemaContext.
+
+1.6: Several savefile trait implementations have now gained 'static-bounds. For example,
+Box<T>, Vec<T> and many more now require T:'static. There was no such bound before, but
+since references cannot be deserialized, it was still typically not possible to deserialize
+anything containing a reference. In principle, someone could have implemented Deserialize
+for some type, leaking memory and returning an instance with a non-static lifetime. However,
+this is a very niche use, and it seems much more likely that deserializing types with arbitrary
+lifetimes is an error. Please file an issue if you have an use-case for deserializing types
+with lifetimes.
 
 ## 0.17.0-beta.13
 

release-plz.toml

@@ -1,5 +1,5 @@
 [workspace]
-git_release_enable = false
+#git_release_enable = false
 
 [[package]]
 name= "savefile"
@@ -18,11 +18,6 @@ name= "savefile-abi"
 publish = true
 release = true
 
-[[package]]
-name= "compile_tests"
-publish = false
-release = false
-
 [[package]]
 name= "savefile-test"
 publish = false

savefile-abi/src/lib.rs

@@ -1174,7 +1174,7 @@ impl<T: AbiExportable + ?Sized> AbiConnection<T> {
                 });
             }
             let mut mask = 0;
-            let mut check_diff = |effective1, effective2, native1, native2, index: Option<usize>| {
+            let mut verify_compatibility = |effective1, effective2, native1, native2, index: Option<usize>| {
                 let effective_schema_diff = diff_schema(effective1, effective2, "".to_string());
                 if let Some(diff) = effective_schema_diff {
                     return Err(SavefileError::IncompatibleSchema {
@@ -1192,9 +1192,6 @@ impl<T: AbiExportable + ?Sized> AbiConnection<T> {
                     });
                 }
 
-                //let caller_isref = caller_native_method.info.arguments[index].can_be_sent_as_ref;
-                //let callee_isref = callee_native_method.info.arguments[index].can_be_sent_as_ref;
-
                 let comp = arg_layout_compatible(native1, native2, effective1, effective2, effective_version);
 
                 if comp {
@@ -1210,10 +1207,10 @@ impl<T: AbiExportable + ?Sized> AbiConnection<T> {
                 let effective2 = &callee_effective_method.info.arguments[index].schema;
                 let native1 = &caller_native_method.info.arguments[index].schema;
                 let native2 = &callee_native_method.info.arguments[index].schema;
-                check_diff(effective1, effective2, native1, native2, Some(index))?;
+                verify_compatibility(effective1, effective2, native1, native2, Some(index))?;
             }
 
-            check_diff(
+            verify_compatibility(
                 &caller_effective_method.info.return_value,
                 &callee_effective_method.info.return_value,
                 &caller_native_method.info.return_value,
@@ -1322,6 +1319,23 @@ impl<T: AbiExportable + ?Sized> AbiConnection<T> {
         Self::from_raw(packed.entry, packed.trait_object, owning)
     }
 
+    /// Check if the given argument 'arg' in method 'method' is memory compatible such that
+    /// it will be sent as a reference, not copied. This will depend on the memory layout
+    /// of the code being called into. It will not change during the lifetime of an
+    /// AbiConnector, but it may change if the target library is recompiled.
+    pub fn get_arg_passable_by_ref(&self, method: &str, arg: usize) -> bool {
+        if let Some(found) = self.template.methods.iter().find(|var|var.method_name == method) {
+            let abi_method: &AbiConnectionMethod = found;
+            if arg >= abi_method.caller_info.arguments.len() {
+                panic!("Method '{}' has only {} arguments, so there is no argument #{}", method, abi_method.caller_info.arguments.len(), arg);
+            }
+            (abi_method.compatibility_mask & (1 << (arg as u64))) != 0
+        } else {
+            let arg_names : Vec<_> =  self.template.methods.iter().map(|x|x.method_name.as_str()).collect();
+            panic!("Trait has no method with name '{}'. Available methods: {}", method, arg_names.join(", "));
+        }
+    }
+
     /// This routine is mostly for tests.
     /// It allows using a raw external API entry point directly.
     /// This is mostly useful for internal testing of the savefile-abi-library.

savefile-derive/src/lib.rs

@@ -253,7 +253,7 @@ pub fn savefile_abi_exportable(
     let uses = quote_spanned! { defspan =>
         extern crate savefile;
         extern crate savefile_abi;
-        use savefile::prelude::{ReprC, Schema, SchemaPrimitive, WithSchema, Serializer, Serialize, Deserializer, Deserialize, SavefileError, deserialize_slice_as_vec, ReadBytesExt,LittleEndian,AbiMethodArgument, AbiMethod, AbiMethodInfo,AbiTraitDefinition};
+        use savefile::prelude::{ReprC, Schema, SchemaPrimitive, WithSchema, WithSchemaContext, 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;
@@ -1574,7 +1574,7 @@ fn implement_withschema(
                     "The Removed type can only be used for removed fields. Use the savefile_version attribute."
                 );
             }
-            fields.push(quote_spanned!( span => #fields1.push(unsafe{#Field::unsafe_new(#name_str.to_string(), std::boxed::Box::new(<#field_type as #WithSchema>::schema(#local_version)), #offset)} )));
+            fields.push(quote_spanned!( span => #fields1.push(unsafe{#Field::unsafe_new(#name_str.to_string(), std::boxed::Box::new(<#field_type as #WithSchema>::schema(#local_version, context)), #offset)} )));
         } else {
             let mut version_mappings = Vec::new();
             let offset = if field_to_version != u32::MAX {
@@ -1589,7 +1589,7 @@ fn implement_withschema(
                 // We don't supply offset in this case, deserialized type doesn't match field type
                 version_mappings.push(quote!{
                     if #local_version >= #dt_from && local_version <= #dt_to {
-                        #fields1.push(#Field ::new( #name_str.to_string(), std::boxed::Box::new(<#dt_field_type as #WithSchema>::schema(#local_version))) );
+                        #fields1.push(#Field ::new( #name_str.to_string(), std::boxed::Box::new(<#dt_field_type as #WithSchema>::schema(#local_version, context))) );
                     }
                 });
             }
@@ -1598,7 +1598,7 @@ fn implement_withschema(
                 #(#version_mappings)*
 
                 if #local_version >= #field_from_version && #local_version <= #field_to_version {
-                    #fields1.push(unsafe{#Field ::unsafe_new( #name_str.to_string(), std::boxed::Box::new(<#field_type as #WithSchema>::schema(#local_version)), #offset )} );
+                    #fields1.push(unsafe{#Field ::unsafe_new( #name_str.to_string(), std::boxed::Box::new(<#field_type as #WithSchema>::schema(#local_version, context)), #offset )} );
                 }
                 ));
         }
@@ -1831,7 +1831,7 @@ fn savefile_derive_crate_withschema(input: DeriveInput) -> TokenStream {
 
                     #[allow(unused_mut)]
                     #[allow(unused_comparisons, unused_variables)]
-                    fn schema(version:u32) -> #Schema {
+                    fn schema(version:u32, context: &mut _savefile::prelude::WithSchemaContext) -> #Schema {
                         let local_version = version;
 
                         #Schema::Enum (
@@ -1912,7 +1912,7 @@ fn savefile_derive_crate_withschema(input: DeriveInput) -> TokenStream {
                 impl #impl_generics #withschema for #name #ty_generics #where_clause #extra_where {
                     #[allow(unused_comparisons)]
                     #[allow(unused_mut, unused_variables)]
-                    fn schema(version:u32) -> #Schema {
+                    fn schema(version:u32, context: &mut _savefile::prelude::WithSchemaContext) -> #Schema {
                         let local_version = version;
                         let mut fields1 = Vec::new();
                         #(#fields;)* ;

savefile-derive/src/savefile_abi.rs

@@ -685,7 +685,7 @@ impl ArgType {
                 caller_arg_serializer1: quote! {
                     #arg_name.serialize(&mut serializer)
                 },
-                schema: quote!( <#arg_type as WithSchema>::schema(version) ),
+                schema: quote!( <#arg_type as WithSchema>::schema(version, context) ),
                 known_size_align1: if compile_time_check_reprc(arg_type) {
                     compile_time_size(arg_type)
                 } else {
@@ -938,7 +938,7 @@ pub(super) fn generate_method_definitions(
         //let can_be_sent_as_ref = instruction.can_be_sent_as_ref;
         metadata_arguments.push(quote! {
                         AbiMethodArgument {
-                            schema: #schema,
+                            schema: { let mut context = WithSchemaContext::new(); let context = &mut context; #schema },
                         }
         });
         if let Some(total_size) = &mut compile_time_known_size {
@@ -975,7 +975,7 @@ pub(super) fn generate_method_definitions(
     let result_default;
     let return_ser_temp;
     if no_return {
-        return_value_schema = quote!(<() as WithSchema>::schema(0));
+        return_value_schema = quote!(<() as WithSchema>::schema(0, &mut WithSchemaContext::new()));
         ret_deserializer = quote!(()); //Zero-sized, no deserialize actually needed
         ret_serialize = quote!(());
         caller_return_type = quote!(());
@@ -1117,7 +1117,7 @@ pub(super) fn generate_method_definitions(
         AbiMethod {
             name: #method_name_str.to_string(),
             info: AbiMethodInfo {
-                return_value: #return_value_schema,
+                return_value: { let mut context = WithSchemaContext::new(); let context = &mut context; #return_value_schema},
                 arguments: vec![ #(#metadata_arguments,)* ],
             }
         }

savefile-min-build/src/lib.rs

@@ -1,13 +1,26 @@
 extern crate savefile_abi;
 extern crate savefile_derive;
 
+use savefile_abi::AbiConnection;
 use savefile_derive::savefile_abi_exportable;
 
 
 #[savefile_abi_exportable(version = 0)]
 pub trait ExampleTrait {
-    fn get(&mut self) -> &'static str;
+    fn test_slices(&mut self, slice: &[u32]) -> u32 {
+        slice.iter().copied().sum()
+    }
+}
+
+impl ExampleTrait for () {
+
 }
 
 #[test]
-fn dummy_test() {}
+fn dummy_test() {
+    let boxed: Box<dyn ExampleTrait> = Box::new(());
+    let conn = AbiConnection::from_boxed_trait(boxed).unwrap();
+
+    assert!( conn.get_arg_passable_by_ref("test_slices", 0) );
+    //conn.test_slices(&[1,2,3,4]);
+}

savefile-test/src/cycles.rs

@@ -0,0 +1,87 @@
+use assert_roundtrip;
+use savefile::Removed;
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Tree {
+    Leaf,
+    Node(Box<Tree>,Box<Tree>)
+}
+
+#[test]
+pub fn test_cyclic() {
+    let example = Tree::Node(Box::new(Tree::Leaf), Box::new(Tree::Leaf));
+    assert_roundtrip(example);
+
+    let example = Tree::Node(Box::new(Tree::Node(Box::new(Tree::Leaf),Box::new(Tree::Leaf))), Box::new(Tree::Leaf));
+    assert_roundtrip(example);
+}
+
+
+#[derive(Savefile, Debug, PartialEq)]
+struct TreeNode {
+    tree: Box<Tree2>
+}
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Tree2 {
+    Leaf(String),
+    Node(TreeNode)
+}
+#[test]
+pub fn test_cyclic2() {
+    let example = Tree2::Node(TreeNode{tree: Box::new(Tree2::Leaf("hej".into()))});
+    assert_roundtrip(example);
+}
+#[derive(Savefile, Debug, PartialEq)]
+enum Version1LevelD {
+    Leaf,
+    Node(Box<Version1LevelA>)
+}
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Version1LevelC {
+    Leaf,
+    Node(Box<Version1LevelD>)
+}
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Version1LevelB {
+    Leaf(Box<Version1LevelC>),
+    Node(Box<Version1LevelC>)
+}
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Version1LevelA {
+    Leaf,
+    Node(Box<Version1LevelB>)
+}
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Version2LevelC {
+    Leaf,
+    Node(Box<Version2LevelA>)
+}
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Version2LevelB {
+    Leaf(Box<Version2LevelC>),
+    Node(Box<Version2LevelC>)
+}
+
+#[derive(Savefile, Debug, PartialEq)]
+enum Version2LevelA {
+    Leaf,
+    Node(Box<Version2LevelB>)
+}
+
+#[test]
+#[should_panic(expected = "Saved schema differs from in-memory schema for version 0. Error: At location [.Version1LevelA/Node/0Version1LevelB/Leaf/0Version1LevelC/Node/0Version1LevelD/Node/0]: In memory schema: <recursion 3>, file schema: enum")]
+fn cycles_vertest1() {
+    use assert_roundtrip_to_new_version;
+    assert_roundtrip_to_new_version(
+        Version1LevelA::Leaf,
+        0,
+        Version2LevelA::Leaf,
+        1,
+    );
+}

savefile-test/src/lib.rs

@@ -460,6 +460,8 @@ pub fn assert_roundtrip_to_new_version<
 
 mod enum_variant_versioning;
 
+mod cycles;
+
 #[test]
 pub fn test_array_string() {
     use arrayvec::ArrayString;

savefile-test/src/savefile_abi_test/advanced_datatypes_test.rs

@@ -12,6 +12,7 @@ pub trait AdvancedTestInterface {
     fn clone_hashmap(&self, x: &HashMap<String, String>) -> HashMap<String, String>;
 
     fn return_trait_object(&self) -> Box<dyn SimpleInterface>;
+    fn test_slices(&mut self, slice: &[u32]) -> u32;
 
     fn return_boxed_closure(&self) -> Box<dyn Fn() -> u32>;
     fn return_boxed_closure2(&self) -> Box<dyn Fn()>;
@@ -52,11 +53,24 @@ impl AdvancedTestInterface for AdvancedTestInterfaceImpl {
         Box::new(|| {})
     }
 
+    fn test_slices(&mut self, slice: &[u32]) -> u32 {
+        slice.iter().copied().sum()
+    }
+
     fn many_callbacks(&mut self, x: &mut dyn FnMut(&dyn Fn(&dyn Fn() -> u32) -> u32) -> u32) -> u32 {
         x(&|y| y())
     }
 }
 
+#[test]
+fn abi_test_slice() {
+    let boxed: Box<dyn AdvancedTestInterface> = Box::new(AdvancedTestInterfaceImpl {});
+    let mut conn = AbiConnection::from_boxed_trait(boxed).unwrap();
+
+    assert!( conn.get_arg_passable_by_ref("test_slices", 0) );
+    assert_eq!(conn.test_slices(&[1,2,3,4]), 10);
+}
+
 #[test]
 fn test_trait_object_in_return_position() {
     let boxed: Box<dyn AdvancedTestInterface> = Box::new(AdvancedTestInterfaceImpl {});

savefile-test/src/savefile_abi_test/argument_backward_compatibility.rs

@@ -1,5 +1,5 @@
 use savefile::prelude::AbiRemoved;
-use savefile::SavefileError;
+use savefile::{SavefileError, WithSchemaContext};
 use savefile_abi::RawAbiCallResult::AbiError;
 use savefile_abi::{verify_compatiblity, AbiConnection, AbiExportable};
 use savefile_abi_test::argument_backward_compatibility::v1::{ArgInterfaceV1, EnumArgument, Implementation1};
@@ -98,7 +98,7 @@ pub fn test_backward_compatibility() -> Result<(), SavefileError> {
 #[test]
 pub fn test_arg_argument_metadata() {
     use savefile::WithSchema;
-    let schema = v2::ArgArgument::schema(0);
+    let schema = v2::ArgArgument::schema(0, &mut WithSchemaContext::new());
     println!("Schema: {:#?}", schema);
     assert!(!schema.layout_compatible(&schema)); //Versions containing removed items should never be considered layout compatible (since their schema type is not identical to the memory type)
 }

savefile-test/src/savefile_abi_test/basic_abi_tests.rs

@@ -24,6 +24,7 @@ pub trait TestInterface {
 
     fn do_mut_nothing(&mut self);
 
+    fn deref_u32(&self, x: &u32) -> u32;
     fn count_chars(&self, x: &String) -> usize;
     fn count_chars_str(&self, x: &str) -> usize;
 
@@ -118,6 +119,10 @@ impl TestInterface for TestInterfaceImpl {
     fn get_static_str(&self) -> &'static str {
         "hello world"
     }
+
+    fn deref_u32(&self, x: &u32) -> u32 {
+        *x
+    }
 }
 
 savefile_abi_export!(TestInterfaceImpl, TestInterface);
@@ -139,7 +144,11 @@ fn test_basic_call_abi() {
 
     assert_eq!(conn.count_chars(&"hejsan".to_string()), 6);
     assert_eq!(conn.count_chars_str("hejsan"), 6);
+    assert!(conn.get_arg_passable_by_ref("count_chars", 0));
     assert_eq!(conn.get_static_str(), "hello world");
+
+    assert_eq!(conn.deref_u32(&42), 42);
+    assert!(conn.get_arg_passable_by_ref("deref_u32", 0));
 }
 
 #[test]