From 2e92028de23fb5a1dc082d866923df6090c3aa06 Mon Sep 17 00:00:00 2001 From: Benedikt Peetz Date: Sat, 4 May 2024 10:12:20 +0200 Subject: [PATCH] feat(src/macros)!: Store allocation location in all stringsThis makes it possible to automatically free the string in the correctway expected by both c and rust.Rust strings can now just be freed by the rust allocator, whilst cstrings are forgotten, so that the c allocator may free them.BREAKING CHANGE: All usage of explicit mentions `std::string::String` in your `handle_cmd` function will need to be replaced by `trixy::types::newtypes::String`. Otherwise these types should provide the same methods. --- example/main/src/bin/main/main.rs | 16 ++-- .../generate/convert/host/c/type/mod.rs | 5 +- .../convert/host/rust/function/mod.rs | 1 - .../generate/convert/host/rust/type/mod.rs | 53 ++++++----- src/types/mod.rs | 1 + src/types/newtypes/mod.rs | 88 +++++++++++++++++++ src/types/traits/try_from_impl.rs | 41 ++++++--- 7 files changed, 159 insertions(+), 46 deletions(-) create mode 100644 src/types/newtypes/mod.rs diff --git a/example/main/src/bin/main/main.rs b/example/main/src/bin/main/main.rs index 0394db2..b9e0341 100644 --- a/example/main/src/bin/main/main.rs +++ b/example/main/src/bin/main/main.rs @@ -20,9 +20,10 @@ * If not, see . */ -use std::{env, ffi::c_int, mem}; +use std::{env, ffi::c_int}; use libloading::{Library, Symbol}; +use trixy::types::newtypes; use crate::dogs::{DogType, TrainedDog}; @@ -30,6 +31,7 @@ include!(concat!(env!("OUT_DIR"), "/api.rs")); // run `cargo run --bin api > ./src/bin/main/generated_api.rs` to output the generated api // mod generated_api; +// pub use generated_api::*; fn handle_cmd(cmd: Commands) { match cmd { @@ -38,9 +40,11 @@ fn handle_cmd(cmd: Commands) { let output = format!("Hi {}!", name); println!("(rust): {}", output); trixy_output.send(output.into()).expect("Will work"); - mem::forget(name); } - dogs::Dogs::train_dog { trixy_output, dog } => { + dogs::Dogs::train_dog { + trixy_output, + dog: _, + } => { trixy_output .send( TrainedDog { @@ -51,23 +55,19 @@ fn handle_cmd(cmd: Commands) { .unwrap(), ) .unwrap(); - mem::forget(dog); } dogs::Dogs::guess_my_favourite_dog { trixy_output, callback, } => { let fav_dog = callback("Frank".into(), 30); - let dog_name: String = fav_dog.name.clone().try_into().unwrap(); + let dog_name: newtypes::String = fav_dog.name.clone().try_into().unwrap(); println!("(rust): They want a dog named: {}", dog_name); trixy_output.send(DogType::Cat.into()).unwrap(); - - mem::forget(dog_name); } }, Commands::outstanding { name } => { println!("(rust): {} is outstanding!", name); - mem::forget(name); } } } diff --git a/src/macros/generate/convert/host/c/type/mod.rs b/src/macros/generate/convert/host/c/type/mod.rs index e6f7f28..13c74eb 100644 --- a/src/macros/generate/convert/host/c/type/mod.rs +++ b/src/macros/generate/convert/host/c/type/mod.rs @@ -28,9 +28,6 @@ use crate::parser::command_spec::{Identifier, NamedType, Type}; mod doc_named_type; mod named_type; -// pub use doc_named_type::*; -// pub use named_type::*; - impl Type { pub fn to_c(&self) -> TokenStream2 { match self { @@ -82,7 +79,7 @@ impl Type { let type_name = trixy_build_in_types .first() - .expect("The names should not be dublicated, this should be the only value"); + .expect("The names should not be duplicated, this should be the only value"); match *type_name { "Result" => { diff --git a/src/macros/generate/convert/host/rust/function/mod.rs b/src/macros/generate/convert/host/rust/function/mod.rs index 1f7a798..45df293 100644 --- a/src/macros/generate/convert/host/rust/function/mod.rs +++ b/src/macros/generate/convert/host/rust/function/mod.rs @@ -26,7 +26,6 @@ use quote::quote; use crate::parser::command_spec::{Function, Identifier, NamedType, Type}; impl Function { - // was called function_identifier_to_rust pub fn to_rust_identifier( &self, input_fmt_fn: fn(&NamedType) -> TokenStream2, diff --git a/src/macros/generate/convert/host/rust/type/mod.rs b/src/macros/generate/convert/host/rust/type/mod.rs index 6556065..e1aa39e 100644 --- a/src/macros/generate/convert/host/rust/type/mod.rs +++ b/src/macros/generate/convert/host/rust/type/mod.rs @@ -61,32 +61,41 @@ impl Type { } } pub fn to_rust_typical(identifier: &Identifier, generic_args: &Vec) -> TokenStream2 { - let ident = identifier.to_rust(); - let namespaces_path = Variant::to_rust_path(&identifier.variant); - - let nasp_path = if let Some(nasp_path) = Variant::to_rust_path(&identifier.variant) { - if nasp_path.is_empty() { + match identifier { + Identifier { name, variant: _ } if name == "String" => { + // We wrap over the string to ensure that we know where it was allocated. quote! { - crate :: - } - } else { - let path = namespaces_path; - quote! { - #path :: + trixy::types::newtypes::String } } - } else { - quote! {} - }; + _ => { + let ident = identifier.to_rust(); - if generic_args.is_empty() { - quote! { - #nasp_path #ident - } - } else { - let generics: Vec = generic_args.iter().map(Type::to_rust).collect(); - quote! { - #nasp_path #ident <#(#generics),*> + let nasp_path = if let Some(path) = &Variant::to_rust_path(&identifier.variant) { + if path.is_empty() { + quote! { + crate :: + } + } else { + quote! { + #path :: + } + } + } else { + quote! {} + }; + + if generic_args.is_empty() { + quote! { + #nasp_path #ident + } + } else { + let generics: Vec = + generic_args.iter().map(Type::to_rust).collect(); + quote! { + #nasp_path #ident <#(#generics),*> + } + } } } } diff --git a/src/types/mod.rs b/src/types/mod.rs index 04b15e0..8f0e876 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -24,6 +24,7 @@ pub mod error; pub mod traits; pub mod types_list; +pub mod newtypes; pub use types_list::*; diff --git a/src/types/newtypes/mod.rs b/src/types/newtypes/mod.rs new file mode 100644 index 0000000..6a2a2c4 --- /dev/null +++ b/src/types/newtypes/mod.rs @@ -0,0 +1,88 @@ +//! This module contains wrapper types for know rust types, where trixy needs to store additional +//! information + +use std::{fmt::Display, mem, string}; + +#[derive(Debug, Default, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct String { + pub(crate) data: Option, + pub(crate) alloc_location: AllocLocation, +} + +#[derive(Clone, Hash, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum AllocLocation { + #[default] + Rust, + C, +} + +impl From<&str> for String { + fn from(value: &str) -> Self { + Self { + // It's cloned here: + data: Some(value.to_owned()), + // The string is always allocated in rust, as we clone it above this comment + alloc_location: AllocLocation::Rust, + } + } +} + +impl From for String { + fn from(value: string::String) -> Self { + Self { + data: Some(value), + // We just assume that every std String is actually allocated in rust + alloc_location: AllocLocation::Rust, + } + } +} + +impl Display for String { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +impl String { + pub fn as_str(&self) -> &str { + &self + .data + .as_ref() + .expect("The string should only be empty when it's dropped") + } + pub fn into_std_string(mut self) -> string::String { + match self.alloc_location { + AllocLocation::C => { + // Re-allocate the string in rust + let c_alloc_string = mem::take(&mut self.data).expect("It should only be Some"); + let rust_alloc_string = c_alloc_string.clone(); + + // This is here because of the same reason as in the drop impl: we just can't free + // a string allocated in c. + mem::forget(c_alloc_string); + + rust_alloc_string + } + AllocLocation::Rust => { + let string = mem::take(&mut self.data).expect("Will always be some"); + string + } + } + } +} + +impl Drop for String { + fn drop(&mut self) { + match self.alloc_location { + AllocLocation::C => { + let string = mem::take(&mut self.data); + // C needs to free it's strings by itself. We cannot do the job of the c allocator, + // thus just forget about it here + mem::forget(string) + } + AllocLocation::Rust => { + // A rust allocated string can be free normally. We don't need to do anything here. + } + } + } +} diff --git a/src/types/traits/try_from_impl.rs b/src/types/traits/try_from_impl.rs index 5dce092..2afa7c0 100644 --- a/src/types/traits/try_from_impl.rs +++ b/src/types/traits/try_from_impl.rs @@ -20,32 +20,51 @@ * If not, see . */ -use crate::types::String; +use crate::types::{ + newtypes::{self, AllocLocation}, + types_list, +}; use std::ffi::CString; use super::convert_trait::Convertible; -impl From for String { +impl From for types_list::String { fn from(value: std::string::String) -> Self { let cstring = CString::new(value).expect("The input is a valid String, this always succeeds"); let c_char = cstring.into_ptr(); - String(c_char) + types_list::String(c_char) } } /// Plainly here for convenience -impl From<&str> for String { +impl From<&str> for types_list::String { fn from(value: &str) -> Self { value.to_owned().into() } } -impl TryFrom for std::string::String { - type Error = crate::types::error::TypeConversionError; - - fn try_from(value: String) -> Result { - let cstring = CString::from_ptr(value.0)?; - let string = cstring.into_string()?; - Ok(string) +impl From for types_list::String { + fn from(value: newtypes::String) -> Self { + // NOTE: This is not really performant, but necessary as we otherwise would need c to + // differentiate between c allocated strings (which are freed by `free`) and rust allocated strings + // (which are freed by `string_free`) <2024-05-04> + let value_string = value.into_std_string(); + value_string.into() + } +} + +impl TryFrom for newtypes::String { + type Error = crate::types::error::TypeConversionError; + + fn try_from(value: types_list::String) -> Result { + let cstring = CString::from_ptr(value.0)?; + let string = cstring.into_string()?; + + Ok(newtypes::String { + data: Some(string), + // TODO: You could of course do a rust-only round trip, but I think it's reasonable to + // assume, that all types_list::String values come from C <2024-05-04> + alloc_location: AllocLocation::C, + }) } }