Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:44706 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 78066 invoked from network); 4 Jul 2009 18:49:30 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 4 Jul 2009 18:49:30 -0000 Authentication-Results: pb1.pair.com smtp.mail=paul.biggar@gmail.com; spf=pass; sender-id=pass Authentication-Results: pb1.pair.com header.from=paul.biggar@gmail.com; sender-id=pass; domainkeys=bad Received-SPF: pass (pb1.pair.com: domain gmail.com designates 72.14.220.152 as permitted sender) DomainKey-Status: bad X-DomainKeys: Ecelerity dk_validate implementing draft-delany-domainkeys-base-01 X-PHP-List-Original-Sender: paul.biggar@gmail.com X-Host-Fingerprint: 72.14.220.152 fg-out-1718.google.com Received: from [72.14.220.152] ([72.14.220.152:43086] helo=fg-out-1718.google.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id FE/EB-06257-834AF4A4 for ; Sat, 04 Jul 2009 14:49:29 -0400 Received: by fg-out-1718.google.com with SMTP id 13so324079fge.0 for ; Sat, 04 Jul 2009 11:49:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :from:date:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=nc2clxgQLrGqLnwNqxDI65+iEVEI2otPaQtFsV4e0aA=; b=bHF+kiCqgNSSuFUZh6aLsFgdytcBrkG5rC34X1/ZynrW+gRDbiKWsvKbBAGQPVHuYX dRo9UiRx35xKJdgFo3FKIKVTLNSzP5m6k4jL2AYeCRS5EM74Dke3F1Xhk4/mdp4FIFRF +o8WghxDHBpFXJ10CnlxYdxYecIsPZSrELt0A= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=f5T8OVtkqCu2MV6z0mGZdbS2z4DysaEY4tQZ16W/WZY6nRi3h/YN31odXp3XtkpZyb nhHkKWU6r5huwgRqD9Iy1Rdr/chYfcX1xFrluupEbgQ9o/qJGL6V1+a1c2DZ7P0eH525 4GbCmkAfR9xf68GmM1x30sWrjZrLjMeG9X5+4= MIME-Version: 1.0 Received: by 10.86.23.16 with SMTP id 16mr1431916fgw.75.1246733366051; Sat, 04 Jul 2009 11:49:26 -0700 (PDT) In-Reply-To: References: Date: Sat, 4 Jul 2009 19:49:06 +0100 Message-ID: To: Ilia Alshanetsky Cc: PHP internals Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [PHP-DEV] RFC: Type hinting revisited for PHP 5.3 From: paul.biggar@gmail.com (Paul Biggar) Hi Ilia, On Wed, Jul 1, 2009 at 5:59 PM, Ilia Alshanetsky 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; } +("bool"|"boolean") { + return T_BOOL_HINT; +} + +("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. +"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