Re: RFC: Type hinting revisited for PHP 5.3

Hi Ilia, On Wed, Jul 1, 2009 at 5:59 PM, Ilia Alshanetsky<ilia@prohost.org> wrote: > There has been quite a bit of discussion on this list, IRC, developer > meetings, etc... about introduction of type hinting to PHP. Most people RE your second patch, from http://ilia.ws/patch/type_hint_53_v2.txt Index: Zend/zend_compile.c =================================================================== RCS file: /repository/ZendEngine2/zend_compile.c,v retrieving revision 1.647.2.27.2.41.2.109 diff -u -p -a -d -u -r1.647.2.27.2.41.2.109 zend_compile.c --- Zend/zend_compile.c 7 Jun 2009 15:46:51 -0000 1.647.2.27.2.41.2.109 +++ Zend/zend_compile.c 4 Jul 2009 17:20:50 -0000 @@ -1511,10 +1514,9 @@ void zend_do_receive_arg(zend_uchar op, zend_error(E_COMPILE_ERROR, "Default value for parameters with a class type hint can only be NULL"); } } - } else { - cur_arg_info->array_type_hint = 1; - cur_arg_info->class_name = NULL; - cur_arg_info->class_name_len = 0; + break; + + case IS_ARRAY: So, to signify an array type hint, we used to use 1, and we now use IS_ARRAY, which is 4. I'm not 100% sure that's an ABI problem, but I just wanted to check. if (op == ZEND_RECV_INIT) { if (Z_TYPE(initialization->u.constant) == IS_NULL || (Z_TYPE(initialization->u.constant) == IS_CONSTANT && !strcasecmp(Z_STRVAL(initialization->u.constant), "NULL"))) { cur_arg_info->allow_null = 1; @@ -1522,6 +1524,56 @@ void zend_do_receive_arg(zend_uchar op, zend_error(E_COMPILE_ERROR, "Default value for parameters with array type hint can only be an array or NULL"); } } + break; + + /* scalar type hinting */ + case IS_BOOL: + case IS_STRING: + case IS_LONG: + case IS_DOUBLE: + case IS_RESOURCE: + case IS_NUMERIC: + case IS_SCALAR: + case IS_OBJECT: + if (op == ZEND_RECV_INIT) { + if (Z_TYPE(initialization->u.constant) != class_type->u.constant.type && Z_TYPE(initialization->u.constant) != IS_NULL) { + zend_error(E_COMPILE_ERROR, "Default value for parameters with %s type hint can only be %s or NULL", zend_get_type_by_const(class_type->u.constant.type), zend_get_type_by_const(class_type->u.constant.type)); That error says NULL is allowed for scalars, which I presume is wrong. + /* type forcing via cast */ + case FORCE_BOOL: + case FORCE_STRING: + case FORCE_LONG: + case FORCE_DOUBLE: + if (op == ZEND_RECV_INIT) { + switch (Z_TYPE(initialization->u.constant)) { + case IS_ARRAY: + case IS_OBJECT: + case IS_RESOURCE: + zend_error(E_COMPILE_ERROR, "Default value for parameters with a forced type of %s can only be a scalar", zend_get_type_by_const(class_type->u.constant.type)); I think a default parameter of the wrong type must signify a programmer error, so should be forbidden. Index: Zend/zend_execute.c =================================================================== RCS file: /repository/ZendEngine2/zend_execute.c,v retrieving revision 1.716.2.12.2.24.2.44 diff -u -p -a -d -u -r1.716.2.12.2.24.2.44 zend_execute.c --- Zend/zend_execute.c 4 Jun 2009 18:20:42 -0000 1.716.2.12.2.24.2.44 +++ Zend/zend_execute.c 4 Jul 2009 17:20:50 -0000 @@ -506,13 +506,82 @@ static inline int zend_verify_arg_type(z } } else if (cur_arg_info->array_type_hint) { if (!arg) { - return zend_verify_arg_error(zf, arg_num, cur_arg_info, "be an array", "", "none", "" TSRMLS_CC); + return zend_verify_arg_error(zf, arg_num, cur_arg_info, "be of the type ", zend_get_type_by_const(cur_arg_info->array_type_hint), "none", "" TSRMLS_CC); } - if (Z_TYPE_P(arg) != IS_ARRAY && (Z_TYPE_P(arg) != IS_NULL || !cur_arg_info->allow_null)) { - return zend_verify_arg_error(zf, arg_num, cur_arg_info, "be an array", "", zend_zval_type_name(arg), "" TSRMLS_CC); + + /* existing type already matches the hint or forced type */ + if (Z_TYPE_P(arg) == cur_arg_info->array_type_hint || Z_TYPE_P(arg) == (cur_arg_info->array_type_hint ^ (1<<7))) { + return 1; + } + + /* NULL type give, check if parameter is optional */ I cant parse this comment. + case IS_NUMERIC: + switch (Z_TYPE_P(arg)) { + case IS_STRING: + if (is_numeric_string(Z_STRVAL_P(arg), Z_STRLEN_P(arg), NULL, NULL, 0)) { + return 1; + } else { + goto type_error; + } + break; + case IS_BOOL: + case IS_LONG: + case IS_DOUBLE: + return 1; + default: + goto type_error; + } + break; I dont think bool should be in "numeric". Index: Zend/zend.h =================================================================== RCS file: /repository/ZendEngine2/zend.h,v retrieving revision 1.293.2.11.2.9.2.37 diff -u -p -a -d -u -r1.293.2.11.2.9.2.37 zend.h --- Zend/zend.h 17 Jun 2009 08:55:23 -0000 1.293.2.11.2.9.2.37 +++ Zend/zend.h 4 Jul 2009 17:20:50 -0000 @@ -536,6 +536,16 @@ typedef int (*zend_write_func_t)(const c +/* used for forcing method/function parameter type */ +#define FORCE_BOOL (IS_BOOL | (1<<7)) +#define FORCE_STRING (IS_STRING | (1<<7)) +#define FORCE_LONG (IS_LONG | (1<<7)) +#define FORCE_DOUBLE (IS_DOUBLE | (1<<7)) +#define FORCE_ARRAY (IS_ARRAY | (1<<7)) Can we have a macro for 1 << 7? It 's used in quite a few places. Index: Zend/zend_language_parser.y =================================================================== RCS file: /repository/ZendEngine2/zend_language_parser.y,v retrieving revision 1.160.2.4.2.8.2.35 diff -u -p -a -d -u -r1.160.2.4.2.8.2.35 zend_language_parser.y --- Zend/zend_language_parser.y 26 Mar 2009 12:37:17 -0000 1.160.2.4.2.8.2.35 +++ Zend/zend_language_parser.y 4 Jul 2009 17:20:50 -0000 @@ -128,6 +128,14 @@ %token T_DOUBLE_ARROW %token T_LIST %token T_ARRAY +%token T_BOOL_HINT +%token T_STRING_HINT +%token T_INT_HINT +%token T_DOUBLE_HINT +%token T_RESOURCE_HINT +%token T_NUMERIC_HINT +%token T_SCALAR_HINT +%token T_OBJECT_HINT %token T_CLASS_C %token T_METHOD_C %token T_FUNC_C Can you use T_BOOL_CHECK etc instead of T_BOOL_HINT? @@ -661,10 +682,10 @@ lexical_vars: lexical_var_list: - lexical_var_list ',' T_VARIABLE { zend_do_fetch_lexical_variable(&$3, 0 TSRMLS_CC); } - | lexical_var_list ',' '&' T_VARIABLE { zend_do_fetch_lexical_variable(&$4, 1 TSRMLS_CC); } - | T_VARIABLE { zend_do_fetch_lexical_variable(&$1, 0 TSRMLS_CC); } - | '&' T_VARIABLE { zend_do_fetch_lexical_variable(&$2, 1 TSRMLS_CC); } + lexical_var_list ',' T_VARIABLE { zend_do_fetch_lexical_variable(&$3, 0 TSRMLS_CC); } + | lexical_var_list ',' '&' T_VARIABLE { zend_do_fetch_lexical_variable(&$4, 1 TSRMLS_CC); } + | T_VARIABLE { zend_do_fetch_lexical_variable(&$1, 0 TSRMLS_CC); } + | '&' T_VARIABLE { zend_do_fetch_lexical_variable(&$2, 1 TSRMLS_CC); } ; I cant see what was changed here? Index: Zend/zend_language_scanner.l =================================================================== RCS file: /repository/ZendEngine2/zend_language_scanner.l,v retrieving revision 1.131.2.11.2.13.2.40 diff -u -p -a -d -u -r1.131.2.11.2.13.2.40 zend_language_scanner.l --- Zend/zend_language_scanner.l 5 May 2009 01:35:44 -0000 1.131.2.11.2.13.2.40 +++ Zend/zend_language_scanner.l 4 Jul 2009 17:20:50 -0000 @@ -1158,6 +1158,38 @@ NEWLINE ("\r"|"\n"|"\r\n") return T_ARRAY; } +<ST_IN_SCRIPTING>("bool"|"boolean") { + return T_BOOL_HINT; +} + +<ST_IN_SCRIPTING>("string"|"binary"|"unicode") { + return T_STRING_HINT; +} Someone asked on your last patch about that "unicode", with relation to 5.3. I think it might be a nice idea for forward compatability, so no objections, but I wanted to ask your plan for 5.3 with this. +<ST_IN_SCRIPTING>"object" { + return T_OBJECT_HINT; +} Great. There is a good argument for allowing "mixed", and a tiny argument for allowing "unset"/"null". It would be great if you could add these. I think that "callback" would be too hard, but if anyone comes up with an easy way, that would be cool too. Thanks for all your work on this, Paul -- Paul Biggar paul.biggar@gmail.com