Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:66925
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
To: internals@lists.php.net,Sara Golemon <pollita@php.net>
Message-ID: <515C8670.7030400@php.net>
Date: Wed, 03 Apr 2013 20:43:44 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4
MIME-Version: 1.0
References: <CAESVnVr_jLwRbVsOOx3urPv0Qzc9Rc-7y2CNp1T0mH+8ad3cRg@mail.gmail.com> <CAF+90c9N81ZoSpwm2hSh=gCYV2Y3jB1e+N4_N_d_QfHk9JPK5g@mail.gmail.com> <CAESVnVrAV4GwhbtUwxez4sAwPboZ1dh=vB2WcW18sv3zt3+MTQ@mail.gmail.com>
In-Reply-To: <CAESVnVrAV4GwhbtUwxez4sAwPboZ1dh=vB2WcW18sv3zt3+MTQ@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] Simplified Array API for extensions
From: krakjoe@php.net (Joe Watkins)

On 04/03/2013 06:23 PM, Sara Golemon wrote:
>>   1a) The "c" modifier seems like an unnecessary microoptimization. Compilers
>> should be able to optimize strlen() calls on constant strings away and even
>> if they didn't, it wouldn't be much of a big deal. Also using the
>> "c"-variants on a non-literal would not throw an error and just use an
>> invalid length instead.
>>
> Yeah, killing this for sure.
>
>>   1b) Imho the "l_safe" case does not need its own modifier. Typically
>> strings in PHP are always zero-terminated (in debug mode you'll actually get
>> warnings if they are not). The cases where they aren't zero-terminated are
>> rare and in these cases one can just write those two extra lines of code to
>> do the right thing.
>>
> Yeah, the safe version wasn't meant to capture a common case, but it
> is one I've found myself running into more a little commonly.  The
> argument that the rest of the runtime doesn't provide this affordance
> is a good one, but I'm still on the fence for this one.
>
>>    c) I think it would be a lot more intuitive if we used the terminology of
>> the normal array APIs instead of the shorthands:
>>
>>      php_array_fetch => php_array_fetch_assoc
>>      php_array_fetchl => php_array_fetch_assoc_ex
>>      php_array_fetchn => php_array_fetch_index
>>      php_array_fetchz => php_array_fetch_zval
>>
> +1 for consistency, combined with separating out the type conversion
> bits the verbosity (which I was trying to avoid) is also not so bad.
>
>> 2. The php_array_fetch*_* APIs currently combined two things: a) Fetching
>> from the array and b) Casting it to some type. I think both should be
>> separate. Not only to avoid the combinatorial explosion of different
>> modifier+type combinations, but also because those casting methods are also
>> applicable to other contexts, not just arrays casts. I asked some time ago
>> to add functions that can directly get a long/double from a zval (though
>> didn't pursue this further). Your APIs add something like that, but tightly
>> coupled to array fetches. There would be more use for it if it were separate
>> :)
>>
> It's outside the scope of what I was originally trying to accomplish,
> but it's got utility in its own right.  So I'll rewrite this entire
> proposal as two separate tasks:
>
> #1 - Array access APIs for fetching zval*
>
>    zend_array_(fetch|exists|unset)_(assoc|assocl|index|zval)()
>
> #1a - (Optional) Array add APIs renamed for consistency and namespacing:
>    I'm not a huge fan of this as we'd need to keep
> add_(assoc|(next_)?index)_{$type}(), but it's the sort of moment to
> embrace consistency in the hopes of deprecating the old calls when we
> hit 6.0
>
> #1b - (Optional) Some kind of foreach structure which specializes
> zend_hash_apply for the array case:
>    void callback(zval *key, zval *val, void *optionalArg) {
>      /* key as a zval owned by the iterator (is_ref=0,rc=2 - force
> callee to copy if they want to keep it)
>         zend_hash_key might be more "to the point", but zval is a much
> more familiar structure */
>      /* user code here, none of that ZEND_HASH_APPLY_KEEP stuff though */
>    }
>
>    zend_array_apply(arr, callback, optionalArg);
>
> #2 Scalar zval* accessors (with implicit type conversions):
>
>    zend_(bool|long|double|string)_value()
>    zend_is_true() technically covers zend_bool_value() already, but for
> consistency...
>
> And yes, I think we should move em down to Zend...
>
> -Sara
>
Hi Sara,

A logical extension of this idea would be to drop _array_ and cover 
objects too, one uniform everything API is very appealing, and way 
easier to document properly.

Something alone the lines of:

static inline
zend_bool php_exists(zval *pzval, const char *key) {
         switch (Z_TYPE(pzval)) {
             case IS_ARRAY:
                 return zend_symtable_exists(Z_ARRVAL_P(pzval), key, 
strlen(key) + 1);

             default: {
                 if (Z_OBJ_HT_P(pzval)->has_property) {
                     return Z_OBJ_HT_P(pzval)->has_property(
                         pzval, key, strlen(key) + 1, 2 NULL TSRMLS_CC
                     );
                 } else {
                     return zend_symtable_exists(
                         Z_OBJPROP_P(pzval), key, strlen(key) + 1);
                 }
             }
         }
}

Just a thought ..

Joe