Skip to content

Commit 2e01785

Browse files
brettmcbrettmc
authored
fix invocation of functions with type hint (#220)
1 parent 84adb0b commit 2e01785

File tree

5 files changed

+91
-51
lines changed

5 files changed

+91
-51
lines changed

‎phper-test/src/cli.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ pub fn test_php_scripts_with_condition(
179179
"execute php test command");
180180

181181
if !condition(output) {
182+
eprintln!("--- stdout ---\n{}", stdout);
183+
eprintln!("--- stderr ---\n{}", stderr);
182184
panic!("test php file `{}` failed", path);
183185
}
184186
}

‎phper/src/functions.rs‎

Lines changed: 4 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,11 @@ use std::{
2828
collections::HashMap,
2929
ffi::{CStr, CString},
3030
marker::PhantomData,
31-
mem::{ManuallyDrop, size_of, transmute, zeroed},
31+
mem::{ManuallyDrop, transmute, zeroed},
3232
ptr::{self, null_mut},
3333
rc::Rc,
34-
slice,
3534
};
3635

37-
/// Used to mark the arguments obtained by the invoke function as mysterious
38-
/// codes from phper
39-
const INVOKE_MYSTERIOUS_CODE: &[u8] = b"PHPER";
40-
4136
/// Used to find the handler in the invoke function.
4237
pub(crate) type HandlerMap = HashMap<(Option<CString>, CString), Rc<dyn Callable>>;
4338

@@ -371,9 +366,6 @@ impl FunctionEntry {
371366

372367
infos.push(zeroed::<zend_internal_arg_info>());
373368

374-
// Will be checked in `invoke` function.
375-
infos.push(Self::create_mysterious_code());
376-
377369
let raw_handler = handler.as_ref().map(|_| invoke as _);
378370

379371
if let Some(handler) = handler {
@@ -397,22 +389,12 @@ impl FunctionEntry {
397389
}
398390
}
399391
}
400-
401-
unsafe fn create_mysterious_code() -> zend_internal_arg_info {
402-
unsafe {
403-
let mut mysterious_code = [0u8; size_of::<zend_internal_arg_info>()];
404-
for (i, n) in INVOKE_MYSTERIOUS_CODE.iter().enumerate() {
405-
mysterious_code[i] = *n;
406-
}
407-
transmute(mysterious_code)
408-
}
409-
}
410392
}
411393

