Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93072
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <F4.52.16256.34F1A275@pb1.pair.com>
To: internals@lists.php.net
Date: Wed, 4 May 2016 17:11:43 +0100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:43.0)
 Gecko/20100101 Firefox/43.0 SeaMonkey/2.40
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: The death of `#ifndef FAST_ZPP`?
From: ajf@ajf.me (Andrea Faulds)

Hi everyone,

You may recall that PHP 7.0 introduced a new API for processing 
arguments to internal functions, the "Fast Parameter Parsing API" or 
FAST_ZPP (https://wiki.php.net/rfc/fast_zpp), which is an alternative to 
the existing zend_parse_parameters function family.

Since FAST_ZPP was implemented, many functions in built-in PHP 
extensions look like this:

#ifndef FAST_ZPP
	if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|S", &str, &what) == 
FAILURE) {
		return;
	}
#else
	ZEND_PARSE_PARAMETERS_START(1, 2)
		Z_PARAM_STR(str)
		Z_PARAM_OPTIONAL
		Z_PARAM_STR(what)
	ZEND_PARSE_PARAMETERS_END();
#endif

That is, they have /two/ sets of parameter processing code: one using 
the old API, and one using the new API, with conditional compilation. 
This is necessary because it is still possible to turn off FAST_ZPP by 
changing the value of the macro.

However, should that be the case? The FAST_ZPP RFC was approved and PHP 
7.0 was released containing it enabled by default, so it's no longer 
just an experimental performance enhancement. Furthermore, I have no 
reason to believe that the option to turn off FAST_ZPP is actually used 
in practice, though feel free to prove me wrong!

Requiring all usages of FAST_ZPP to be accompanied by a 
zend_parse_parameters() fallback creates code bloat. It also creates 
hidden, potentially build-breaking bugs, because the code is rarely 
built with FAST_ZPP disabled. It's like the old TSRMLS_CC situation.

Furthermore, for those who would prefer the FAST_ZPP API, it is 
inconvenient that they cannot use it exclusively and must also use 
zend_parse_parameters(), doubling the effort.

So, I would like to ask: would there be any objections to going through 
and removing all unnecessary zend_parse_parameters fallbacks, and making 
it impossible to disable FAST_ZPP? This would make the two APIs equally 
valid.

Thanks!

-- 
Andrea Faulds
https://ajf.me/