Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:60204
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain sugarcrm.com designates 67.192.241.113 as permitted sender)
Message-ID: <4F8F33A9.30302@sugarcrm.com>
Date: Wed, 18 Apr 2012 14:35:37 -0700
Organization: SugarCRM
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20120327 Thunderbird/11.0.1
MIME-Version: 1.0
To: Gustavo Lopes <glopes@nebm.ist.utl.pt>
CC: PHP Internals <internals@lists.php.net>
References: <4F8DF4B1.2040307@sugarcrm.com> <op.wcy87qrzidpuyk@damnation.mshome.net> <4F8F254F.4080100@sugarcrm.com> <op.wczbl4pnidpuyk@damnation.mshome.net>
In-Reply-To: <op.wczbl4pnidpuyk@damnation.mshome.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] skipping optional parameters
From: smalyshev@sugarcrm.com (Stas Malyshev)

Hi!

> Not sure what you mean here. What is this "varargs return"? As far as I  

zend_parse_parameters has varargs options. See how var_dump is implemented.

> 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.

> addition. There is no reason to think functions shouldn't do manual  
> argument parsing. In fact, they do, and there are even many ways to have  

There is. We have API for argument passing, functions should use it. If
you're not using APIs, you'd have to fix your function when something
changes. This is why we have APIs.

> them do it -- zpp with Z*/z*/Z+/z+, _zend_get_parameters_array,  
> zend_get_parameters_ex and more. These are not one or two and they will  
> stop working,

Z params won't stop working, why should they? They'd just have nulls
there. You'd have to handle these nulls of course.

> But to my argument: many functions use ZEND_NUM_ARGS() to determine if an  
> argument was passed. Sometimes, this is the only way to determine if an  
> argument was passed -- for instance, if we're using zpp with "l", see  
> mt_srand() for an example. For a more drastic example, see the  
> implementation of PDOStatement::fetchAll().

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.
For fetchAll() NULL there might be indeed unexpected - so we may think
about either fixing such cases or letting zend_parse_parameters know if
this parameter can handle default (most can) or it does some fancy stuff
with argument dependencies.

> Now, you may argue that this is a bad practice because you can't pass an  
> argument with a value with same effect of not passing it, and I would  
> agree. In fact, I've just gone through a great deal of trouble to avoid  
> having internal functions with this kind of behavior. See  
> 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. With or without this feature, I would seriously
consider redesigning it and making it simpler. But again, I have no clue
what this function is supposed to do. Which, BTW, is pretty bad since
apparently it is committed to main code base without having any
comments, prototypes, RFCs, documentation or anything - how one would be
supposed to make any use of it?

> 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. There will be some cases where they aren't - like when default
parameters depend on each other like in fetchAll() - but those can be
handled. Functions like yours above are extremely rare and should be.
And if we ever get names params you'd have to rewrite it anyway so I'd
seriously consider starting to think how it would work with it then. I'd
go with "+" or "*" but that's just a guess.
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227