Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Bob Weinand]

> Am 02.06.2016 um 14:11 schrieb Dmitry Stogov :
> 
> 
> 
> On 06/02/2016 03:01 PM, Bob Weinand wrote:
>>> Am 01.06.2016 um 12:55 schrieb Dmitry Stogov :
>>> 
>>> hi,
>>> 
>>> 
>>> Please take a look into the proposal.
>>> 
>>> 
>>> https://wiki.php.net/rfc/too_few_args
>>> 
>>> 
>>> The RFC is extremely simple (both proposal and implementation) and almost 
>>> completely described by the email subject.
>>> 
>>> I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm going 
>>> to start the vote on next week.
>>> 
>>> 
>>> Thanks. Dmitry.
>> Very nice!
>> 
>> Just a question: Does the RFC also impact calls to internal functions? (With 
>> internal functions we don't have the problem as they're typically 
>> immediately aborted, but it would be inconsistent with userland functions to 
>> have once a warning, once an exception)
>> 
>> Judging from the patch this isn't the case? (at least I see no related 
>> changes)
>> 
>> Thus I think the scope of the RFC is a bit too small. [and at least it 
>> should be explicitly mentioned in the RFC if you decide against that].
>> 
>> Bob
> The RFC doesn't propose to change behavior of internal functions.
> In case of wrong number of arguments, they are not executed anyway.
> It may make sense to change their behavior as well, but I don't see a big 
> value.
> 
> Thanks. Dmitry.

Right, then the RFC should explicitly mention it. It only talks about "function 
calls" in general right now, but doesn't restrict the scope to userland 
functions only.

The value is basically in having equal and consistent behavior (in failure 
case) for all functions.
I do not see it a must, but it would be a very-nice-to-have in my eyes.

Bob
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Dmitry Stogov]

On 06/02/2016 03:01 PM, Bob Weinand wrote:

Am 01.06.2016 um 12:55 schrieb Dmitry Stogov :

hi,


Please take a look into the proposal.


https://wiki.php.net/rfc/too_few_args


The RFC is extremely simple (both proposal and implementation) and almost 
completely described by the email subject.

I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm going to 
start the vote on next week.


Thanks. Dmitry.

Very nice!

Just a question: Does the RFC also impact calls to internal functions? (With 
internal functions we don't have the problem as they're typically immediately 
aborted, but it would be inconsistent with userland functions to have once a 
warning, once an exception)

Judging from the patch this isn't the case? (at least I see no related changes)

Thus I think the scope of the RFC is a bit too small. [and at least it should 
be explicitly mentioned in the RFC if you decide against that].

Bob

The RFC doesn't propose to change behavior of internal functions.
In case of wrong number of arguments, they are not executed anyway.
It may make sense to change their behavior as well, but I don't see a 
big value.


Thanks. Dmitry.



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Dmitry Stogov]

On 06/02/2016 02:42 PM, Benjamin Eberlei wrote:



On Wed, Jun 1, 2016 at 12:55 PM, Dmitry Stogov > wrote:


hi,


Please take a look into the proposal.


https://wiki.php.net/rfc/too_few_args


The RFC is extremely simple (both proposal and implementation) and
almost completely described by the email subject.

I think, this mini-RFC doesn't need 2-weeks discussion period, so
I'm going to start the vote on next week.


Quick question, I think the error message might be a little confusing 
asking for "exactly $n" parameters, because it could be a variadic 
function using func_get_args(), shouldn't the message be?


throw Error("Too few arguments to function %s(), 0 passed in %s on 
line %d and at least %d expected")
"exactly" is going to be used for functions without variadic arguments 
and arguments with default values.

In that cases PHP will write "at least".
See the patch 
https://github.com/php/php-src/pull/1928/files#diff-3054389ad750ce9a9f5895cd6d27800fR4753


This is similar to other PHP error messages.

zend_internal_type_error(ZEND_ARG_USES_STRICT_TYPES(), "%s%s%s() 
expects %s %d parameter%s, %d given",

class_name, \
class_name[0] ? "::" : "", \
ZSTR_VAL(active_function->common.function_name),
min_num_args == max_num_args ? "exactly" : num_args < 
min_num_args ? "at least" : "at most",

num_args < min_num_args ? min_num_args : max_num_args,
(num_args < min_num_args ? min_num_args : max_num_args) == 1 ? 
"" : "s",

num_args);




This only changes the case where the passed arguments are smaller than 
the required ones or not? This is acceptable BC, but passing more 
arguments should still not be an error.

Of course.

Thanks. Dmitry.




Thanks. Dmitry.

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Bob Weinand]

