From 614f5b42bec78d8491e0e4216d7670937ff81c9d Mon Sep 17 00:00:00 2001 From: Joe Hoyle Date: Mon, 13 Feb 2023 13:23:47 +0200 Subject: [PATCH 1/3] Support function args being passed by reference Currently it's not possible for a function to accept args by reference. This PR adds early support for doing so, by transforming any function args that are `&mut T` into args that are passed by reference. E.g.: ``` \#[php_function] fn my_function( arg: &mut Zval ) { arg.reference_mut().map( |zval| zval.set_bool(true) ); } ``` --- crates/macros/src/function.rs | 60 +++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/crates/macros/src/function.rs b/crates/macros/src/function.rs index 5e3def243..477d6fb49 100644 --- a/crates/macros/src/function.rs +++ b/crates/macros/src/function.rs @@ -26,6 +26,7 @@ pub struct Arg { pub ty: String, pub nullable: bool, pub default: Option, + pub as_ref: bool, } #[derive(Debug, Clone)] @@ -249,12 +250,19 @@ pub fn get_return_type(output_type: &ReturnType) -> Result) -> Self { + pub fn new( + name: String, + ty: String, + nullable: bool, + default: Option, + as_ref: bool, + ) -> Self { Self { name, ty, nullable, default, + as_ref, } } @@ -268,6 +276,7 @@ impl Arg { match ty { Type::Path(TypePath { path, .. }) => { let mut path = path.clone(); + let mut pass_by_ref = false; path.drop_lifetimes(); let seg = path.segments.last()?; @@ -283,9 +292,45 @@ impl Arg { None } }); + + // For for types that are `Option<&mut T>` to turn them into `Option<&T>`, marking the Arg as + // as "passed by reference". + let option = Some(seg) + .filter(|seg| seg.ident == "Option") + .and_then(|seg| { + if let PathArguments::AngleBracketed(args) = &seg.arguments { + args.args + .iter() + .find(|arg| matches!(arg, GenericArgument::Type(_))) + .map(|ga| { + let new_ga = match ga { + GenericArgument::Type(ty) => { + let _rtype = match ty { + Type::Reference(r) => { + let mut new_ref = r.clone(); + new_ref.mutability = None; + pass_by_ref = true; + Type::Reference(new_ref) + }, + _ => ty.clone(), + }; + GenericArgument::Type(_rtype) + }, + _ => ga.clone(), + }; + new_ga.to_token_stream().to_string() + }) + } else { + None + } + }); + let stringified = match result { Some(result) if is_return => result, - _ => path.to_token_stream().to_string(), + _ => match option { + Some(result) => result, + None => path.to_token_stream().to_string(), + }, }; Some(Arg::new( @@ -293,15 +338,23 @@ impl Arg { stringified, seg.ident == "Option" || default.is_some(), default, + pass_by_ref, )) } Type::Reference(ref_) => { // Returning references is invalid, so let's just create our arg + + // Change any `&mut T` into `&T` and set the `as_ref` attribute on the Arg + // to marked it as a "passed by ref" PHP argument. + let mut ref_ = ref_.clone(); + let is_mutable_ref = ref_.mutability.is_some(); + ref_.mutability = None; Some(Arg::new( name, ref_.to_token_stream().to_string(), false, default, + is_mutable_ref, )) } _ => None, @@ -361,6 +414,7 @@ impl Arg { let ty = self.get_type_ident(); let null = self.nullable.then(|| quote! { .allow_null() }); + let passed_by_ref = self.as_ref.then(|| quote! { .as_ref() }); let default = self.default.as_ref().map(|val| { quote! { .default(#val) @@ -368,7 +422,7 @@ impl Arg { }); quote! { - ::ext_php_rs::args::Arg::new(#name, #ty) #null #default + ::ext_php_rs::args::Arg::new(#name, #ty) #null #passed_by_ref #default } } } From 68059bdda157ce3e51f4b7353bc2d803b60048be Mon Sep 17 00:00:00 2001 From: Joe Hoyle Date: Thu, 13 Jul 2023 15:38:49 +0200 Subject: [PATCH 2/3] Fix mutable refs --- crates/macros/src/function.rs | 8 +------- guide/src/types/bool.md | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/macros/src/function.rs b/crates/macros/src/function.rs index 477d6fb49..4600e3440 100644 --- a/crates/macros/src/function.rs +++ b/crates/macros/src/function.rs @@ -343,18 +343,12 @@ impl Arg { } Type::Reference(ref_) => { // Returning references is invalid, so let's just create our arg - - // Change any `&mut T` into `&T` and set the `as_ref` attribute on the Arg - // to marked it as a "passed by ref" PHP argument. - let mut ref_ = ref_.clone(); - let is_mutable_ref = ref_.mutability.is_some(); - ref_.mutability = None; Some(Arg::new( name, ref_.to_token_stream().to_string(), false, default, - is_mutable_ref, + ref_.mutability.is_some(), )) } _ => None, diff --git a/guide/src/types/bool.md b/guide/src/types/bool.md index 1974ff305..498b40ec3 100644 --- a/guide/src/types/bool.md +++ b/guide/src/types/bool.md @@ -45,3 +45,17 @@ pub fn test_bool(input: bool) -> String { var_dump(test_bool(true)); // string(4) "Yes!" var_dump(test_bool(false)); // string(3) "No!" ``` + +## Rust example, taking by reference + +```rust,no_run +# #![cfg_attr(windows, feature(abi_vectorcall))] +# extern crate ext_php_rs; +# use ext_php_rs::prelude::*; +# use ext_php_rs::types; +#[php_function] +pub fn test_bool(input: &mut types::Zval) { + input.reference_mut().unwrap().set_bool(false); +} +# fn main() {} +``` From 64bf9e7366537a542eb5924c302806be039d7560 Mon Sep 17 00:00:00 2001 From: Joe Hoyle Date: Thu, 13 Jul 2023 15:44:14 +0200 Subject: [PATCH 3/3] fmt --- crates/macros/src/function.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/macros/src/function.rs b/crates/macros/src/function.rs index 4600e3440..5a389f1fe 100644 --- a/crates/macros/src/function.rs +++ b/crates/macros/src/function.rs @@ -293,8 +293,8 @@ impl Arg { } }); - // For for types that are `Option<&mut T>` to turn them into `Option<&T>`, marking the Arg as - // as "passed by reference". + // For for types that are `Option<&mut T>` to turn them into `Option<&T>`, + // marking the Arg as as "passed by reference". let option = Some(seg) .filter(|seg| seg.ident == "Option") .and_then(|seg| { @@ -311,11 +311,11 @@ impl Arg { new_ref.mutability = None; pass_by_ref = true; Type::Reference(new_ref) - }, + } _ => ty.clone(), }; GenericArgument::Type(_rtype) - }, + } _ => ga.clone(), }; new_ga.to_token_stream().to_string() @@ -327,7 +327,7 @@ impl Arg { let stringified = match result { Some(result) if is_return => result, - _ => match option { + _ => match option { Some(result) => result, None => path.to_token_stream().to_string(), },