Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] Excess arguments for internal functions

Posted by Christoph M. Becker 
Christoph M. Becker
[PHP-DEV] Excess arguments for internal functions
July 02, 2018 11:10PM
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Nikita Popov
Re: [PHP-DEV] Excess arguments for internal functions
July 02, 2018 11:20PM
On Mon, Jul 2, 2018 at 11:00 PM, Christoph M. Becker <[email protected]>
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
Christoph M. Becker
Re: [PHP-DEV] Excess arguments for internal functions
July 02, 2018 11:50PM
On 02.07.2018 at 23:16, Nikita Popov wrote:

> On Mon, Jul 2, 2018 at 11:00 PM, Christoph M. Becker <[email protected]>
> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Nikita Popov
Re: [PHP-DEV] Excess arguments for internal functions
July 03, 2018 12:20PM
On Mon, Jul 2, 2018 at 11:45 PM, Christoph M. Becker <[email protected]>
wrote:

> On 02.07.2018 at 23:16, Nikita Popov wrote:
>
> > On Mon, Jul 2, 2018 at 11:00 PM, Christoph M. Becker <[email protected]>
> > 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.
>

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
Sara Golemon
Re: [PHP-DEV] Excess arguments for internal functions
July 03, 2018 04:50PM
On Tue, Jul 3, 2018 at 5:11 AM, Nikita Popov <[email protected]> wrote:
> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Stanislav Malyshev
Re: [PHP-DEV] Excess arguments for internal functions
July 03, 2018 09:20PM
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login