> Am 01.06.2016 um 12:55 schrieb Dmitry Stogov :
> 
> hi,
> 
> 
> Please take a look into the proposal.
> 
> 
> https://wiki.php.net/rfc/too_few_args
> 
> 
> The RFC is extremely simple (both proposal and implementation) and almost 
> completely described by the email subject.
> 
> I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm going 
> to start the vote on next week.
> 
> 
> Thanks. Dmitry.

Very nice!

Just a question: Does the RFC also impact calls to internal functions? (With 
internal functions we don't have the problem as they're typically immediately 
aborted, but it would be inconsistent with userland functions to have once a 
warning, once an exception)

Judging from the patch this isn't the case? (at least I see no related changes)

Thus I think the scope of the RFC is a bit too small. [and at least it should 
be explicitly mentioned in the RFC if you decide against that].

Bob
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Benjamin Eberlei]

On Wed, Jun 1, 2016 at 12:55 PM, Dmitry Stogov  wrote:

> hi,
>
>
> Please take a look into the proposal.
>
>
> https://wiki.php.net/rfc/too_few_args
>
>
> The RFC is extremely simple (both proposal and implementation) and almost
> completely described by the email subject.
>
> I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm
> going to start the vote on next week.
>

Quick question, I think the error message might be a little confusing
asking for "exactly $n" parameters, because it could be a variadic function
using func_get_args(), shouldn't the message be?

throw Error("Too few arguments to function %s(), 0 passed in %s on line %d
and at least %d expected")

This only changes the case where the passed arguments are smaller than the
required ones or not? This is acceptable BC, but passing more arguments
should still not be an error.

>
>
> Thanks. Dmitry.
>

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Rowan Collins]

On 02/06/2016 00:19, Aaron Piotrowski wrote:



On Jun 1, 2016, at 3:56 PM, Rowan Collins  wrote:

On 01/06/2016 19:36, Aaron Piotrowski wrote:

While this might be considered a BC break, I can't imagine there's an actual 
code out there relying on suppressing the warning just to call a function 
without enough arguments. I see no problem putting this change in 7.1.


I think you're overestimating how much people care about their code running 
without warnings. You don't have to suppress anything, just lazily ignore 
warnings, or log them to a file you never get round to reading. It *might* be 
that most users spot and act on the warnings, but I'm not sure how we could 
know that with any confidence.



Perhaps I'm overestimating how much people care about eliminating warnings, but 
I doubt many functions work as intended without being supplied all required 
arguments. The few situations where code worked as intended when too few 
arguments were provided to a function are fragile and should be fixed. This 
change will better alert users to those problems.


Sure, but "a good thing in the long run" is not the opposite of "BC 
break". If people upgrade to PHP 7.1 and get a load of errors, they will 
just delay adoption of that version. That is not a risk that should be 
dismissed lightly, IMHO.


Regards,
--
Rowan Collins
[IMSoP]


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Stanislav Malyshev]

Hi!

> Please take a look into the proposal.
> 
> 
> https://wiki.php.net/rfc/too_few_args
> 
> 
> The RFC is extremely simple (both proposal and implementation) and almost 
> completely described by the email subject.

Looks fine to me. I don't think having undef function parameters is a
very valuable feature that is widely used.

-- 
Stas Malyshev
smalys...@gmail.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Aaron Piotrowski]

> On Jun 1, 2016, at 3:56 PM, Rowan Collins  wrote:
> 
> On 01/06/2016 19:36, Aaron Piotrowski wrote:
>> While this might be considered a BC break, I can't imagine there's an actual 
>> code out there relying on suppressing the warning just to call a function 
>> without enough arguments. I see no problem putting this change in 7.1.
> 
> I think you're overestimating how much people care about their code running 
> without warnings. You don't have to suppress anything, just lazily ignore 
> warnings, or log them to a file you never get round to reading. It *might* be 
> that most users spot and act on the warnings, but I'm not sure how we could 
> know that with any confidence.
> 

Perhaps I'm overestimating how much people care about eliminating warnings, but 
I doubt many functions work as intended without being supplied all required 
arguments. The few situations where code worked as intended when too few 
arguments were provided to a function are fragile and should be fixed. This 
change will better alert users to those problems.

Aaron Piotrowski


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Stanislav Malyshev]

Hi!

> Note that func_get_args() should die too since we have variadic
> arguments now. 

It should not. There is nothing wrong with this function, the fact that
some of it's functionality is covered by variadic args notwithstanding.
-- 
Stas Malyshev
smalys...@gmail.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Rowan Collins]

On 01/06/2016 19:36, Aaron Piotrowski wrote:

While this might be considered a BC break, I can't imagine there's an actual 
code out there relying on suppressing the warning just to call a function 
without enough arguments. I see no problem putting this change in 7.1.


