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 wrote: > On Mon, Mar 11, 2013 at 8:42 PM, Dmitry Stogov wrote: > >> >> >> On Mon, Mar 11, 2013 at 10:27 PM, Nikita Popov wrote: >> >>> On Mon, Ma

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 wrote: > > > On Mon, Mar 11, 2013 at 10:27 PM, Nikita Popov wrote: > >> On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov wrote: >> >>> Hi Nikita, >>> >>> The patch looks good. I have just few comments >>> >>> - In ZEND_FE_FETCH handler PLAIN_OBJECT ma

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 wrote: > On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov 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 co

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 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 case, that c

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 SUCCESS

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

2013-03-10 Thread Dmitry Stogov
Hi Nikita, Thanks. I'll review it today. Dmitry. On Sun, Mar 10, 2013 at 1:47 AM, Nikita Popov wrote: > On Wed, Mar 6, 2013 at 6:28 PM, Dmitry Stogov 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 wo

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 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 writ

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 wrote: > On Wed, Mar 6, 2013 at 5:41 PM, Dmitry Stogov 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 d

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 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 from >

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 zv