Re: com php-src: Don't check argument types for internal functions without type hinting: Zend/zend_API.c Zend/zend_compile.h Zend/zend_vm_def.h
Zend/zend_vm_execute.h

Hi Nikita, I wouldn't object, but I ask you to wait a bit (at least till New Year). May be we would come back with proposal of some kind of scalar type hinting. Also, currently, zend_parse_parameters() takes significant time (about 4% on wordpress-3.6.1 home page) and may be it makes sense to use some chipper parameter parsing mechanism. Thanks. Dmitry. On Thu, Nov 28, 2013 at 12:06 PM, Nikita Popov <nikita.ppv@gmail.com> wrote: > On Thu, Nov 28, 2013 at 8:44 AM, Dmitry Stogov <dmitry@php.net> wrote: > >> Commit: 57c1335fec064d8022d3d86b01ceb4eea0a027a7 >> Author: Dmitry Stogov <dmitry@zend.com> Thu, 28 Nov 2013 >> 11:44:14 +0400 >> Parents: fcb98cbce8b85326763f352cb789581b55d85ec8 >> Branches: PHP-5.6 master >> >> Link: >> >> http://git.php.net/?p=php-src.git;a=commitdiff;h=57c1335fec064d8022d3d86b01ceb4eea0a027a7 >> >> Log: >> Don't check argument types for internal functions without type hinting >> >> Changed paths: >> M Zend/zend_API.c >> M Zend/zend_compile.h >> M Zend/zend_vm_def.h >> M Zend/zend_vm_execute.h >> >> >> Diff: >> diff --git a/Zend/zend_API.c b/Zend/zend_API.c >> index 56b1fda..23729ea 100644 >> --- a/Zend/zend_API.c >> +++ b/Zend/zend_API.c >> @@ -2138,6 +2138,19 @@ ZEND_API int >> zend_register_functions(zend_class_entry *scope, const zend_functio >> str_efree(lowercase_name); >> break; >> } >> + >> + /* If types of arguments have to be checked */ >> + if (reg_function->common.arg_info && >> reg_function->common.num_args) { >> + int i; >> + for (i = 0; i < reg_function->common.num_args; >> i++) { >> + if >> (reg_function->common.arg_info[i].class_name || >> + >> reg_function->common.arg_info[i].type_hint) { >> + reg_function->common.fn_flags |= >> ZEND_ACC_HAS_TYPE_HINTS; >> + break; >> + } >> + } >> + } >> + >> if (scope) { >> /* Look for ctor, dtor, clone >> * If it's an old-style constructor, store it >> only if we don't have >> diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h >> index 23c5b22..5e28488 100644 >> --- a/Zend/zend_compile.h >> +++ b/Zend/zend_compile.h >> @@ -212,6 +212,9 @@ typedef struct _zend_try_catch_element { >> #define ZEND_ACC_RETURN_REFERENCE 0x4000000 >> #define ZEND_ACC_DONE_PASS_TWO 0x8000000 >> >> +/* function has arguments with type hinting */ >> +#define ZEND_ACC_HAS_TYPE_HINTS 0x10000000 >> + >> char *zend_visibility_string(zend_uint fn_flags); >> >> >> diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h >> index 45cfe90..6eca6d0 100644 >> --- a/Zend/zend_vm_def.h >> +++ b/Zend/zend_vm_def.h >> @@ -1951,7 +1951,7 @@ ZEND_VM_HELPER(zend_do_fcall_common_helper, ANY, >> ANY) >> LOAD_OPLINE(); >> >> if (fbc->type == ZEND_INTERNAL_FUNCTION) { >> - if (fbc->common.arg_info) { >> + if (fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) { >> zend_uint i=0; >> zval **p = (zval**)EX(function_state).arguments; >> ulong arg_count = opline->extended_value; >> diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h >> index 107e460..dcb1183 100644 >> --- a/Zend/zend_vm_execute.h >> +++ b/Zend/zend_vm_execute.h >> @@ -530,7 +530,7 @@ static int ZEND_FASTCALL >> zend_do_fcall_common_helper_SPEC(ZEND_OPCODE_HANDLER_AR >> LOAD_OPLINE(); >> >> if (fbc->type == ZEND_INTERNAL_FUNCTION) { >> - if (fbc->common.arg_info) { >> + if (fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) { >> zend_uint i=0; >> zval **p = (zval**)EX(function_state).arguments; >> ulong arg_count = opline->extended_value; >> > > I wonder whether we should drop arginfo type checking for internal > functions altogether. Normally the checking is handled by zpp and doing the > same checks in arginfo is just a perf loss. To avoid that nearly no > internal functions specify type hints in arginfo. They are usually written > commented-outed next to a generic arg (e.g. see > > http://lxr.php.net/xref/PHP_TRUNK/ext/standard/basic_functions.c#218). > Disabling arginfo checks for internal functions would allow us to have > better reflection information about argument types without incurring a perf > loss. > > Nikita >