Hi!
Is there any particular reason why internal functions raise a warning/an
error regarding excess arguments, unless they are not supposed to
receive any arguments at all? In other words, why don't we use
ZEND_PARSE_PARAMETERS_NONE() (except for net_get_interfaces())?
--
Christoph M. Becker
On Mon, Jul 2, 2018 at 11:00 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:
Hi!
Is there any particular reason why internal functions raise a warning/an
error regarding excess arguments, unless they are not supposed to
receive any arguments at all? In other words, why don't we use
ZEND_PARSE_PARAMETERS_NONE() (except for net_get_interfaces())?
The more common macro for this is zend_parse_parameters_none(), which a
quick grep shows to be used by 519 functions. If a function accepts no
arguments and does not use zend_parse_parameters_none(), that's generally a
bug.
Nikita
On Mon, Jul 2, 2018 at 11:00 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:Is there any particular reason why internal functions raise a warning/an
error regarding excess arguments, unless they are not supposed to
receive any arguments at all? In other words, why don't we use
ZEND_PARSE_PARAMETERS_NONE() (except for net_get_interfaces())?The more common macro for this is zend_parse_parameters_none(), which a
quick grep shows to be used by 519 functions. If a function accepts no
arguments and does not use zend_parse_parameters_none(), that's generally a
bug.
Ah, thanks! I couldn't find zend_parse_parameters_none() in
README.PARAMETER_PARSING_API[1] – guess it should be added there.
Anyhow, I've noticed this due to bug 33502[2], and indeed for
output_reset_rewrite_vars()[3] there is still no ZPP in place. Then
I've checked func_num_args()[4] (which just came to mind), and found
that it doesn't do any ZPP either, as well as other parameterless
functions defined immediately before it.
Should these (and other occurences, if there are any) be fixed for
PHP-7.1, or for master only? I'm somewhat concerned regarding the
potential BC break, particularly with strict_types.
[1]
https://github.com/php/php-src/blob/a7028d96719fef1939eb3bf14ddb05491ef4e09f/README.PARAMETER_PARSING_API
[2] https://bugs.php.net/bug.php?id=33502
[3]
https://github.com/php/php-src/blob/f2c4f06f84fd3a9fbb241dd84179eaa591f196a4/main/output.c#L1542-L1552
[4]
https://github.com/php/php-src/blob/2543e61aed67add7522e0b4cdf9a13cf3e441f6f/Zend/zend_builtin_functions.c
--
Christoph M. Becker
On Mon, Jul 2, 2018 at 11:45 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:
On Mon, Jul 2, 2018 at 11:00 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:Is there any particular reason why internal functions raise a warning/an
error regarding excess arguments, unless they are not supposed to
receive any arguments at all? In other words, why don't we use
ZEND_PARSE_PARAMETERS_NONE() (except for net_get_interfaces())?The more common macro for this is zend_parse_parameters_none(), which a
quick grep shows to be used by 519 functions. If a function accepts no
arguments and does not use zend_parse_parameters_none(), that's
generally a
bug.Ah, thanks! I couldn't find zend_parse_parameters_none() in
README.PARAMETER_PARSING_API[1] – guess it should be added there.Anyhow, I've noticed this due to bug 33502[2], and indeed for
output_reset_rewrite_vars()[3] there is still no ZPP in place. Then
I've checkedfunc_num_args()[4] (which just came to mind), and found
that it doesn't do any ZPP either, as well as other parameterless
functions defined immediately before it.Should these (and other occurences, if there are any) be fixed for
PHP-7.1, or for master only? I'm somewhat concerned regarding the
potential BC break, particularly with strict_types.
I'd go for master only as it's technically BC breaking.
Some people have plans to get rid of the warning/error on too many
arguments to internal functions to make them consistent with user
functions, so maybe this will become unnecessary in the future. But given
the current state of things, it would certainly be good to add
zend_parse_parameters_none() calls where they are missing.
Nikita
I'd go for master only as it's technically BC breaking.
Ditto this.
Some people have plans to get rid of the warning/error on too many
arguments to internal functions to make them consistent with user
functions, so maybe this will become unnecessary in the future. But given
the current state of things, it would certainly be good to add
zend_parse_parameters_none() calls where they are missing.
IMO, we should go the other way. Deprecate excess args in userspace
functions (making them an error in 8.0). We've had formal variadics
in userspace for a long time (5.5 IIRC), this is a solvable problem.
I also know that's not going to happen (for well defensible reasons)
and I accept that. Felt the need to state my piece though.
-Sara
Hi!
IMO, we should go the other way. Deprecate excess args in userspace
functions (making them an error in 8.0). We've had formal variadics
in userspace for a long time (5.5 IIRC), this is a solvable problem.
I think it's premature. We do have variadics, since 5.6
(https://github.com/php/php-src/blob/PHP-5.6/UPGRADING) but that's not
"a long time" - not for PHP code and for the speeds people upgrade.
There are still major code bases running on 5.x. There's still tons of
code out there which does variadics the old-fashioned way, and I don't
see any reason to break it - we do not gain anything from it. We should
not force people to use variadics just for variadics' sake where their
old code is already working.
My position remains - there should not be BC breaks that don't gain
people some benefit in some way. For example, there should not be BC
breaks just to promote a new feature, even if it's way better than the
old way - as long as the old way still works fine and does not cause
trouble. This seems to be such case - non-variadic calls still work, and
as far as I see, there's no improvement that allowing extra arguments
breaks or makes harder.
I think I'd rather allow extra args everywhere than ban them in user
functions. But just having a warning in zero-arg internals is fine too -
there we can know these weren't intended to be variadics, so we can
know it's probably a mistake, unlike with user functions where in the
generic case we don't know that.
--
Stas Malyshev
smalyshev@gmail.com