diff --git a/plugins/cairo-lang-macro-attributes/src/lib.rs b/plugins/cairo-lang-macro-attributes/src/lib.rs index 81856948f..e39294732 100644 --- a/plugins/cairo-lang-macro-attributes/src/lib.rs +++ b/plugins/cairo-lang-macro-attributes/src/lib.rs @@ -17,10 +17,20 @@ pub fn attribute_macro(_args: TokenStream, input: TokenStream) -> TokenStream { #[no_mangle] pub unsafe extern "C" fn expand(token_stream: cairo_lang_macro_stable::StableTokenStream) -> cairo_lang_macro_stable::StableProcMacroResult { - let token_stream = TokenStream::from_stable(token_stream); + let token_stream = cairo_lang_macro::TokenStream::from_stable(token_stream); let result = #item_name(token_stream); result.into_stable() } }; TokenStream::from(expanded) } + +#[proc_macro] +pub fn macro_commons(_input: TokenStream) -> TokenStream { + TokenStream::from(quote! { + #[no_mangle] + pub unsafe extern "C" fn free_result(result: cairo_lang_macro_stable::StableProcMacroResult) { + cairo_lang_macro::ProcMacroResult::from_owned_stable(result); + } + }) +} diff --git a/plugins/cairo-lang-macro-stable/src/lib.rs b/plugins/cairo-lang-macro-stable/src/lib.rs index 2592f2fb6..a431094a0 100644 --- a/plugins/cairo-lang-macro-stable/src/lib.rs +++ b/plugins/cairo-lang-macro-stable/src/lib.rs @@ -1,15 +1,15 @@ -use std::ffi::CString; +use std::ffi::{CStr, CString}; use std::os::raw::c_char; /// Token stream. /// /// This struct implements FFI-safe stable ABI. #[repr(C)] -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct StableTokenStream(*mut c_char); #[repr(C)] -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum StableAuxData { None, Some(*mut c_char), @@ -19,7 +19,7 @@ pub enum StableAuxData { /// /// This struct implements FFI-safe stable ABI. #[repr(C)] -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum StableProcMacroResult { /// Plugin has not taken any action. Leave, @@ -41,7 +41,13 @@ impl StableTokenStream { /// /// # Safety pub unsafe fn to_string(&self) -> String { - raw_to_string(self.0) + // Note that this does not deallocate the c-string. + // The memory must still be freed with `CString::from_raw`. + CStr::from_ptr(self.0).to_string_lossy().to_string() + } + + pub fn into_owned_string(self) -> String { + unsafe { raw_to_string(self.0) } } } diff --git a/plugins/cairo-lang-macro/src/lib.rs b/plugins/cairo-lang-macro/src/lib.rs index 2b99cdde0..735f37504 100644 --- a/plugins/cairo-lang-macro/src/lib.rs +++ b/plugins/cairo-lang-macro/src/lib.rs @@ -1,4 +1,4 @@ -use std::ffi::{c_char, CString}; +use std::ffi::{c_char, CStr, CString}; use std::fmt::Display; pub use cairo_lang_macro_attributes::*; @@ -72,7 +72,9 @@ impl ProcMacroResult { } } - /// Convert to native Rust representation. + /// Convert to native Rust representation, without taking the ownership of the string. + /// + /// Note that you still need to free the memory by calling `from_owned_stable`. /// /// # Safety #[doc(hidden)] @@ -85,7 +87,28 @@ impl ProcMacroResult { aux_data, } => ProcMacroResult::Replace { token_stream: TokenStream::from_stable(token_stream), - aux_data: AuxData::from_stable(aux_data).unwrap(), + aux_data: AuxData::from_stable(aux_data), + }, + } + } + + /// Convert to native Rust representation, with taking the ownership of the string. + /// + /// Useful when you need to free the allocated memory. + /// Only use on the same side of FFI-barrier, where the memory has been allocated. + /// + /// # Safety + #[doc(hidden)] + pub unsafe fn from_owned_stable(result: StableProcMacroResult) -> Self { + match result { + StableProcMacroResult::Leave => ProcMacroResult::Leave, + StableProcMacroResult::Remove => ProcMacroResult::Remove, + StableProcMacroResult::Replace { + token_stream, + aux_data, + } => ProcMacroResult::Replace { + token_stream: TokenStream::from_owned_stable(token_stream), + aux_data: AuxData::from_owned_stable(aux_data), }, } } @@ -101,13 +124,26 @@ impl TokenStream { StableTokenStream::new(cstr.into_raw()) } - /// Convert to native Rust representation. + /// Convert to native Rust representation, without taking the ownership of the string. + /// + /// Note that you still need to free the memory by calling `from_owned_stable`. /// /// # Safety #[doc(hidden)] pub unsafe fn from_stable(token_stream: StableTokenStream) -> Self { Self::new(token_stream.to_string()) } + + /// Convert to native Rust representation, with taking the ownership of the string. + /// + /// Useful when you need to free the allocated memory. + /// Only use on the same side of FFI-barrier, where the memory has been allocated. + /// + /// # Safety + #[doc(hidden)] + pub unsafe fn from_owned_stable(token_stream: StableTokenStream) -> Self { + Self::new(token_stream.into_owned_string()) + } } impl AuxData { @@ -131,19 +167,37 @@ impl AuxData { StableAuxData::Some(cstr.into_raw()) } - /// Convert to native Rust representation. + /// Convert to native Rust representation, without taking the ownership of the string. + /// + /// Note that you still need to free the memory by calling `from_owned_stable`. /// /// # Safety #[doc(hidden)] - pub unsafe fn from_stable(aux_data: StableAuxData) -> Result, serde_json::Error> { + pub unsafe fn from_stable(aux_data: StableAuxData) -> Option { match aux_data { - StableAuxData::None => Ok(None), - StableAuxData::Some(raw) => Some(Self::try_new(raw_to_string(raw))).transpose(), + StableAuxData::None => None, + StableAuxData::Some(raw) => Some(Self::new(from_raw_cstr(raw))), + } + } + + /// Convert to native Rust representation, with taking the ownership of the string. + /// + /// Useful when you need to free the allocated memory. + /// Only use on the same side of FFI-barrier, where the memory has been allocated. + /// + /// # Safety + #[doc(hidden)] + pub unsafe fn from_owned_stable(aux_data: StableAuxData) -> Option { + match aux_data { + StableAuxData::None => None, + StableAuxData::Some(raw) => Some(Self::new(from_raw_cstring(raw))), } } } -unsafe fn raw_to_string(raw: *mut c_char) -> String { +// Create a string from a raw pointer to a c_char. +// Note that this will free the underlying memory. +unsafe fn from_raw_cstring(raw: *mut c_char) -> String { if raw.is_null() { String::default() } else { @@ -151,3 +205,14 @@ unsafe fn raw_to_string(raw: *mut c_char) -> String { cstr.to_string_lossy().to_string() } } + +// Note that this will not free the underlying memory. +// You still need to free the memory by calling `CString::from_raw`. +unsafe fn from_raw_cstr(raw: *mut c_char) -> String { + if raw.is_null() { + String::default() + } else { + let cstr = CStr::from_ptr(raw); + cstr.to_string_lossy().to_string() + } +} diff --git a/scarb/src/compiler/plugin/proc_macro/ffi.rs b/scarb/src/compiler/plugin/proc_macro/ffi.rs index 746dd7666..309631c65 100644 --- a/scarb/src/compiler/plugin/proc_macro/ffi.rs +++ b/scarb/src/compiler/plugin/proc_macro/ffi.rs @@ -65,17 +65,40 @@ impl ProcMacroInstance { } /// Apply expansion to token stream. + /// + /// This function implements the actual calls to functions from the dynamic library. + /// + /// All values passing the FFI-barrier must implement a stable ABI. + /// + /// Please be aware that the memory management of values passing the FFI-barrier is tricky. + /// The memory must be freed on the same side of the barrier, where the allocation was made. pub(crate) fn generate_code(&self, token_stream: TokenStream) -> ProcMacroResult { - let ffi_token_stream = token_stream.into_stable(); - let result = (self.plugin.vtable.expand)(ffi_token_stream); - unsafe { ProcMacroResult::from_stable(result) } + // This must be manually freed with call to from_owned_stable. + let stable_token_stream = token_stream.into_stable(); + // Call FFI interface for code expansion. + // Note that `stable_result` has been allocated by the dynamic library. + let stable_result = (self.plugin.vtable.expand)(stable_token_stream.clone()); + // Free the memory allocated by the `stable_token_stream`. + // This will call `CString::from_raw` under the hood, to take ownership. + unsafe { + TokenStream::from_owned_stable(stable_token_stream); + }; + // Create Rust representation of the result. + // Note, that the memory still needs to be freed on the allocator side! + let result = unsafe { ProcMacroResult::from_stable(stable_result.clone()) }; + // Call FFI interface to free the `stable_result` that has been allocated by previous call. + (self.plugin.vtable.free_result)(stable_result); + // Return obtained result. + result } } type ExpandCode = extern "C" fn(StableTokenStream) -> StableProcMacroResult; +type FreeResult = extern "C" fn(StableProcMacroResult); struct VTableV0 { expand: RawSymbol, + free_result: RawSymbol, } impl VTableV0 { @@ -84,7 +107,14 @@ impl VTableV0 { .get(b"expand\0") .context("failed to load expand function for procedural macro")?; let expand = expand.into_raw(); - Ok(VTableV0 { expand }) + let free_result: Symbol<'_, FreeResult> = library + .get(b"free_result\0") + .context("failed to load free_result function for procedural macro")?; + let free_result = free_result.into_raw(); + Ok(VTableV0 { + expand, + free_result, + }) } } diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index fe9d183db..ae08c3d6e 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -69,8 +69,10 @@ fn simple_project(t: &impl PathChild) { .manifest_extra(r#"[cairo-plugin]"#) }) .lib_rs(indoc! {r#" - use cairo_lang_macro::{ProcMacroResult, TokenStream, attribute_macro}; + use cairo_lang_macro::{ProcMacroResult, TokenStream, attribute_macro, macro_commons}; + macro_commons!(); + #[attribute_macro] pub fn some_macro(token_stream: TokenStream) -> ProcMacroResult { let _code = token_stream.to_string(); @@ -110,8 +112,9 @@ fn compile_cairo_plugin() { .unwrap(); assert!( output.status.success(), - "{}", - String::from_utf8_lossy(&output.stderr) + "stdout={}\n stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), ); let stdout = String::from_utf8_lossy(&output.stdout).to_string(); assert!(stdout.contains("Compiling hello v1.0.0"));