Hi Ilia,

On Wed, Jul 1, 2009 at 5:59 PM, Ilia Alshanetsky<i...@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.big...@gmail.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to