Skip to content

Commit 3979019

Browse files
davidcole1340davidcole1340
authored
Fixed ArgParser lifetimes (#100)
* Fixed `ArgParser` lifetimes Now stores a mutable reference to the underlying zval. Instead of passing the execution data to the arg parser, a vector of args is now passed to prevent a shared lifetime issue with the `$this` object. * Implememt from traits for classes
1 parent 3403cba commit 3979019

File tree

12 files changed

+428
-149
lines changed

12 files changed

+428
-149
lines changed

‎ext-php-rs-derive/src/class.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,11 @@ pub fn parser(args: AttributeArgs, mut input: ItemStruct) -> Result<TokenStream>
117117

118118
state.classes.insert(ident.to_string(), class);
119119

120-
Ok(quote! { #input })
120+
Ok(quote! {
121+
#input
122+
123+
::ext_php_rs::class_derives!(#ident);
124+
})
121125
}
122126

123127
#[derive(Debug)]

‎ext-php-rs-derive/src/function.rs‎

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,12 @@ pub fn parser(args: AttributeArgs, input: ItemFn) -> Result<(TokenStream, Functi
5656
let args = build_args(inputs, &attr_args.defaults)?;
5757
let optional = find_optional_parameter(args.iter(), attr_args.optional);
5858
let arg_definitions = build_arg_definitions(&args);
59-
let arg_parser = build_arg_parser(args.iter(), &optional, &quote! { return; })?;
59+
let arg_parser = build_arg_parser(
60+
args.iter(),
61+
&optional,
62+
&quote! { return; },
63+
ParserType::Function,
64+
)?;
6065
let arg_accessors = build_arg_accessors(&args);
6166

6267
let return_type = get_return_type(output)?;
@@ -154,26 +159,29 @@ pub fn find_optional_parameter<'a>(
154159
optional
155160
}
156161

162+
pub enum ParserType {
163+
Function,
164+
Method,
165+
StaticMethod,
166+
}
167+
157168
pub fn build_arg_parser<'a>(
158169
args: impl Iterator<Item = &'a Arg>,
159170
optional: &Option<String>,
160171
ret: &TokenStream,
172+
ty: ParserType,
161173
) -> Result<TokenStream> {
162174
let mut rest_optional = false;
163175

164176
let args = args
165177
.map(|arg| {
166178
let name = arg.get_name_ident();
167-
let prelude = if let Some(optional) = optional {
168-
if *optional == arg.name {
169-
rest_optional = true;
170-
quote! { .not_required() }
171-
} else {
172-
quote! {}
173-
}
179+
let prelude = optional.as_ref().and_then(|opt| if *opt == arg.name {
180+
rest_optional = true;
181+
Some(quote! { .not_required() })
174182
} else {
175-
quote! {}
176-
};
183+
None
184+
});
177185

178186
if rest_optional && !arg.nullable && arg.default.is_none() {
179187
bail!(
@@ -188,15 +196,37 @@ pub fn build_arg_parser<'a>(
188196
}
189197
})
190198
.collect::<Result<Vec<_>>>()?;
199+
let (parser, this) = match ty {
200+
ParserType::Function | ParserType::StaticMethod => {
201+
(quote! { let parser = ex.parser(); }, None)
202+
}
203+
ParserType::Method => (
204+
quote! { let (parser, this) = ex.parser_method::<Self>(); },
205+
Some(quote! {
206+
let this = match this {
207+
Some(this) => this,
208+
None => {
209+
::ext_php_rs::php::exceptions::PhpException::default("Failed to retrieve reference to `$this`".into())
210+
.throw()
211+
.unwrap();
212+
return;
213+
},
214+
};
215+
}),
216+
),
217+
};
191218

192219
Ok(quote! {
193-
let parser = ::ext_php_rs::php::args::ArgParser::new(ex)
220+
#parser
221+
let parser = parser
194222
#(#args)*
195223
.parse();
196224

197225
if parser.is_err() {
198226
#ret
199227
}
228+
229+
#this
200230
})
201231
}
202232

‎ext-php-rs-derive/src/method.rs‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use quote::ToTokens;
33
use std::collections::HashMap;
44

55
use crate::{
6-
function,
6+
function::{self, ParserType},
77
impl_::{parse_attribute, ParsedAttribute, PropAttrTy, RenameRule, Visibility},
88
};
99
use proc_macro2::{Ident, Span, TokenStream};
@@ -131,7 +131,16 @@ pub fn parser(input: &mut ImplItemMethod, rename_rule: RenameRule) -> Result<Par
131131
optional,
132132
);
133133
let (arg_definitions, is_static) = build_arg_definitions(&args);
134-
let arg_parser = build_arg_parser(args.iter(), &optional, &bail)?;
134+
let arg_parser = build_arg_parser(
135+
args.iter(),
136+
&optional,
137+
&bail,
138+
if is_static {
139+
ParserType::StaticMethod
140+
} else {
141+
ParserType::Method
142+
},
143+
)?;
135144
let arg_accessors = build_arg_accessors(&args, &bail);
136145
let this = if is_static {
137146
quote! { Self:: }
@@ -225,38 +234,32 @@ fn build_args(
225234
fn build_arg_definitions(args: &[Arg]) -> (Vec<TokenStream>, bool) {
226235
let mut _static = true;
227236

228-
(args.iter()
229-
.map(|ty| match ty {
230-
Arg::Receiver(mutability) => {
231-
let mutability = mutability.then(|| quote! { mut });
232-
_static = false;
233-
234-
quote! {
235-
// SAFETY: We are calling this on an execution data from a class method.
236-
let #mutability this = match unsafe { ex.get_object::<Self>() } {
237-
Some(this) => this,
238-
None => return ::ext_php_rs::php::exceptions::throw(
239-
::ext_php_rs::php::class::ClassEntry::exception(),
240-
"Failed to retrieve reference to object function was called on."
241-
).expect("Failed to throw exception: Failed to retrieve reference to object function was called on."),
242-
};
237+
(
238+
args.iter()
239+
.filter_map(|ty| match ty {
240+
Arg::Receiver(_) => {
241+
_static = false;
242+
243+
None
243244
}
244-
}
245-
Arg::Typed(arg) => {
246-
let ident = arg.get_name_ident();
247-
let definition = arg.get_arg_definition();
248-
quote! {
249-
let mut #ident = #definition;
245+
Arg::Typed(arg) => {
246+
let ident = arg.get_name_ident();
247+
let definition = arg.get_arg_definition();
248+
Some(quote! {
249+
let mut #ident = #definition;
250+
})
250251
}
251-
},
252-
})
253-
.collect(), _static)
252+
})
253+
.collect(),
254+
_static,
255+
)
254256
}
255257

256258
fn build_arg_parser<'a>(
257259
args: impl Iterator<Item = &'a Arg>,
258260
optional: &Option<String>,
259261
ret: &TokenStream,
262+
ty: ParserType,
260263
) -> Result<TokenStream> {
261264
function::build_arg_parser(
262265
args.filter_map(|arg| match arg {
@@ -265,6 +268,7 @@ fn build_arg_parser<'a>(
265268
}),
266269
optional,
267270
ret,
271+
ty,
268272
)
269273
}
270274

‎src/errors.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pub enum Error {
2121
/// The enum carries two integers - the first representing the minimum
2222
/// number of arguments expected, and the second representing the number of
2323
/// arguments that were received.
24-
IncorrectArguments(u32, u32),
24+
IncorrectArguments(usize, usize),
2525
/// There was an error converting a Zval into a primitive type.
2626
///
2727
/// The enum carries the data type of the Zval.

‎src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ pub use ext_php_rs_derive::php_extern;
127127
///
128128
/// pub extern "C" fn _internal_php_hello(ex: &mut ExecutionData, retval: &mut Zval) {
129129
/// let mut name = Arg::new("name", <String as FromZval>::TYPE);
130-
/// let parser = ArgParser::new(ex)
130+
/// let parser = ex.parser()
131131
/// .arg(&mut name)
132132
/// .parse();
133133
///

‎src/macros.rs‎

Lines changed: 146 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,7 @@ macro_rules! call_user_func {
112112
#[macro_export]
113113
macro_rules! parse_args {
114114
($ed: expr, $($arg: expr),*) => {{
115-
use $crate::php::args::ArgParser;
116-
117-
let parser = ArgParser::new($ed)
115+
let parser = $ed.parser()
118116
$(.arg(&mut $arg))*
119117
.parse();
120118
if parser.is_err() {
@@ -125,7 +123,7 @@ macro_rules! parse_args {
125123
($ed: expr, $($arg: expr),* ; $($opt: expr),*) => {{
126124
use $crate::php::args::ArgParser;
127125

128-
let parser = ArgParser::new($ed)
126+
let parser = $ed.parser()
129127
$(.arg(&mut $arg))*
130128
.not_required()
131129
$(.arg(&mut $opt))*
@@ -163,3 +161,147 @@ macro_rules! throw {
163161
return;
164162
};
165163
}
164+
165+
/// Implements a set of traits required to convert types that implement [`RegisteredClass`] to and
166+
/// from [`ZendObject`]s and [`Zval`]s. Generally, this macro should not be called directly, as it
167+
/// is called on any type that uses the [`#[php_class]`] macro.
168+
///
169+
/// The following traits are implemented:
170+
///
171+
/// * `FromZendObject for &'a T`
172+
/// * `FromZendObjectMut for &'a mut T`
173+
/// * `FromZval for &'a T`
174+
/// * `FromZvalMut for &'a mut T`
175+
/// * `IntoZendObject for T`
176+
/// * `IntoZval for T`
177+
///
178+
/// These implementations are required while we wait on the stabilisation of specialisation.
179+
///
180+
/// # Examples
181+
///
182+
/// ```
183+
/// # use ext_php_rs::{php::types::{zval::{Zval, IntoZval, FromZval}, object::{ZendObject, RegisteredClass}}};
184+
/// use ext_php_rs::class_derives;
185+
///
186+
/// struct Test {
187+
/// a: i32,
188+
/// b: i64
189+
/// }
190+
///
191+
/// impl RegisteredClass for Test {
192+
/// const CLASS_NAME: &'static str = "Test";
193+
///
194+
/// const CONSTRUCTOR: Option<ext_php_rs::php::types::object::ConstructorMeta<Self>> = None;
195+
///
196+
/// fn get_metadata() -> &'static ext_php_rs::php::types::object::ClassMetadata<Self> {
197+
/// todo!()
198+
/// }
199+
///
200+
/// fn get_properties<'a>(
201+
/// ) -> std::collections::HashMap<&'static str, ext_php_rs::php::types::props::Property<'a, Self>>
202+
/// {
203+
/// todo!()
204+
/// }
205+
/// }
206+
///
207+
/// class_derives!(Test);
208+
///
209+
/// fn into_zval_test() -> Zval {
210+
/// let x = Test { a: 5, b: 10 };
211+
/// x.into_zval(false).unwrap()
212+
/// }
213+
///
214+
/// fn from_zval_test<'a>(zv: &'a Zval) -> &'a Test {
215+
/// <&Test>::from_zval(zv).unwrap()
216+
/// }
217+
/// ```
218+
///
219+
/// [`RegisteredClass`]: crate::php::types::object::RegisteredClass
220+
/// [`ZendObject`]: crate::php::types::object::ZendObject
221+
/// [`Zval`]: crate::php::types::zval::Zval
222+
/// [`#[php_class]`]: crate::php_class
223+
#[macro_export]
224+
macro_rules! class_derives {
225+
($type: ty) => {
226+
impl<'a> $crate::php::types::object::FromZendObject<'a> for &'a $type {
227+
#[inline]
228+
fn from_zend_object(
229+
obj: &'a $crate::php::types::object::ZendObject,
230+
) -> $crate::errors::Result<Self> {
231+
let obj = $crate::php::types::object::ZendClassObject::<$type>::from_zend_obj(obj)
232+
.ok_or($crate::errors::Error::InvalidScope)?;
233+
Ok(&**obj)
234+
}
235+
}
236+
237+
impl<'a> $crate::php::types::object::FromZendObjectMut<'a> for &'a mut $type {
238+
#[inline]
239+
fn from_zend_object_mut(
240+
obj: &'a mut $crate::php::types::object::ZendObject,
241+
) -> $crate::errors::Result<Self> {
242+
let obj =
243+
$crate::php::types::object::ZendClassObject::<$type>::from_zend_obj_mut(obj)
244+
.ok_or($crate::errors::Error::InvalidScope)?;
245+
Ok(&mut **obj)
246+
}
247+
}
248+
249+
impl<'a> $crate::php::types::zval::FromZval<'a> for &'a $type {
250+
const TYPE: $crate::php::enums::DataType = $crate::php::enums::DataType::Object(Some(
251+
<$type as $crate::php::types::object::RegisteredClass>::CLASS_NAME,
252+
));
253+
254+
#[inline]
255+
fn from_zval(zval: &'a $crate::php::types::zval::Zval) -> ::std::option::Option<Self> {
256+
<Self as $crate::php::types::object::FromZendObject>::from_zend_object(
257+
zval.object()?,
258+
)
259+
.ok()
260+
}
261+
}
262+
263+
impl<'a> $crate::php::types::zval::FromZvalMut<'a> for &'a mut $type {
264+
const TYPE: $crate::php::enums::DataType = $crate::php::enums::DataType::Object(Some(
265+
<$type as $crate::php::types::object::RegisteredClass>::CLASS_NAME,
266+
));
267+
268+
#[inline]
269+
fn from_zval_mut(
270+
zval: &'a mut $crate::php::types::zval::Zval,
271+
) -> ::std::option::Option<Self> {
272+
<Self as $crate::php::types::object::FromZendObjectMut>::from_zend_object_mut(
273+
zval.object_mut()?,
274+
)
275+
.ok()
276+
}
277+
}
278+
279+
impl $crate::php::types::object::IntoZendObject for $type {
280+
#[inline]
281+
fn into_zend_object(
282+
self,
283+
) -> $crate::errors::Result<
284+
$crate::php::boxed::ZBox<$crate::php::types::object::ZendObject>,
285+
> {
286+
Ok($crate::php::types::object::ZendClassObject::new(self).into())
287+
}
288+
}
289+
290+
impl $crate::php::types::zval::IntoZval for $type {
291+
const TYPE: $crate::php::enums::DataType = $crate::php::enums::DataType::Object(Some(
292+
<$type as $crate::php::types::object::RegisteredClass>::CLASS_NAME,
293+
));
294+
295+
#[inline]
296+
fn set_zval(
297+
self,
298+
zv: &mut $crate::php::types::zval::Zval,
299+
persistent: bool,
300+
) -> $crate::errors::Result<()> {
301+
use $crate::php::types::object::IntoZendObject;
302+
303+
self.into_zend_object()?.set_zval(zv, persistent)
304+
}
305+
}
306+
};
307+
}

0 commit comments

Comments
 (0)