I think you're overestimating how much people care about their code 
running without warnings. You don't have to suppress anything, just 
lazily ignore warnings, or log them to a file you never get round to 
reading. It *might* be that most users spot and act on the warnings, but 
I'm not sure how we could know that with any confidence.


Regards,

--
Rowan Collins
[IMSoP]


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Nikita Popov]

On Wed, Jun 1, 2016 at 8:24 PM, Dmitry Stogov  wrote:

> In the SSA optimisation framework  (part of opcache), we predict and
> propagate types of variables.
>
> We also may detect situations when some variables may be undefined.
>
> If variable can't be "undefined", we may avoid unnecessary run-time checks
> (e.g .like in ZEND_ADD_LONG handler).
>
> Local variables almost always initialized before usage, but with exiting
> behavior, function arguments always may be uninitialized.
>
To expand on this a bit... if a variable is potentially undefined, we
pretty much have to exclude it from optimization entirely, as nearly any
transformation we can make is prone to impact the undefined variable notice
in some way (remove, duplicate, reorder). The saving grace here is that
(untyped) function parameters don't have any type information, so we
typically can't apply transformations anyway (e.g. type specialization is
obviously not applicable, but many other transformation require guaranteed
warning/notice freedom, which usually requires more specific type
information than "any").

However, there are some optimizations that would be applicable if not for
the fact that the variable is potentially undefined. One such example is
copy propagation. Namely, in

function test($param) {
$var = $param;
// ...
use($var);
}

we wouldn't be able to simply replace all uses of $var with $param and save
that assignment, because this might duplicate or reorder "undefined
variable" notices. Such "useless" variable-to-variable assignments are
typically introduced by inlining. (Note: Both copy propagation and inlining
are not currently in php-src, but may well be in the future.)

Nikita

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Aaron Piotrowski]

> 
> On Jun 1, 2016, at 5:55 AM, Dmitry Stogov  wrote:
> 
> hi,
> 
> 
> Please take a look into the proposal.
> 
> 
> https://wiki.php.net/rfc/too_few_args
> 
> 
> The RFC is extremely simple (both proposal and implementation) and almost 
> completely described by the email subject.
> 
> I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm going 
> to start the vote on next week.
> 
> 
> Thanks. Dmitry.

+1 on this change. IMO, this is one of the few remaining unusual PHP behaviors 
that exists for no obvious reason. A function should not be called if too few 
arguments are provided.

While this might be considered a BC break, I can't imagine there's an actual 
code out there relying on suppressing the warning just to call a function 
without enough arguments. I see no problem putting this change in 7.1.

Aaron Piotrowski
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Dmitry Stogov]

In the SSA optimisation framework  (part of opcache), we predict and propagate 
types of variables.

We also may detect situations when some variables may be undefined.

If variable can't be "undefined", we may avoid unnecessary run-time checks (e.g 
.like in ZEND_ADD_LONG handler).

Local variables almost always initialized before usage, but with exiting 
behavior, function arguments always may be uninitialized.


$ sapi/cli/php -d opcache.opt_debug_level=0x40 2>&1 ../Zend/bench.php | less

...

fibo_r: ; (lines=14, args=1, vars=1, tmps=3, ssa_vars=9, no_loops)
...
#1.CV0($n) [undef, any] = RECV 1
#2.T1 [bool] RANGE[0..1] = IS_SMALLER #1.CV0($n) [undef, any] int(2)
JMPZ #2.T1 [bool] RANGE[0..1] BB2
...


With the patch $n won't include "undef" anymore.


Thanks. Dmitry.


From: Joe Watkins <pthre...@pthreads.org>
Sent: Wednesday, June 1, 2016 8:42:49 PM
To: Dmitry Stogov
Cc: PHP internals; Nikita Popov
Subject: Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few 
arguments" exception

Evening,

Could you expand on the "obvious optimization opportunities" in the 
document ?

They are not obvious to me, and even if they were, this document is part of 
the history of this change, so a little more detail would be nice.

Cheers
Joe

On Wed, Jun 1, 2016 at 11:55 AM, Dmitry Stogov 
<dmi...@zend.com<mailto:dmi...@zend.com>> wrote:
hi,


Please take a look into the proposal.


https://wiki.php.net/rfc/too_few_args


The RFC is extremely simple (both proposal and implementation) and almost 
completely described by the email subject.

I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm going to 
start the vote on next week.


Thanks. Dmitry.

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Joe Watkins]

Evening,

Could you expand on the "obvious optimization opportunities" in the
document ?

They are not obvious to me, and even if they were, this document is
part of the history of this change, so a little more detail would be nice.

Cheers
Joe

On Wed, Jun 1, 2016 at 11:55 AM, Dmitry Stogov  wrote:

