Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:60208
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain nebm.ist.utl.pt from 193.136.128.21 cause and error)
Content-Type: text/plain; charset=utf-8; format=flowed; delsp=yes
Cc: "PHP Internals" <internals@lists.php.net>
In-Reply-To: <4F8F33A9.30302@sugarcrm.com>
Organization: =?utf-8?Q?N=C3=BAcleo_de_Eng=2E_Biom=C3=A9di?=
 =?utf-8?Q?ca_do_I=2ES=2ET=2E?=
References: <4F8DF4B1.2040307@sugarcrm.com>
 <op.wcy87qrzidpuyk@damnation.mshome.net> <4F8F254F.4080100@sugarcrm.com>
 <op.wczbl4pnidpuyk@damnation.mshome.net> <4F8F33A9.30302@sugarcrm.com>
To: "Stas Malyshev" <smalyshev@sugarcrm.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Date: Thu, 19 Apr 2012 02:01:13 +0200
Message-ID: <op.wczjobp1idpuyk@damnation.mshome.net>
User-Agent: Opera Mail/11.62 (Win32)
Subject: Re: [PHP-DEV] [RFC] skipping optional parameters
From: glopes@nebm.ist.utl.pt ("Gustavo Lopes")

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