> 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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
