Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83606
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain mabe.berlin from 80.237.132.167 cause and error)
Message-ID: <54EB905C.6030706@mabe.berlin>
Date: Mon, 23 Feb 2015 21:41:00 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0
MIME-Version: 1.0
To: marcio3w@gmail.com
CC: PHP internals <internals@lists.php.net>
References: <54EB8043.7050108@mabe.berlin> <CAOsHV+uj4_aTLFeQL188v0VoX7DzKtWfAMYK9DtyM5ft27y8yA@mail.gmail.com>
In-Reply-To: <CAOsHV+uj4_aTLFeQL188v0VoX7DzKtWfAMYK9DtyM5ft27y8yA@mail.gmail.com>
Content-Type: multipart/alternative;
 boundary="------------010106050703040003070005"
Subject: Re: [PHP-DEV] Proposal: deprecate undefined passed arguments
From: dev@mabe.berlin (Marc Bennewitz)

--------------010106050703040003070005
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi Marcio,

Am 23.02.2015 um 21:15 schrieb Marcio Almada:
>
> 2015-02-23 16:32 GMT-03:00 Marc Bennewitz <dev@mabe.berlin 
> <mailto:dev@mabe.berlin>>:
>
>     Hi all,
>
>     Because the feature freeze for PHP 7 is near I like to know your
>     thoughts about one more change in passing arguments. Sure this one
>     would introduce one more deprecated message / BC break but this
>     one s for reducing BC break in the future!
>
>     Currently a caller can pass undefined arguments as much he like.
>     Such passed arguments are only visible by the callee using
>     func_get_args/func_num_args but the need for such function
>     definitions has been gone with argument unpacking feature. The
>     other rare possibility for ignoring passed arguments by callables
>     from functions like array_walk has been gone, too, with the
>     introduction of clusures.
>
>     By the way we already have internal functions triggering warnings
>     on unknown arguments. This is also an unneeded inconsistency and
>     more it's invisible by the user without testing each function for it.
>
>     At least simply ignoring passed arguments is a violation as
>     described in the following examples (http://3v4l.org/U2lnf):
>
>     class Calculator {
>         public function calc($v) {
>             return $v + 1;
>         }
>     }
>
>     class MyCalculator extends Calculator {
>         public function calc($v1, $v2 = 0) {
>             return parent::calc($v1) + $v2;
>         }
>     }
>
>     function calcArray(array $values, Calculator $calculator) {
>         foreach ($values as &$v) {
>             $v = $calculator->calc($v, $v); // Second argument is wrong !!
>         }
>         return $values;
>     }
>
>     $ar = [1,2,3];
>     var_dump(calcArray($ar, new Calculator));   // ignores the second
>     argument
>     var_dump(calcArray($ar, new MyCalculator)); // UNEXPECTED:  the
>     second argument will be used
>
>
>     Both calculators should be 100% compatible but they aren't as the
>     function "calcArray" shows.
>
>
>     So because of the described issues with the existing code base I
>     like to propose the change to no longer ignore undefined arguments
>     and introduce E_DEPRECATED messages if a function get's an
>     undefined argument. So the user get enough time to fix their code
>     before throwing errors in this case in PHP 8.
>
>     As this should be consistent over the engine I would propose this
>     change for user defined functions and for internal functions as well.
>
>     Again, yes this one would introduces one more BC break but first
>     this would be deprecated before disabled and next it's for
>     reducing BC in the future.
>
>     Marc
>
>
>     -- 
>     PHP Internals - PHP Runtime Development Mailing List
>     To unsubscribe, visit: http://www.php.net/unsub.php
>
>
> Have you checked https://wiki.php.net/rfc/strict_argcount? Only 
> difference is that I'm proposing a warning instead of E_DEPRECATE, but 
> it's still a draft and open to discussion. Maybe you would like to 
> contribute to the RFC before it reaches discussion this week?
>
Wow, nice to see this - didn't noted it before.

But I have to questions/notes:
1. Why you throw only a warning and not an error (deprecating this 
feature first). I don't see the benefit.

2. As I understand it correctly passing unknown arguments will be 
allowed if the function uses one of the|func_[get|num]_arg[s]()| functions.

This looks wired to me as the caller should not know the body to know 
how a function can be called
and because of the dynamic nature of PHP this check isn't possible in 
all cases as you already noted.

Additionally such strict check is for detecting wrong calls but this 
detection doesn't work with code like this: 
https://github.com/zendframework/zf2/blob/master/library/Zend/Cache/Storage/Adapter/Memcached.php#L213

In my opinion functions using |func_[get|num]_arg[s]() |to implement 
variadic arguments are totally outdated and should be refactored using 
new semantic. I also think such code isn't used very often and 
deprecating it + removing later on (PHP 8) will give more than enough 
time to update it as it's simple to find and simple to fix.

Also to previous example given for ZF2 shows that the function 
|func_num_args()| have it's rights to exit but i'm unsure about 
|func_get_arg[s]()|.

Marc

--------------010106050703040003070005--