412394
/// Builder for registering php function.
413395
pub struct FunctionEntity {
414-
name: CString,
415-
handler: Rc<dyn Callable>,
396+
pub(crate) name: CString,
397+
pub(crate) handler: Rc<dyn Callable>,
416398
arguments: Vec<Argument>,
417399
return_type: Option<ReturnType>,
418400
}
@@ -797,7 +779,6 @@ impl ZFunc {
797779
pub(crate) union CallableTranslator {
798780
pub(crate) callable: *const dyn Callable,
799781
pub(crate) internal_arg_info: zend_internal_arg_info,
800-
pub(crate) arg_info: zend_arg_info,
801782
}
802783

803784
/// The entry for all registered PHP functions.
@@ -806,25 +787,7 @@ unsafe extern "C" fn invoke(execute_data: *mut zend_execute_data, return_value:
806787
let execute_data = ExecuteData::from_mut_ptr(execute_data);
807788
let return_value = ZVal::from_mut_ptr(return_value);
808789

809-
let num_args = execute_data.common_num_args();
810-
let arg_info = execute_data.common_arg_info();
811-
812-
// should be mysterious code
813-
let mysterious_arg_info = arg_info.offset((num_args + 1) as isize);
814-
let mysterious_code = slice::from_raw_parts(
815-
mysterious_arg_info as *const u8,
816-
INVOKE_MYSTERIOUS_CODE.len(),
817-
);
818-
819-
let handler = if mysterious_code == INVOKE_MYSTERIOUS_CODE {
820-
// hiddden real handler
821-
let last_arg_info = arg_info.offset((num_args + 2) as isize);
822-
let translator = CallableTranslator {
823-
arg_info: *last_arg_info,
824-
};
825-
let handler = translator.callable;
826-
handler.as_ref().expect("handler is null")
827-
} else {
790+
let handler = {
828791
let function_name = execute_data
829792
.func()
830793
.get_function_name()

‎phper/src/modules.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ unsafe extern "C" fn module_startup(_type: c_int, module_number: c_int) -> c_int
5555
interface_entity.init();
5656
}
5757

58+
for function_entity in &module.function_entities {
59+
module.handler_map.insert(
60+
(None, function_entity.name.clone()),
61+
function_entity.handler.clone(),
62+
);
63+
}
64+
5865
for class_entity in &module.class_entities {
5966
let ce = class_entity.init();
6067
class_entity.declare_properties(ce);

‎tests/integration/src/typehints.rs‎

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@
99
// See the Mulan PSL v2 for more details.
1010

1111
use phper::{
12+
arrays::ZArray,
1213
classes::{ClassEntity, Interface, InterfaceEntity, StateClass, Visibility},
1314
functions::{Argument, ReturnType},
1415
modules::Module,
1516
types::{ArgumentTypeHint, ReturnTypeHint},
1617
values::ZVal,
1718
};
19+
use std::convert::Infallible;
1820

1921
const I_FOO: &str = r"IntegrationTest\TypeHints\IFoo";
2022

@@ -26,6 +28,45 @@ pub fn integrate(module: &mut Module) {
2628
module.add_class(make_arg_typehint_class());
2729
module.add_class(make_return_typehint_class());
2830
module.add_class(make_arg_default_value_class());
31+
module
32+
.add_function(
33+
"integration_function_return_bool",
34+
|_| -> Result<bool, Infallible> { Ok(true) },
35+
)
36+
.return_type(ReturnType::new(ReturnTypeHint::Bool));
37+
module
38+
.add_function(
39+
"integration_function_return_int",
40+
|_| -> Result<i64, Infallible> { Ok(42) },
41+
)
42+
.return_type(ReturnType::new(ReturnTypeHint::Int));
43+
module
44+
.add_function(
45+
"integration_function_return_float",
46+
|_| -> Result<f64, Infallible> { Ok(1.234) },
47+
)
48+
.return_type(ReturnType::new(ReturnTypeHint::Float));
49+
module
50+
.add_function(
51+
"integration_function_return_string",
52+
|_| -> Result<&'static str, Infallible> { Ok("phper") },
53+
)
54+
.return_type(ReturnType::new(ReturnTypeHint::String));
55+
module
56+
.add_function(
57+
"integration_function_return_array",
58+
|_| -> Result<ZArray, Infallible> { Ok(ZArray::new()) },
59+
)
60+
.return_type(ReturnType::new(ReturnTypeHint::Array));
61+
module
62+
.add_function(
63+
"integration_function_return_mixed",
64+
|_| -> Result<ZVal, Infallible> { Ok(ZVal::from(1.23)) },
65+
)
66+
.return_type(ReturnType::new(ReturnTypeHint::Mixed));
67+
module
68+
.add_function("integration_function_return_void", |_| phper::ok(()))
69+
.return_type(ReturnType::new(ReturnTypeHint::Void));
2970
module
3071
.add_function("integration_function_typehints", |_| phper::ok(()))
3172
.argument(
@@ -455,11 +496,9 @@ fn make_return_typehint_class() -> ClassEntity<()> {
455496
.return_type(ReturnType::new(ReturnTypeHint::Null));
456497

457498
class
458-
.add_method(
459-
"returnString",
460-
Visibility::Public,
461-
move |_, _| phper::ok(()),
462-
)
499+
.add_method("returnString", Visibility::Public, move |_, _| {
500+
phper::ok("phper")
501+
})
463502
.return_type(ReturnType::new(ReturnTypeHint::String));
464503

465504
class
@@ -469,7 +508,9 @@ fn make_return_typehint_class() -> ClassEntity<()> {
469508
.return_type(ReturnType::new(ReturnTypeHint::String).allow_null());
470509

471510
class
472-
.add_method("returnBool", Visibility::Public, move |_, _| phper::ok(()))
511+
.add_method("returnBool", Visibility::Public, move |_, _| {
512+
phper::ok(true)
513+
})
473514
.return_type(ReturnType::new(ReturnTypeHint::Bool));
474515

475516
class
@@ -479,7 +520,7 @@ fn make_return_typehint_class() -> ClassEntity<()> {
479520
.return_type(ReturnType::new(ReturnTypeHint::Bool).allow_null());
480521

481522
class
482-
.add_method("returnInt", Visibility::Public, move |_, _| phper::ok(()))
523+
.add_method("returnInt", Visibility::Public, move |_, _| phper::ok(42))
483524
.return_type(ReturnType::new(ReturnTypeHint::Int));
484525

485526
class
@@ -489,7 +530,9 @@ fn make_return_typehint_class() -> ClassEntity<()> {
489530
.return_type(ReturnType::new(ReturnTypeHint::Int).allow_null());
490531

491532
class
492-
.add_method("returnFloat", Visibility::Public, move |_, _| phper::ok(()))
533+
.add_method("returnFloat", Visibility::Public, move |_, _| {
534+
phper::ok(1.234)
535+
})
493536
.return_type(ReturnType::new(ReturnTypeHint::Float));
494537

495538
class
@@ -499,7 +542,9 @@ fn make_return_typehint_class() -> ClassEntity<()> {
499542
.return_type(ReturnType::new(ReturnTypeHint::Float).allow_null());
500543

501544
class
502-
.add_method("returnArray", Visibility::Public, move |_, _| phper::ok(()))
545+
.add_method("returnArray", Visibility::Public, move |_, _| {
546+
phper::ok(ZArray::new())
547+
})
503548
.return_type(ReturnType::new(ReturnTypeHint::Array));
504549

505550
class

‎tests/integration/tests/php/typehints.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,4 +209,27 @@ public function setValue($value): void {
209209
echo "PASS" . PHP_EOL;
210210
}
211211
assert_eq('void', $reflection->getReturnType()->getName(), 'integration_function_typehints return type is void');
212-
}
212+
}
213+
214+
//invoke type-hinted functions to exercise handlers
215+
echo PHP_EOL . 'Testing return type-hinted function invocation' . PHP_EOL;
216+
assert_true(integration_function_return_bool());
217+
assert_eq(42, integration_function_return_int());
218+
assert_eq(1.234, integration_function_return_float());
219+
assert_eq('phper', integration_function_return_string());
220+
assert_eq(array(), integration_function_return_array());
221+
assert_eq(1.23, integration_function_return_mixed());
222+
223+
//invoke type-hinted class methods to exercise handlers
224+
echo PHP_EOL . 'Testing return type-hinted method invocation' . PHP_EOL;
225+
$cls = new \IntegrationTest\TypeHints\ReturnTypeHintTest();
226+
assert_eq(true, $cls->returnBool(), 'returnBool');
227+
assert_eq(null, $cls->returnBoolNullable(), 'returnBoolNullable');
228+
assert_eq(42, $cls->returnInt(), 'returnInt');
229+
assert_eq(null, $cls->returnIntNullable(), 'returnIntNullable');
230+
assert_eq(1.234, $cls->returnFloat(), 'returnFloat');
231+
assert_eq(null, $cls->returnFloatNullable(), 'returnFloatNullable');
232+
assert_eq('phper', $cls->returnString(), 'returnString');
233+
assert_eq(null, $cls->returnStringNullable(), 'returnStringNullable');
234+
assert_eq(array(), $cls->returnArray(), 'returnArray');
235+
assert_eq(null, $cls->returnArrayNullable(), 'returnArrayNullable');

0 commit comments

Comments
 (0)