> hi,
>
>
> Please take a look into the proposal.
>
>
> https://wiki.php.net/rfc/too_few_args
>
>
> The RFC is extremely simple (both proposal and implementation) and almost
> completely described by the email subject.
>
> I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm
> going to start the vote on next week.
>
>
> Thanks. Dmitry.
>

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Fleshgrinder]

On 6/1/2016 1:33 PM, Rowan Collins wrote:
> I like the concept behind this, but I worry it might have a rather large
> BC impact. Like removing call-time pass-by-reference, there may be a lot
> of long-untouched code that needs fixing to work within this constraint.
> I'm not even sure how easy it would be for static analysis to identify
> these cases. I guess on the flip-side, people will *probably* have
> Warnings displayed or logged already...
> 
> The RFC would also benefit from more discussion of the current behaviour
> in different cases:
> 
> function foo($a) {}
> foo(); // Warning
> 
> function bar(A $a) {}
> bar(); // Error
> 
> function baz(A $a=null) {}
> baz(); // OK
> 
> function quux(?A $a) {}
> quux(); // Not sure. Error?
> 
> function wibble(int $a) {}
> wibble(); // Error (regardless of strict_types, I think)
> 
> function wobble(?int $a) {}
> wobble(); // Error?
> 
> Any other cases?
> 
> Certainly, making these consistent would be nice, but like a lot of
> consistency, does it come at too high a price?
> 

I fully agree with Rowan here. First we need a proper plan for all these
cases and a consistent behavior. Next we need to discuss if something
like this can be introduced in a feature release or requires a major
release.

Note that func_get_args() should die too since we have variadic
arguments now. However, this can only be introduced in a major version.
(Correct me if I am wrong and didn't thought of a very common use case
that requires func_get_args(). I for one never need it.)

Whether references need to die completely or not is a more complicated
discussion. There are valid use cases for them. However, they should be
avoided and other solutions should be preferred if possible.

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Rowan Collins]

On 01/06/2016 11:55, Dmitry Stogov wrote:

hi,


Please take a look into the proposal.


https://wiki.php.net/rfc/too_few_args


The RFC is extremely simple (both proposal and implementation) and almost 
completely described by the email subject.

I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm going to 
start the vote on next week.



I like the concept behind this, but I worry it might have a rather large 
BC impact. Like removing call-time pass-by-reference, there may be a lot 
of long-untouched code that needs fixing to work within this constraint. 
I'm not even sure how easy it would be for static analysis to identify 
these cases. I guess on the flip-side, people will *probably* have 
Warnings displayed or logged already...


The RFC would also benefit from more discussion of the current behaviour 
in different cases:


function foo($a) {}
foo(); // Warning

function bar(A $a) {}
bar(); // Error

function baz(A $a=null) {}
baz(); // OK

function quux(?A $a) {}
quux(); // Not sure. Error?

function wibble(int $a) {}
wibble(); // Error (regardless of strict_types, I think)

function wobble(?int $a) {}
wobble(); // Error?

Any other cases?

Certainly, making these consistent would be nice, but like a lot of 
consistency, does it come at too high a price?


Regards,
--
Rowan Collins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Dmitry Stogov]

func_get_args() useful, when we pass more actual parameters than expected.
This case is unaffected.

Thanks. Dmitry.


From: Florian Anderiasch <m...@anderiasch.de>
Sent: Wednesday, June 1, 2016 2:08:09 PM
To: Dmitry Stogov; PHP internals
Cc: Nikita Popov
Subject: Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few 
arguments" exception

On 06/01/2016 12:55 PM, Dmitry Stogov wrote:
> hi,
> Please take a look into the proposal.
>
> https://wiki.php.net/rfc/too_few_args

My first thought went into how people use func_get_args() as this is (in
a sense) the opposite thereof.

That said I couldn't think of a good reason to disagree, but maybe
someone has a valid example where this change would have unforeseen
consequences.

~Florian

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Florian Anderiasch]

On 06/01/2016 12:55 PM, Dmitry Stogov wrote:
> hi,
> Please take a look into the proposal.
> 
> https://wiki.php.net/rfc/too_few_args

My first thought went into how people use func_get_args() as this is (in
a sense) the opposite thereof.

That said I couldn't think of a good reason to disagree, but maybe
someone has a valid example where this change would have unforeseen
consequences.

~Florian

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

[PHP-DEV] [RFC] Replace "Missing argument" warning with "Too few arguments" exception

[Dmitry Stogov]

hi,


Please take a look into the proposal.


https://wiki.php.net/rfc/too_few_args


The RFC is extremely simple (both proposal and implementation) and almost 
completely described by the email subject.

I think, this mini-RFC doesn't need 2-weeks discussion period, so I'm going to 
start the vote on next week.


Thanks. Dmitry.