Re: [RFC] skipping optional parameters

On Wed, 18 Apr 2012 23:35:37 +0200, Stas Malyshev <smalyshev@sugarcrm.com> wrote: Before addressing the points individually, I have to reiterate that internal functions have no concept of default values. I think this proposal or a named arguments proposal is *really* ill-advised without introducing proper default values for internal functions. Then the engine could pass those default values on the stack and we would have none of these problems. Instead, we have lots of functions that will not only break (see below for a definition of "break"), but break subtly (e.g. uninitialized values) and only under very particular conditions (using this new feature). Plus, they increase the complexity of doing argument parsing (surely no one will forget checking for NULLs). All the functions that use ZEND_NUM_ARGS() have to be reviewed and the PHP core is just the tip of the iceberg.

the number of arguments actually passed. But even if it returned the number of arguments passed, it would be irrelevant because you would not know *which* arguments were passed.

I would know. Unless you're using "+" in zend_parse_parameters but actually have positional arguments - so far I know no such functions but if I discover any I'll see how to handle it.

I was talking about ZEND_NUM_ARGS(). My point is that ZEND_NUM_ARGS() now gives near-0 useful information. Yes, you can know which arguments were passed through other means, but not via ZEND_NUM_ARGS(), which what many functions are currently using.

Z params won't stop working, why should they? They'd just have nulls there. You'd have to handle these nulls of course.

When I say this "breaks" current functions, I don't mean they will never work again. I mean that, due to different semantics in argument passing, they will have to be changed. I'm assuming even the though cases like PDOStatement::fetchAll() can be fixed.

Passing default to mt_srand makes little sense since mt_srand doesn't have default documented. But we can define it as being 0. Of course, mt_srand(default) will be different from mt_srand() then, but so what? It doesn't break anything.

mt_srand() was a bad example. But let's supposed it didn't initialize seed. It would be perfectly correct under current semantics, but as far as I understand under the patch mt_srand(default) would result in uninitialized data being used.

https://github.com/cataphract/php-src/blob/intl_calendar/ext/intl/calendar/calendar_methods.cpp#L399 -- and guess what -- your feature would break this.

I don't think it will break that. You may have to do a bit more work to do it, but it can be done. Though frankly I have no idea what that code is supposed to do and suspect it is not really a good API if it requires this level of variation in one function. It basically doing all parameters handling by itself instead of using system API and this is usually a bad sign.

All it does is check for (PHP) NULLs before using zpp with "l", because "l" would convert the NULL to 0 and I want to distinguish the two cases, so that I can use NULL to the same effect of not passing the argument. This was actually discussed in a pull request a few weeks ago.

Good or bad, usage of ZEND_NUM_ARGS() is all over the place: glopes@nebm:~/php/php-src$ git grep -n ZEND_NUM_ARGS | grep -v zend_parse | grep -v zend_get_parameters | wc -l 364

Yeah, I've fallen into this trap too initially. Most of those are multiline calls to zend_parse_parameters and checks that are completely harmless.

Quickly skimming the output with shows that in some cases it's multiline calls, but those are in a clear minority (most are in if/switch statements). And remember, this is just PHP core, a small fraction of all extensions. -- Gustavo Lopes