Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-12 Thread Nikita Popov
On Mon, Mar 11, 2013 at 8:42 PM, Dmitry Stogov dmi...@zend.com wrote: On Mon, Mar 11, 2013 at 10:27 PM, Nikita Popov nikita@gmail.comwrote: On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov dmi...@zend.com wrote: Hi Nikita, The patch looks good. I have just few comments - In

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-12 Thread Dmitry Stogov
Hi Nikita, I suppose it must fine now, but let me take a quick look tomorrow morning. Thanks. Dmitry. On Tue, Mar 12, 2013 at 8:43 PM, Nikita Popov nikita@gmail.com wrote: On Mon, Mar 11, 2013 at 8:42 PM, Dmitry Stogov dmi...@zend.com wrote: On Mon, Mar 11, 2013 at 10:27 PM, Nikita

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-11 Thread Dmitry Stogov
Hi Nikita, Thanks. I'll review it today. Dmitry. On Sun, Mar 10, 2013 at 1:47 AM, Nikita Popov nikita@gmail.com wrote: On Wed, Mar 6, 2013 at 6:28 PM, Dmitry Stogov dmi...@zend.com wrote: I wonder what would be a good way to avoid allocating a temporary zval for the key and freeing it

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-11 Thread Dmitry Stogov
Hi Nikita, The patch looks good. I have just few comments - In ZEND_FE_FETCH handler PLAIN_OBJECT may have only STRING keys. I didn't get why you added unreachable code for INT and NULL. - At first, I fought, that it might be a good idea to change zend_user_it_get_current_key() to return

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-11 Thread Nikita Popov
On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov dmi...@zend.com wrote: Hi Nikita, The patch looks good. I have just few comments - In ZEND_FE_FETCH handler PLAIN_OBJECT may have only STRING keys. I didn't get why you added unreachable code for INT and NULL. You are right about the NULL

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-11 Thread Dmitry Stogov
On Mon, Mar 11, 2013 at 10:27 PM, Nikita Popov nikita@gmail.com wrote: On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov dmi...@zend.com wrote: Hi Nikita, The patch looks good. I have just few comments - In ZEND_FE_FETCH handler PLAIN_OBJECT may have only STRING keys. I didn't get why

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-09 Thread Nikita Popov
On Wed, Mar 6, 2013 at 6:28 PM, Dmitry Stogov dmi...@zend.com wrote: I wonder what would be a good way to avoid allocating a temporary zval for the key and freeing it again. Do you think it would be okay to pass EX_T((opline+1)-result.var).tmp_var into -get_current_key() so the value can be

[PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-06 Thread Nikita Popov
On Wed, Feb 27, 2013 at 8:33 PM, Nikita Popov nikita@gmail.com wrote: Hi internals! I've opened the voting the the foreach-keys RFC: https://wiki.php.net/rfc/foreach-non-scalar-keys#vote The discussion for this RFC can be found here: http://markmail.org/message/rzoacqaesxbd67lu The

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-06 Thread Dmitry Stogov
Hi Nikita, few notes about the patch... - you may avoid estrndup() in zend_hash_current_key_zval_ex() for interned strings. - I didn't completely get why did you change the key operand type from IS_TMP_VAR to IS_VAR and how it affects performance As I understood now you need to allocate new

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-06 Thread Nikita Popov
On Wed, Mar 6, 2013 at 5:41 PM, Dmitry Stogov dmi...@zend.com wrote: Hi Nikita, few notes about the patch... - you may avoid estrndup() in zend_hash_current_key_zval_ex() for interned strings. Good idea, I'll do that. - I didn't completely get why did you change the key operand type

Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-06 Thread Dmitry Stogov
On Wed, Mar 6, 2013 at 9:16 PM, Nikita Popov nikita@gmail.com wrote: On Wed, Mar 6, 2013 at 5:41 PM, Dmitry Stogov dmi...@zend.com wrote: Hi Nikita, few notes about the patch... - you may avoid estrndup() in zend_hash_current_key_zval_ex() for interned strings. Good idea, I'll do

[PHP-DEV] 回复: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach

2013-03-06 Thread Reeze Xia
Hi Nikita, I got some test failure with the patch, one test failure and some memory leaks. see: https://gist.github.com/reeze/5101596 -- reeze | reeze.cn 已使用 Sparrow (http://www.sparrowmailapp.com/?sig) 在 2013年3月7日星期四,上午1:28,Dmitry Stogov 写道: On Wed, Mar 6, 2013 at 9:16 PM, Nikita