[RFC][DISCUSSION] Strict Argument Count

9 years ago by Yasuo Ohgaki — view source

unread

Hi Marcio,

I'm moving the "Strict Argument Count" RFC into discussion phase:

RFC: https://wiki.php.net/rfc/strict_argcount
PR: https://github.com/php/php-src/pull/1108

Many different opinions were collected during research phase and the RFC
was updated with real BC break measurements and other important sections.
So, before discussing:

Even if you already read the RFC in the past, read it again now.

Don't claim possible massive BC breaks before read the
measurements already done. No matter how seasoned you are with PHP, real
numbers matter most than assumptions. Your measurements are welcome too.

Try the patch. Really.

Consider reading the use case present on this post: goo.gl/3ykdIy

Keep discussion on topic and remember we are all trying to improve PHP
in some way :)

I like the idea.

/** fn expects a variable-length argument lists */
function fn($arg) {
$arg = func_get_arg();
$args = func_get_args();
}

fn(1); // Ok
fn(...[1, 2, 3, 4, 5]); // Ok
call_user_func_array("fn", [1, 2, 3, 4, 5, 6, 7]); // Ok

I understand motivation why your patch behave like this. It's for BC, right?
However, isn't it better to declare variable length parameters by function
signature
in the long run?

function fn($arg, ...) {}

Is it possible to have E_DEPRECATED error without "..."? and do not care
about func_get_arg*() existence? Make E_DEPRECATED error E_WARNING
in PHP 7.2 or 7.3.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Marcio Almada — view source

unread

Hi Yasuo,

2015-03-02 1:43 GMT-03:00 Yasuo Ohgaki yohgaki@ohgaki.net:

Hi Marcio,

On Mon, Mar 2, 2015 at 8:02 AM, Marcio Almada marcio.web2@gmail.com
wrote:

I like the idea.

/** fn expects a variable-length argument lists */
function fn($arg) {
$arg = func_get_arg();
$args = func_get_args();
}

fn(1); // Ok
fn(...[1, 2, 3, 4, 5]); // Ok
call_user_func_array("fn", [1, 2, 3, 4, 5, 6, 7]); // Ok

I understand motivation why your patch behave like this. It's for BC,
right?

Yes. If you search github for func_get_arg or func_get_args you wil get
around 2,734,673 results. That's a lot.

However, isn't it better to declare variable length parameters by function
signature
in the long run?

function fn($arg, ...) {}

Is it possible to have E_DEPRECATED error without "..."? and do not care
about func_get_arg*() existence? Make E_DEPRECATED error E_WARNING
in PHP 7.2 or 7.3.

I'm not against doing this in a future when PHP v5.5 starts to fade away.
But, right now, a lot of code still needs to support PHP 5.5+ with no
alternative other than use func_get_args. Remember we only got the
dedicated syntax for variadic functions recently on PHP v5.6.

I'm afraid it's too soon to deprecate func_get_arg*s() or to overlook it.
Maybe in the future somebody will build upon this RFC, specially if it gets
approval, and start the deprecation. But at current pace I don't see this
as an alternative unless we all reach consensus, which is unlikely.

Another point is that internal functions are currently using warnings to
signalize wrong argument counts:

strlen http://www.php.net/strlen("foo", "bar");
// PHP warning: strlen() expects exactly 1 parameter, 2 given on line 1

If the mailing list reach consensus that we should emit deprecation
instead, maybe internal functions will need to be updated too. I'm not
against it also, but I think it would be too soon for a lot of people here.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Thanks,
Márcio

9 years ago by Yasuo Ohgaki — view source

unread

Hi Marcio,

2015-03-02 1:43 GMT-03:00 Yasuo Ohgaki yohgaki@ohgaki.net:

Hi Marcio,

On Mon, Mar 2, 2015 at 8:02 AM, Marcio Almada marcio.web2@gmail.com
wrote:

I like the idea.

/** fn expects a variable-length argument lists */
function fn($arg) {
$arg = func_get_arg();
$args = func_get_args();
}

fn(1); // Ok
fn(...[1, 2, 3, 4, 5]); // Ok
call_user_func_array("fn", [1, 2, 3, 4, 5, 6, 7]); // Ok

I understand motivation why your patch behave like this. It's for BC,
right?

Yes. If you search github for func_get_arg or func_get_args you wil get
around 2,734,673 results. That's a lot.

However, isn't it better to declare variable length parameters by
function signature
in the long run?

function fn($arg, ...) {}

Is it possible to have E_DEPRECATED error without "..."? and do not care
about func_get_arg*() existence? Make E_DEPRECATED error E_WARNING
in PHP 7.2 or 7.3.

I'm not against doing this in a future when PHP v5.5 starts to fade away.
But, right now, a lot of code still needs to support PHP 5.5+ with no
alternative other than use func_get_args. Remember we only got the
dedicated syntax for variadic functions recently on PHP v5.6.

I'm afraid it's too soon to deprecate func_get_arg*s() or to overlook it.
Maybe in the future somebody will build upon this RFC, specially if it gets
approval, and start the deprecation. But at current pace I don't see this
as an alternative unless we all reach consensus, which is unlikely.

Another point is that internal functions are currently using warnings to
signalize wrong argument counts:

strlen http://www.php.net/strlen("foo", "bar");
// PHP warning: strlen() expects exactly 1 parameter, 2 given on line 1

If the mailing list reach consensus that we should emit deprecation
instead, maybe internal functions will need to be updated too. I'm not
against it also, but I think it would be too soon for a lot of people here.

I understand your reasons. Compatibility is important, but detecting
function body contents and
suppressing errors by engine is too hacky. Raising E_DEPRECATE/E_STRICT by
function definition seems
the way to go. IMO.

[yohgaki@dev github-php-src]$ php -r 'function f(...$a) {var_dump($a);};
f(1,2,3);'
array(3) {
[0]=>
int(1)
[1]=>
int(2)
[2]=>
int(3)
}
[yohgaki@dev github-php-src]$ php -r 'function f($a) {var_dump($a);};
f(1,2,3);'
int(1)

is current behavior. The latter would be

[yohgaki@dev github-php-src]$ php -r 'function f($a) {var_dump($a);};
f(1,2,3);'
Deprecated: Excessive argument for f() without "...", called in Command
line code on line 1 and defined in Command line code on line 1
int(1)

User may have a lot of E_DEPRECATED errors anyway if strict scalar type
hint passes.
I like the idea. Please use function definition for errors. (E_DEPRECTED or
E_STRICT whichever
is suitable)

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Marcio Almada — view source

unread

Hi,

2015-03-03 16:48 GMT-03:00 Yasuo Ohgaki yohgaki@ohgaki.net:

Hi Marcio,

I understand your reasons. Compatibility is important, but detecting

function body contents and
suppressing errors by engine is too hacky. Raising E_DEPRECATE/E_STRICT by
function definition seems
the way to go. IMO.

Just one correction, we are not "suppressing" the warning. It just emits
the warning conditionally and that's not hacky at all. We already have
other situations where warnings are emitted just when certain conditions
are met.

[yohgaki@dev github-php-src]$ php -r 'function f(...$a) {var_dump($a);};
f(1,2,3);'
array(3) {
[0]=>
int(1)
[1]=>
int(2)
[2]=>
int(3)
}
[yohgaki@dev github-php-src]$ php -r 'function f($a) {var_dump($a);};
f(1,2,3);'
int(1)

is current behavior. The latter would be

[yohgaki@dev github-php-src]$ php -r 'function f($a) {var_dump($a);};
f(1,2,3);'
Deprecated: Excessive argument for f() without "...", called in Command
line code on line 1 and defined in Command line code on line 1
int(1)

Using only the function signatures, overlooking func_get_args(), would be
in practice an indirect deprecation of* func_get_args()* and that's not
negotiable at all, at least on this RFC.

We simply can't do this now because it would be an enormous BC break and
PHP 5.5, which has no "first class" variadics, is still supported. I
understand the reasons you want to do it, but I think we should wait a bit
more before discussing a possible func_get_args deprecation (if it will
ever be deprecated). That's not the point addresses by this RFC.

User may have a lot of E_DEPRECATED errors anyway if strict scalar type
hint passes.

That's highly speculative. We don't even know if the proposal of scalar
typehints with BC breaks will ever get voted, neither if it will pass. It's
not a good idea to take decisions based upon that. Don't you think so?

I like the idea. Please use function definition for errors. (E_DEPRECTED
or E_STRICT whichever
is suitable)

I'm proposing a warning because currently PHP internal functions already
use a warning to indicate wrong argument count. Using E_STRICT or whichever
would look inconsistent IMMO at a first sight. But I think that E_STRICT ||
E_WARNING || E_DEPRECATED is a very debatable topic.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Márcio.

9 years ago by Yasuo Ohgaki — view source

unread

Hi Marcio,

2015-03-03 16:48 GMT-03:00 Yasuo Ohgaki yohgaki@ohgaki.net:

Hi Marcio,

I understand your reasons. Compatibility is important, but detecting

function body contents and
suppressing errors by engine is too hacky. Raising E_DEPRECATE/E_STRICT
by function definition seems
the way to go. IMO.

Just one correction, we are not "suppressing" the warning. It just emits
the warning conditionally and that's not hacky at all. We already have
other situations where warnings are emitted just when certain conditions
are met.

[yohgaki@dev github-php-src]$ php -r 'function f(...$a) {var_dump($a);};
f(1,2,3);'
array(3) {
[0]=>
int(1)
[1]=>
int(2)
[2]=>
int(3)
}
[yohgaki@dev github-php-src]$ php -r 'function f($a) {var_dump($a);};
f(1,2,3);'
int(1)

is current behavior. The latter would be

[yohgaki@dev github-php-src]$ php -r 'function f($a) {var_dump($a);};
f(1,2,3);'
Deprecated: Excessive argument for f() without "...", called in Command
line code on line 1 and defined in Command line code on line 1
int(1)

Using only the function signatures, overlooking func_get_args(), would
be in practice an indirect deprecation of* func_get_args()* and that's
not negotiable at all, at least on this RFC.

We simply can't do this now because it would be an enormous BC break and
PHP 5.5, which has no "first class" variadics, is still supported. I
understand the reasons you want to do it, but I think we should wait a bit
more before discussing a possible func_get_args deprecation (if it will
ever be deprecated). That's not the point addresses by this RFC.

I don't think we need to deprecate func_get_args(). We may have

<?php
function f($a, ...) {
var_dump(func_get_args());
}

f(1,2,3);
?>

array(3) {
[0]=>
int(1)
[1]=>
int(2)
[2]=>
int(3)
}

"...$a" packs arguments. Using "..." as variable parameter indication may
be allowed.
Strictly speaking, it's not needed, though.

User may have a lot of E_DEPRECATED errors anyway if strict scalar type
hint passes.

That's highly speculative. We don't even know if the proposal of scalar
typehints with BC breaks will ever get voted, neither if it will pass. It's
not a good idea to take decisions based upon that. Don't you think so?

It's related, but it's different issue.

I like the idea. Please use function definition for errors. (E_DEPRECTED
or E_STRICT whichever
is suitable)

I'm proposing a warning because currently PHP internal functions already
use a warning to indicate wrong argument count. Using E_STRICT or whichever
would look inconsistent IMMO at a first sight. But I think that E_STRICT ||
E_WARNING || E_DEPRECATED is a very debatable topic

I don't care much about error types, because I change code so that any
errors are not raised anyway.
E_WARNING in this case makes sense because users made obvious errors.

I understand you would like to address errors in older code with compiler
change. We don't have to
care much for older broken code. IMO. I understand importance of argument
count check. Your
wordpress research clearly shows that we must raise error for wrong number
of arguments.

Looking into function body and checking certain function existence seems
just too much for a
language. However, I don't have too strong opinion for this. I would like
to have comments from engine
developers. Any thoughts?

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Marcio Almada — view source

unread

Hi,

2015-03-04 21:51 GMT-03:00 Yasuo Ohgaki yohgaki@ohgaki.net:

I don't think we need to deprecate func_get_args(). We may have

<?php
function f($a, ...) {
var_dump(func_get_args());
}

f(1,2,3);
?>

"...$a" packs arguments. Using "..." as variable parameter indication may
be allowed.
Strictly speaking, it's not needed, though.

I think you are missing a "detail" here. Even if we add "..." to the
language, people on PHP 5.5 would not be able to have it because this new
syntax would be available only on PHP 7.0+ . We still have to account for
func_get_args otherwise people maintaining packages for both PHP 7 and 5.5
will have no alternative to fix the warning. Do you get me?

I like the idea. Please use function definition for errors. (E_DEPRECTED

or E_STRICT whichever
is suitable)

I'm proposing a warning because currently PHP internal functions already
use a warning to indicate wrong argument count. Using E_STRICT or whichever
would look inconsistent IMMO at a first sight. But I think that E_STRICT ||
E_WARNING || E_DEPRECATED is a very debatable topic

I don't care much about error types, because I change code so that any
errors are not raised anyway.
E_WARNING in this case makes sense because users made obvious errors.

Great, so warning seems ok and it's good to keep consistency with other
already well established language behaviors.

I understand you would like to address errors in older code with compiler
change. We don't have to
care much for older broken code. IMO. I understand importance of argument
count check. Your
wordpress research clearly shows that we must raise error for wrong
number of arguments.

It's not "old broken code", it's PHP 5.5+ code and I already told you that
this would be a bad BC break because:

people still have to support PHP 5.5, emit a warning ignoring
func_get_args will only cause pain and won't these people.
this would impede the RFC to pass because we would now have a major BC
break instead of a minor helpful one, and the RFC was proposed to help
people, you saw the uses cases on the RFC.

Looking into function body and checking certain function existence seems
just too much for a
language. However, I don't have too strong opinion for this. I would like
to have comments from engine
developers. Any thoughts?

It would be nothing new or weird. It's just a simple compile time check. We
already "inspect" code in many other situations while compiling and a lot
of behaviors during execution already depend on compile time checks. I
don't think this is "too much". It might sound unusual, at first, but there
is nothing wrong with this.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Yasuo Ohgaki — view source

unread

Hi Marcio,

It would be nothing new or weird. It's just a simple compile time check.
We already "inspect" code in many other situations while compiling and a
lot of behaviors during execution already depend on compile time checks. I
don't think this is "too much". It might sound unusual, at first, but there
is nothing wrong with this.

I really like your idea. You proved usefulness very well by your research.
My only concern is non-language construct detection by compiler. I'll vote
"yes" regardless of my suggestion if it's not an issue.

Zeev suggested better path for migration, not to raise errors
(E_ERROR/E_WARNING/E_NOTICE) instead raise E_DEPRECATED/E_STRICT, for PHP7
in other thread. You may consider the suggestion while I think E_WARNING is
OK.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Marcio Almada — view source

unread

Hi,

2015-03-05 20:08 GMT-03:00 Yasuo Ohgaki yohgaki@ohgaki.net:

Hi Marcio,

On Fri, Mar 6, 2015 at 6:36 AM, Marcio Almada marcio.web2@gmail.com
wrote:

It would be nothing new or weird. It's just a simple compile time check.
We already "inspect" code in many other situations while compiling and a
lot of behaviors during execution already depend on compile time checks. I
don't think this is "too much". It might sound unusual, at first, but there
is nothing wrong with this.

I really like your idea. You proved usefulness very well by your research.
My only concern is non-language construct detection by compiler. I'll vote
"yes" regardless of my suggestion if it's not an issue.

Great, thanks.

Zeev suggested better path for migration, not to raise errors
(E_ERROR/E_WARNING/E_NOTICE) instead raise E_DEPRECATED/E_STRICT, for PHP7
in other thread. You may consider the suggestion while I think E_WARNING is
OK.

You are right about this. I'll setup a yes/no vote + a vote to decide
between E_WARNING (for consistency), E_DEPRECATED or E_STRICT. For me this
is just a detail but maybe it's very important to others, so better to let
each voter decide upon it.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Regards,
Márcio

9 years ago by Patrick ALLAERT — view source

unread

Hello,

Le ven. 6 mars 2015 à 00:44, Marcio Almada marcio.web2@gmail.com a écrit :

You are right about this. I'll setup a yes/no vote + a vote to decide
between E_WARNING (for consistency), E_DEPRECATED or E_STRICT. For me this
is just a detail but maybe it's very important to others, so better to let
each voter decide upon it.

In case of language changes, shouldn't the 2/3 of majority be required at
any levels?

In situations like:

Main feature: No/Yes
Option: A, B or C

My gut feeling is that it would be better to rally a 2/3 majority of people
behind one of:
No / Yes (A) / Yes (B) / Yes (C)
in order to not dilute the importance of language changes.

It would prevent accepting an important change where a lot of people agrees
on a general idea but have strong opinions/arguments on
implementation/details.

Cheers,
Patrick

9 years ago by Anthony Ferrara — view source

unread

Patrick,

My viewpoint is that options in an RFC are dangerous. I would much
rather have a single RFC, with a single vote (yes/no). I think we
should be discouraging the options as much as possible.

The reason is simple: an RFC should be an encapsulated idea, not a
menu of options. The author should take a stance. If there are details
that the author can't decide on, then either take a straw poll in the
mailing list, or create a separate RFC for that option.

The problem with options is that it makes the vote much more
confusing. With 3 options, you have 3 different proposals. Some recent
votes have had upwards of 12 different proposals built in to a single
RFC (2 options + 3 options + 2 options). It's enough to ask someone to
read and understand one proposal completely without having them have
to comprehend all the possible permutations of voting outcomes.

It also encourages weird voting patterns. Take your example of No/Yes,
A/B/C. In that case, you have 4 permutations as you pointed out. But
what's deeper, is how should someone vote if they are opposed to B? I
mean opposed, not just preferring a different one? The tendency would
likely be to watch the vote and if it looks like B will pass, vote no
on the entire proposal.

Can we please come down to a single RFC, with a single vote yes/no?
It's easier to understand, easier to manage and has less possibility
of gaming.

Anthony

On Tue, Mar 10, 2015 at 10:39 AM, Patrick ALLAERT
patrickallaert@php.net wrote:

Hello,

Le ven. 6 mars 2015 à 00:44, Marcio Almada marcio.web2@gmail.com a écrit :

You are right about this. I'll setup a yes/no vote + a vote to decide
between E_WARNING (for consistency), E_DEPRECATED or E_STRICT. For me this
is just a detail but maybe it's very important to others, so better to let
each voter decide upon it.

In case of language changes, shouldn't the 2/3 of majority be required at
any levels?

In situations like:

Main feature: No/Yes
Option: A, B or C

My gut feeling is that it would be better to rally a 2/3 majority of people
behind one of:
No / Yes (A) / Yes (B) / Yes (C)
in order to not dilute the importance of language changes.

It would prevent accepting an important change where a lot of people agrees
on a general idea but have strong opinions/arguments on
implementation/details.

Cheers,
Patrick

9 years ago by Patrick ALLAERT — view source

unread

2015-03-10 16:02 GMT+01:00 Anthony Ferrara ircmaxell@gmail.com:

Patrick,

My viewpoint is that options in an RFC are dangerous. I would much
rather have a single RFC, with a single vote (yes/no). I think we
should be discouraging the options as much as possible.

The reason is simple: an RFC should be an encapsulated idea, not a
menu of options. The author should take a stance. If there are details
that the author can't decide on, then either take a straw poll in the
mailing list, or create a separate RFC for that option.

The problem with options is that it makes the vote much more
confusing. With 3 options, you have 3 different proposals. Some recent
votes have had upwards of 12 different proposals built in to a single
RFC (2 options + 3 options + 2 options). It's enough to ask someone to
read and understand one proposal completely without having them have
to comprehend all the possible permutations of voting outcomes.

It also encourages weird voting patterns. Take your example of No/Yes,
A/B/C. In that case, you have 4 permutations as you pointed out. But
what's deeper, is how should someone vote if they are opposed to B? I
mean opposed, not just preferring a different one? The tendency would
likely be to watch the vote and if it looks like B will pass, vote no
on the entire proposal.

Can we please come down to a single RFC, with a single vote yes/no?
It's easier to understand, easier to manage and has less possibility
of gaming.

Anthony

That is much more stricter than my thoughts but I can't agree more
with you on all the points you mentioned.
You even presented cases I had in mind, thanks for the verbosity :)

We should probably add this to https://wiki.php.net/rfc/voting which
should probably RFC'ed...

Thanks!

Patrick

9 years ago by Derick Rethans — view source

unread

2015-03-10 16:02 GMT+01:00 Anthony Ferrara ircmaxell@gmail.com:

Can we please come down to a single RFC, with a single vote yes/no?
It's easier to understand, easier to manage and has less possibility
of gaming.

That is much more stricter than my thoughts but I can't agree more
with you on all the points you mentioned.
You even presented cases I had in mind, thanks for the verbosity :)

+1

We should probably add this to https://wiki.php.net/rfc/voting which
should probably RFC'ed...

I think you just volunteerd ;-)

cheers,
Derick

9 years ago by Dan Ackroyd — view source

unread

Can we please come down to a single RFC, with a single vote yes/no?
It's easier to understand, easier to manage and has less possibility
of gaming.

While I generally agree, in the case where there is a small detail
that needs to be addresses by a vote, I think having two votes in one
RFC is better than having two almost identical RFCs.

However the question that is being voted on needs to be setup properly
so that it does not prevent people from being able to vote on both
issues.

For example the group use RFC
(https://wiki.php.net/rfc/group_use_declarations) has a small detail
of whether there should be a trailing slash in the syntax, which did
not deserve a separate RFC imo.

Unfortunately, the vote options were:

Yes - with a trailing ""
Yes - without a trailing ""
No

This meant it was impossible for people who wanted to vote no to the
general idea, to say what was their preferred choice of syntax. The
questions and voting choices should have been:

"Should Grouped Use Declarations be added to PHP 7"

Yes
No

"If added, should the syntax be with trailing "" or without."

With a trailing ""
Without a trailing ""

This would have allowed all voters to express their intent for both
parts of the question, without being forced to vote 'yes' if they want
a say in the exact syntax used.

cheers
Dan
Ack

9 years ago by Anthony Ferrara — view source

unread

Dan,

Can we please come down to a single RFC, with a single vote yes/no?
It's easier to understand, easier to manage and has less possibility
of gaming.

While I generally agree, in the case where there is a small detail
that needs to be addresses by a vote, I think having two votes in one
RFC is better than having two almost identical RFCs.

However the question that is being voted on needs to be setup properly
so that it does not prevent people from being able to vote on both
issues.

For example the group use RFC
(https://wiki.php.net/rfc/group_use_declarations) has a small detail
of whether there should be a trailing slash in the syntax, which did
not deserve a separate RFC imo.

Unfortunately, the vote options were:

Yes - with a trailing ""

Yes - without a trailing ""

No

In this case, a straw-poll ahead of time for "with or without" could
have solved that. Or just choosing one.

But in more complex situations it doesn't need to be competing RFCs,
but a RFC for the main thing, and a RFC to choose which option. This
case (with/without "") isn't what I was referring to. I was talking
more about situations like:

https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference#vote

Specifically where the options have pretty significant difference in
potential functionality.

https://wiki.php.net/rfc/pecl_http#vote

Here, enabled/disabled by default, and the namespace?

The namespace is a pretty significant concern. I believe that the RFC
should have taken a stance on it. But if it didn't want to, it could
split it off into its own proposal. So you'd have RFC#1: add pecl_http
to core, and RFC#2: change pecl_http to use the php\ namespace prefix.

By splitting it apart it's a lot clearer what's going on, and the
impact of the decision can be weighed.

If I was doing the proposal though, I would make a single RFC that
takes a stance (picks one). Then let the discussion guide the change.
If people really feel that another option is better, it will become
clear, so the RFC can be updated. That's the point of discussion, no?

Anthony

9 years ago by Marcio Almada — view source

unread

2015-03-10 13:52 GMT-03:00 Anthony Ferrara ircmaxell@gmail.com:

Dan,

On Tue, Mar 10, 2015 at 11:45 AM, Dan Ackroyd danack@basereality.com
wrote:

Can we please come down to a single RFC, with a single vote yes/no?
It's easier to understand, easier to manage and has less possibility
of gaming.

While I generally agree, in the case where there is a small detail
that needs to be addresses by a vote, I think having two votes in one
RFC is better than having two almost identical RFCs.

However the question that is being voted on needs to be setup properly
so that it does not prevent people from being able to vote on both
issues.

For example the group use RFC
(https://wiki.php.net/rfc/group_use_declarations) has a small detail
of whether there should be a trailing slash in the syntax, which did
not deserve a separate RFC imo.

Unfortunately, the vote options were:

Yes - with a trailing ""

Yes - without a trailing ""

No

In this case, a straw-poll ahead of time for "with or without" could
have solved that. Or just choosing one.

But in more complex situations it doesn't need to be competing RFCs,
but a RFC for the main thing, and a RFC to choose which option. This
case (with/without "") isn't what I was referring to. I was talking
more about situations like:

https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference#vote

Specifically where the options have pretty significant difference in
potential functionality.

https://wiki.php.net/rfc/pecl_http#vote

Here, enabled/disabled by default, and the namespace?

The namespace is a pretty significant concern. I believe that the RFC
should have taken a stance on it. But if it didn't want to, it could
split it off into its own proposal. So you'd have RFC#1: add pecl_http
to core, and RFC#2: change pecl_http to use the php\ namespace prefix.

By splitting it apart it's a lot clearer what's going on, and the
impact of the decision can be weighed.

If I was doing the proposal though, I would make a single RFC that
takes a stance (picks one). Then let the discussion guide the change.
If people really feel that another option is better, it will become
clear, so the RFC can be updated. That's the point of discussion, no?

Yes, that is the point of discussion. But, unfortunately, a lot of RFCs
only start to get discussed when the voting is open. I don't know why this
happens, but it's a pattern I've been observing for some time. In general,
I agree with you, we should make some effort to eliminate voting options
during discussion phase, or at least reduce the options to a minimum amount.

Anthony

9 years ago by Marcio Almada — view source

unread

Hi,

2015-03-10 12:45 GMT-03:00 Dan Ackroyd danack@basereality.com:

Can we please come down to a single RFC, with a single vote yes/no?
It's easier to understand, easier to manage and has less possibility
of gaming.

While I generally agree, in the case where there is a small detail
that needs to be addresses by a vote, I think having two votes in one
RFC is better than having two almost identical RFCs.

However the question that is being voted on needs to be setup properly
so that it does not prevent people from being able to vote on both
issues.

For example the group use RFC
(https://wiki.php.net/rfc/group_use_declarations) has a small detail
of whether there should be a trailing slash in the syntax, which did
not deserve a separate RFC imo.

Unfortunately, the vote options were:

Yes - with a trailing ""

Yes - without a trailing ""

No

This meant it was impossible for people who wanted to vote no to the
general idea, to say what was their preferred choice of syntax. The
questions and voting choices should have been:

"Should Grouped Use Declarations be added to PHP 7"

Yes

No

"If added, should the syntax be with trailing "" or without."

With a trailing ""

Without a trailing ""

This would have allowed all voters to express their intent for both
parts of the question, without being forced to vote 'yes' if they want
a say in the exact syntax used.

cheers
Dan
Ack

That's so true. The Group Use... was my first RFC and I have to admit this
voting setup was poor decision taking on my part, sorry.

Later some people even confessed that they didn't vote "yes" because they
haven't noticed it was necessary when in reality they just couldn't realize
they should sum the "yes" votes to know if the RFC was passing or not.

I'll avoid to setup a vote like this again and will always prefer the
multiple questions approach in situations where options are inevitable.

Thanks,
Márcio

9 years ago by Marcio Almada — view source

unread

Hi,

2015-03-10 11:39 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Hello,

Le ven. 6 mars 2015 à 00:44, Marcio Almada marcio.web2@gmail.com a
écrit :

You are right about this. I'll setup a yes/no vote + a vote to decide
between E_WARNING (for consistency), E_DEPRECATED or E_STRICT. For me this
is just a detail but maybe it's very important to others, so better to let
each voter decide upon it.

In case of language changes, shouldn't the 2/3 of majority be required at
any levels?

I don't think it's possible. What would happen if the yes/no vote passes
but the secondary vote doesn't reach 2/3 for some option? This would be a
weird situation.

In situations like:

Main feature: No/Yes
Option: A, B or C

My gut feeling is that it would be better to rally a 2/3 majority of
people behind one of:
No / Yes (A) / Yes (B) / Yes (C)
in order to not dilute the importance of language changes.

It would prevent accepting an important change where a lot of people
agrees on a general idea but have strong opinions/arguments on
implementation/details.

Cheers,
Patrick

I think we should do some effort to discuss and discard as much options as
possible so we can have max 2 options or maybe eliminate the secondary
voting at all (which is the perfect scenario IMMO), but this requires a
good absolute number of opinions.

9 years ago by Larry Garfield — view source

unread

I think we should do some effort to discuss and discard as much options as
possible so we can have max 2 options or maybe eliminate the secondary
voting at all (which is the perfect scenario IMMO), but this requires a
good absolute number of opinions.

If for some reason more than a binary question is necessary, at least go
for instant-runoff style voting as that can represent "if not X, then Y
is at least tolerable" better than any mechanism currently in use in
Internals.

--Larry Garfield

9 years ago by Patrick ALLAERT — view source

unread

Le mar. 10 mars 2015 à 19:29, Marcio Almada marcio.web2@gmail.com a
écrit :

Hi,

2015-03-10 11:39 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Hello,

Le ven. 6 mars 2015 à 00:44, Marcio Almada marcio.web2@gmail.com a
écrit :

You are right about this. I'll setup a yes/no vote + a vote to decide
between E_WARNING (for consistency), E_DEPRECATED or E_STRICT. For me
this
is just a detail but maybe it's very important to others, so better to
let
each voter decide upon it.

In case of language changes, shouldn't the 2/3 of majority be required at
any levels?

I don't think it's possible. What would happen if the yes/no vote passes
but the secondary vote doesn't reach 2/3 for some option? This would be a
weird situation.

Pretty simple actually: it would simply not pass because it wouldn't gather
enough support.

Discuss the options, see what gather the most support and the better
reasonings and then suggest that the RFC "yes" vote means A, B or C while
summarizing the reasons of the choice for it in the RFC itself.

A language change vote requiring 2/3 majority on a Yes/No and a simple
majority in an option basically means not requiring 2/3 at all, but 50%
(with 2 options) at most!

9 years ago by S.A.N — view source

unread

I am satisfied, the possibility of group declarations, but the that lack:

<?php

use App\RestException\ // name "RestException", not imported to
current namespace :(
{
Gone,
NotFound,
BadRequest
};

?>

Unfortunately have to write so:

<?php

use App\RestException;
use App\RestException
{
Gone,
NotFound,
BadRequest
};

?>

It looks ugly and very strange.
My proposition, the imported end name, if end of without slash.

Like this:

<?php

use App\RestException
{
Gone,
NotFound,
BadRequest
};

echo RestException::class; // App\RestException

?>

2015-03-11 11:08 GMT+02:00 Patrick ALLAERT patrickallaert@php.net:

Le mar. 10 mars 2015 à 19:29, Marcio Almada marcio.web2@gmail.com a
écrit :

Hi,

2015-03-10 11:39 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Hello,

Le ven. 6 mars 2015 à 00:44, Marcio Almada marcio.web2@gmail.com a
écrit :

You are right about this. I'll setup a yes/no vote + a vote to decide
between E_WARNING (for consistency), E_DEPRECATED or E_STRICT. For me
this
is just a detail but maybe it's very important to others, so better to
let
each voter decide upon it.

In case of language changes, shouldn't the 2/3 of majority be required at
any levels?

I don't think it's possible. What would happen if the yes/no vote passes
but the secondary vote doesn't reach 2/3 for some option? This would be a
weird situation.

Pretty simple actually: it would simply not pass because it wouldn't gather
enough support.

Discuss the options, see what gather the most support and the better
reasonings and then suggest that the RFC "yes" vote means A, B or C while
summarizing the reasons of the choice for it in the RFC itself.

A language change vote requiring 2/3 majority on a Yes/No and a simple
majority in an option basically means not requiring 2/3 at all, but 50%
(with 2 options) at most!

9 years ago by Marcio Almada — view source

unread

Hi, you replied to the wrong thread ;)

2015-07-22 19:38 GMT-03:00 S.A.N ua.san.alex@gmail.com:

I am satisfied, the possibility of group declarations, but the that lack:

<?php

use App\RestException\ // name "RestException", not imported to
current namespace :(
{
Gone,
NotFound,
BadRequest
};

?>

Unfortunately have to write so:

<?php

use App\RestException;
use App\RestException{Gone, NotFound, BadRequest};

?>

It looks ugly and very strange.

There is nothing strange on it (except, possibly, the trailing \
which was discussed to death and voted).

Even if we had no trailing '' it wouldn't make any sense to import
"App\RestException" when "use App\RestException{Gone, NotFound,
BadRequest};" is used.

My proposition, the imported end name, if end of without slash.

Like this:

<?php

use App\RestException
{
Gone,
NotFound,
BadRequest
};

echo RestException::class; // App\RestException

?>

Importing from within a namespace is not the same thing as importing a
class. I'd be against it.

My 2 cents: why do you have "RestException" with "Exception" suffix
and then don't have the same suffix on the other exception names? This
unpredictable exception hierarchy is the "ugly" part and subtle
alternative syntax won't make it better, IMMO.

Marcio

9 years ago by S.A.N — view source

unread

2015-07-23 18:10 GMT+03:00 Marcio Almada marcio.web2@gmail.com:

Hi, you replied to the wrong thread ;)

2015-07-22 19:38 GMT-03:00 S.A.N ua.san.alex@gmail.com:

I am satisfied, the possibility of group declarations, but the that lack:

<?php

use App\RestException\ // name "RestException", not imported to
current namespace :(
{
Gone,
NotFound,
BadRequest
};

?>

Unfortunately have to write so:

<?php

use App\RestException;
use App\RestException{Gone, NotFound, BadRequest};

?>

It looks ugly and very strange.

There is nothing strange on it (except, possibly, the trailing \
which was discussed to death and voted).

Even if we had no trailing '' it wouldn't make any sense to import
"App\RestException" when "use App\RestException{Gone, NotFound,
BadRequest};" is used.

My proposition, the imported end name, if end of without slash.

Like this:

<?php

use App\RestException
{
Gone,
NotFound,
BadRequest
};

echo RestException::class; // App\RestException

?>

Importing from within a namespace is not the same thing as importing a
class. I'd be against it.

My 2 cents: why do you have "RestException" with "Exception" suffix
and then don't have the same suffix on the other exception names? This
unpredictable exception hierarchy is the "ugly" part and subtle
alternative syntax won't make it better, IMMO.

Marcio

RestException is the base class for classes Gone, NotFound, BadRequest...
He needed to catch all RestExceptions class.

We do not use suffixes for names of exception classes, simple clear
named classes of exceptions, this is our agreement.

9 years ago by David Muir — view source

unread

2015-07-23 18:10 GMT+03:00 Marcio Almada marcio.web2@gmail.com:

Hi, you replied to the wrong thread ;)

2015-07-22 19:38 GMT-03:00 S.A.N ua.san.alex@gmail.com:

I am satisfied, the possibility of group declarations, but the that lack:

<?php

use App\RestException\ // name "RestException", not imported to
current namespace :(
{
Gone,
NotFound,
BadRequest
};

?>

Unfortunately have to write so:

<?php

use App\RestException;
use App\RestException{Gone, NotFound, BadRequest};

?>

It looks ugly and very strange.

There is nothing strange on it (except, possibly, the trailing \
which was discussed to death and voted).

Even if we had no trailing '' it wouldn't make any sense to import
"App\RestException" when "use App\RestException{Gone, NotFound,
BadRequest};" is used.

My proposition, the imported end name, if end of without slash.

Like this:

<?php

use App\RestException
{
Gone,
NotFound,
BadRequest
};

echo RestException::class; // App\RestException

?>

Importing from within a namespace is not the same thing as importing a
class. I'd be against it.

My 2 cents: why do you have "RestException" with "Exception" suffix
and then don't have the same suffix on the other exception names? This
unpredictable exception hierarchy is the "ugly" part and subtle
alternative syntax won't make it better, IMMO.

Marcio

RestException is the base class for classes Gone, NotFound, BadRequest...
He needed to catch all RestExceptions class.

We do not use suffixes for names of exception classes, simple clear
named classes of exceptions, this is our agreement.

--

Then why not put the base class in the namespace of which it is a base class for?

Cheers,
David

9 years ago by S.A.N — view source

unread

Then why not put the base class in the namespace of which it is a base class for?

Locations source PHP file is the same as in namespace.

App/RestException.php
App/RestException/NotFound.php
App/RestException/BadRequest.php

I did refactoring code, without group use:
<?php

use App\RestException;

try {
...
} catch (RestException\BadRequest $e) {
...
} catch (RestException\NotFound $e) {
...
} catch (RestException $e) {
...
}

9 years ago by Marcio Almada — view source

unread

Hi,

This is a remainder that the voting for the "Strict Argument Count" RFC is
scheduled to start on March 14th, so we still have this week for discussion
and it's still a good time to give feedback.

Thanks,
Márcio

2015-03-01 20:02 GMT-03:00 Marcio Almada marcio.web2@gmail.com:

Hi, internals

I'm moving the "Strict Argument Count" RFC into discussion phase:

RFC: https://wiki.php.net/rfc/strict_argcount
PR: https://github.com/php/php-src/pull/1108

Many different opinions were collected during research phase and the RFC
was updated with real BC break measurements and other important sections
[...]

9 years ago by Patrick ALLAERT — view source

unread

Hello,

Le lun. 2 mars 2015 à 00:03, Marcio Almada marcio.web2@gmail.com a écrit :

Hi, internals

I'm moving the "Strict Argument Count" RFC into discussion phase:

RFC: https://wiki.php.net/rfc/strict_argcount
PR: https://github.com/php/php-src/pull/1108

Many different opinions were collected during research phase and the RFC
was updated with real BC break measurements and other important sections.
So, before discussing:

Even if you already read the RFC in the past, read it again now.

Don't claim possible massive BC breaks before read the
measurements already done. No matter how seasoned you are with PHP, real
numbers matter most than assumptions. Your measurements are welcome too.

Try the patch. Really.

Consider reading the use case present on this post: goo.gl/3ykdIy

Keep discussion on topic and remember we are all trying to improve PHP
in some way :)

Thanks,
Márcio

I'm globally +0.5, however I have some concerns:

What about constructors?

Children classes may have a bigger number of arguments for their ctors than
their parents. Even if not very elegant, it is possible some are passing a
fixed number of arguments while constructing an object, whether that object
will take it into account or not.

Something like:

class A {
function __construct($a)
}

class B extends A {
function __construct($a, $b)
}

$kind = $bool ? "A" : "B";

$object = new $kind($foo, $bar);

Why aren't you using E_NOTICE?

[1]: Run-time notices. Indicate that the script encountered something that
could indicate an error, but could also happen in the normal course of
running a script.

E_DEPRECATED:
-1, what is E_DEPRECATED is supposed to be removed in a future version. And
that is a huge BC break if it happens. Btw, you're not mentioning in which
version of PHP the support of extra parameters would be removed.

E_WARNING:
-1, IMHO, calling functions/methods with more arguments generally has less
impact than other aspects that currently generate E_NOTICES (e.g. using
undefined variables, constants,...). Using an error reporting level
stronger than for those cases looks inconsistent.

Cheers and thanks for the impressive work so far!

Patrick

[1] http://php.net/manual/en/errorfunc.constants.php

9 years ago by Marcio Almada — view source

unread

2015-03-10 12:31 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Hello,

Le lun. 2 mars 2015 à 00:03, Marcio Almada marcio.web2@gmail.com a
écrit :

I'm globally +0.5, however I have some concerns:

What about constructors?

Children classes may have a bigger number of arguments for their ctors
than their parents. Even if not very elegant, it is possible some are
passing a fixed number of arguments while constructing an object, whether
that object will take it into account or not.

Something like:

class A {
function __construct($a)
}

class B extends A {
function __construct($a, $b)
}

$kind = $bool ? "A" : "B";

$object = new $kind($foo, $bar);

I think this is somehow covered here
https://wiki.php.net/rfc/strict_argcount#hassle_factor, the example is not
explicit to ctors but the same principles seem to apply. Not sure you have
a deeper point on it though as PHP is a weirdo and allows constructors on
interfaces.

Also, FYI, before we reach discussion phase, there was an idea to ignore
ctors and other magic methods but you are the first person to bring it up
on the ML. I'm not very inclined to ignore any other magic methods other
than __call and __callStatic for now.

Why aren't you using E_NOTICE?

[1]: Run-time notices. Indicate that the script encountered something that
could indicate an error, but could also happen in the normal course of
running a script.

I have nothing against E_NOTICE in this case and indeed E_NOTICE seems like
a good fit. I'll add it as an option.

E_DEPRECATED:
-1, what is E_DEPRECATED is supposed to be removed in a future version.
And that is a huge BC break if it happens. Btw, you're not mentioning in
which version of PHP the support of extra parameters would be removed.

I was sympathetic towards E_DEPRECATED in the early stage of discussion
phase but I'm not anymore. If somebody would like to E_DEPRECATE something
related to argument count in the future, the this person will have to
create another RFC and start the deprecation on PHP 7.1. I'm removing this
option now :)

E_WARNING:
-1, IMHO, calling functions/methods with more arguments generally has less
impact than other aspects that currently generate E_NOTICES (e.g. using
undefined variables, constants,...). Using an error reporting level
stronger than for those cases looks inconsistent.

I disagree with the "looks inconsistent" part. The E_WARNING option would
actually be the most consistent with current PHP behavior. Ex:

function fn($a, $b){}
fn(1);
PHP warning: Missing argument 2 for fn(), called in...

If we choose E_WARNING both minimum and maximum argument count will have
the same error level. BTW, in some cases an exceeding argument can be even
more dangerous than a missing argument.

I have no strong feelings regarding to the error level, the E_WARNING vs
E_NOTICE seems legit so I'm waiting for more opinions.

Cheers and thanks for the impressive work so far!

Patrick

[1] http://php.net/manual/en/errorfunc.constants.php

You're welcome. Thanks for the useful feedback. Hope we can reach a
consensus on the options before the voting starts.

Márcio.

9 years ago by Patrick ALLAERT — view source

unread

Le mar. 10 mars 2015 à 21:04, Marcio Almada marcio.web2@gmail.com a
écrit :

2015-03-10 12:31 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Hello,

Le lun. 2 mars 2015 à 00:03, Marcio Almada marcio.web2@gmail.com a
écrit :

I'm globally +0.5, however I have some concerns:

What about constructors?

Children classes may have a bigger number of arguments for their ctors
than their parents. Even if not very elegant, it is possible some are
passing a fixed number of arguments while constructing an object, whether
that object will take it into account or not.

Something like:

class A {
function __construct($a)
}

class B extends A {
function __construct($a, $b)
}

$kind = $bool ? "A" : "B";

$object = new $kind($foo, $bar);

I think this is somehow covered here
https://wiki.php.net/rfc/strict_argcount#hassle_factor, the example is
not explicit to ctors but the same principles seem to apply. Not sure you
have a deeper point on it though as PHP is a weirdo and allows constructors
on interfaces.

Also, FYI, before we reach discussion phase, there was an idea to ignore
ctors and other magic methods but you are the first person to bring it up
on the ML. I'm not very inclined to ignore any other magic methods other
than __call and __callStatic for now.

No deep point, just wanted to bring your attention on it.

E_WARNING:
-1, IMHO, calling functions/methods with more arguments generally has
less impact than other aspects that currently generate E_NOTICES (e.g.
using undefined variables, constants,...). Using an error reporting level
stronger than for those cases looks inconsistent.

I disagree with the "looks inconsistent" part. The E_WARNING option would
actually be the most consistent with current PHP behavior. Ex:

function fn($a, $b){}
fn(1);
PHP warning: Missing argument 2 for fn(), called in...

If we choose E_WARNING both minimum and maximum argument count will have
the same error level. BTW, in some cases an exceeding argument can be even
more dangerous than a missing argument.

You have a (debatable) point :)

It depends according to what aspect of PHP your are comparing it with.
Greping zend_error(E_NOTICE,.*) in the code I had the feeling that already
many notices where the sign of a bigger problem than when passing extra
parameters, hence why I suggested E_NOTICE.

My consistency argument was therefore "severity based".

I have no strong feelings regarding to the error level, the E_WARNING vs
E_NOTICE seems legit so I'm waiting for more opinions.

I wouldn't -1 for E_WARNING because of your extra arguments (sorry for the
pun ;), someone had to do it!). Still in favor of E_NOTICE though.

Patrick

9 years ago by Marcio Almada — view source

unread

Hi

2015-03-11 6:50 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Le mar. 10 mars 2015 à 21:04, Marcio Almada marcio.web2@gmail.com a
écrit :

2015-03-10 12:31 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Hello,

Le lun. 2 mars 2015 à 00:03, Marcio Almada marcio.web2@gmail.com a
écrit :

I'm globally +0.5, however I have some concerns:

What about constructors? [...]

I think this is somehow covered here

https://wiki.php.net/rfc/strict_argcount#hassle_factor, the example is
not explicit to ctors but the same principles seem to apply. Not sure you
have a deeper point on it though as PHP is a weirdo and allows constructors
on interfaces.

Also, FYI, before we reach discussion phase, there was an idea to ignore
ctors and other magic methods but you are the first person to bring it up
on the ML. I'm not very inclined to ignore any other magic methods other
than __call and __callStatic for now.

No deep point, just wanted to bring your attention on it.

E_WARNING:
-1, IMHO, calling functions/methods with more arguments generally has
less impact than other aspects that currently generate E_NOTICES (e.g.
using undefined variables, constants,...). Using an error reporting level
stronger than for those cases looks inconsistent.

I disagree with the "looks inconsistent" part. The E_WARNING option would
actually be the most consistent with current PHP behavior. Ex:

function fn($a, $b){}
fn(1);
PHP warning: Missing argument 2 for fn(), called in...

If we choose E_WARNING both minimum and maximum argument count will have
the same error level. BTW, in some cases an exceeding argument can be even
more dangerous than a missing argument.

You have a (debatable) point :)

It depends according to what aspect of PHP your are comparing it with.
Greping zend_error(E_NOTICE,.*) in the code I had the feeling that already
many notices where the sign of a bigger problem than when passing extra
parameters, hence why I suggested E_NOTICE.

My consistency argument was therefore "severity based".

Ok, that's a good POV. It just turns out that the severity may vary from
case to case.

I have no strong feelings regarding to the error level, the E_WARNING vs

E_NOTICE seems legit so I'm waiting for more opinions.

I wouldn't -1 for E_WARNING because of your extra arguments (sorry for the
pun ;), someone had to do it!). Still in favor of E_NOTICE though.

That's great to know. Looks like E_WARNING vs E_NOTICE vs E_STRICT seems to
be good setup even though I still don't like to have 3 options though.

Patrick

Thanks,
Márcio

9 years ago by Stanislav Malyshev — view source

unread

Hi!

Even if you already read the RFC in the past, read it again now.

Don't claim possible massive BC breaks before read the
measurements already done. No matter how seasoned you are with PHP, real
numbers matter most than assumptions. Your measurements are welcome too.

I'd say your own data - 4597 warnings on 27 different cases of calls -
is quite fitting the "massive BC break" category. If I had some app and
had to evaluate the upgrade to PHP 7 and would see that it produces
thousands of errors and I have to patch dozens of separate places to fix
it (your data doesn't say if the sources of the calls are the same, so I
take the best interpretation - that both source and target are the same)

I would postpone the upgrade as far as possible, because it looks like
a huge disruption.

Additionally, ZF2 tests couldn't even run - and from this you conclude
that your patch is fine. I conclude that we already have a big problem,
and making it worse is not a good idea. If we do it couple of more
times, we'd probably get people to run PHP 7 somewhere in 2025.

Additionally, I feel bad about magic features that change depending on
which code is inside the function. That doesn't feel like a good design.

Additionally, there is more than variadic functions which may use extra
parameters. Such as dynamic dispatch:

function getVariables($vars, $arg) {
foreach($vars as $var) {
$getter = "get$var";
$result[] = $this->$getter($arg);
}
return $result;
}

grossly simplified of course. Now imagine some getters need the arg and
some don't. I can't write this code anymore without either:

Adding argument to all getters even though they don't need it
(unclean and confusing)
Checking how many arguments each getter needs and modifying the loop
accordingly (even more confusing)

Try the patch. Really.

Consider reading the use case present on this post: goo.gl/3ykdIy

The case you bring there looks artificially constructed to support this
RFC. If your unit tests are so bad as to not actually check that the
functions like createUser() do actually create the user you asked to
create and don't just drop information on the floor, and your code
refactoring practices are so bad as to allow people just drop arguments
from public APIs and replace them with other arguments, and all this
without checks or typing - parameter counts is not what is going to save
you. Your own example would break if both changes are done at the same
time (if you can do either of them, no reason why you can't do both).

--
Stas Malyshev
smalyshev@gmail.com

9 years ago by Marcio Almada — view source

unread

Hi,

2015-03-10 21:31 GMT-03:00 Stanislav Malyshev smalyshev@gmail.com:

Hi!

Even if you already read the RFC in the past, read it again now.

Don't claim possible massive BC breaks before read the
measurements already done. No matter how seasoned you are with PHP,
real
numbers matter most than assumptions. Your measurements are welcome
too.

I'd say your own data - 4597 warnings on 27 different cases of calls -
is quite fitting the "massive BC break" category. If I had some app and
had to evaluate the upgrade to PHP 7 and would see that it produces
thousands of errors and I have to patch dozens of separate places to fix
it (your data doesn't say if the sources of the calls are the same, so I
take the best interpretation - that both source and target are the same)

I would postpone the upgrade as far as possible, because it looks like
a huge disruption.

Have you read the rest of the paragraph you distorted?

Because you just took a slice of the paragraph, presented it out of context
like if everybody will just buy your malformed misinterpretation without go
check the RFC and see:

[...]
Running the test suite resulted in a slay of 4.597 warnings directly
related to the proposed RFC. But after some heuristics it was noticeable
that most warnings had a common cause. I parsed the output file to remove
duplicated warnings and the result was a derisive number of only 27 unique
warnings. You can see the condensed list of unique warnings below:
[...]
After checking case by case among the 27 unique warnings found, the
conclusion is that all of them are a result of human mistake from some
code refactory that left function calls with residual parameters behind.
This could be fixed in less than 1 hour.
[...]

The wordpress test suite is basically a bunch of integration tests. If you
have a single warning
it will repeat 4 thousand times, if you fix this warning it disappears from
all 4 thousand tests.

I even parsed the output file to prove that it's only 27 warnings, not the
apparent 4.597 warnings
you decided to quote out of context. It was clearly proven that the BC
break is very isolated even
in a massive codebase like wordpress.

Fixing 27 function calls won't impede Wordpress or anyone to migrate to
PHP7.
It's an easy fix that could prevent nasty dangerous bugs later.

Additionally, ZF2 tests couldn't even run - and from this you conclude
that your patch is fine. I conclude that we already have a big problem,
and making it worse is not a good idea. If we do it couple of more
times, we'd probably get people to run PHP 7 somewhere in 2025.

The ZF2 test suite couldn't run because of fatal errors caused by PHP7
incompatibilities
or bugs that caused segfaults. Nothing related to the RFC itself.

Hope you don't expect to prove a point just by saying that an isolated BC
break will make
the PHP7 migration "much worse" based solely on your instincts about a test
suite that didn't
even run - for unrelated reasons to the RFC proposal - (look at the other
test suites that ran fine).

I measured the BC breaks responsibly before claim anything. Try to do the
same.
That's not enough effort of your part on a mailing list discussion, sorry.

Additionally, I feel bad about magic features that change depending on
which code is inside the function. That doesn't feel like a good design.

If compilation is "magic"... we are all magicians here. Calling the
implementation "magic"
won't change the fact that it's just a compilation check :D

Additionally, there is more than variadic functions which may use extra
parameters. Such as dynamic dispatch:

function getVariables($vars, $arg) {
foreach($vars as $var) {
$getter = "get$var";
$result[] = $this->$getter($arg);
}
return $result;
}

grossly simplified of course. Now imagine some getters need the arg and
some don't. I can't write this code anymore without either:

Adding argument to all getters even though they don't need it
(unclean and confusing)

Checking how many arguments each getter needs and modifying the loop
accordingly (even more confusing)

If you plan to use methods|functions interchangeably then you should make
them compatible
from now on. That's one of the points of the RFC. It's not "unclean".

BTW, the current PHP silent behavior should be considered even more
confusing otherwise we
wouldn't have these measurements:

https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world

It basically means that even experienced package maintainers are shooting
on their own foot and
they are using code evaluation tools, static analyzers, etc. Now imagine
the common PHP programmer.

Try the patch. Really.

Consider reading the use case present on this post: goo.gl/3ykdIy

The case you bring there looks artificially constructed to support this
RFC. If your unit tests are so bad as to not actually check that the
functions like createUser() do actually create the user you asked to
create and don't just drop information on the floor, and your code
refactoring practices are so bad as to allow people just drop arguments
from public APIs and replace them with other arguments, and all this
without checks or typing - parameter counts is not what is going to save
you. Your own example would break if both changes are done at the same
time (if you can do either of them, no reason why you can't do both).

There is nothing bad about using examples to illustrate something, just
because
the example is "artificial" it doesn't mean it's disconnected from reality.
The example
on goo.gl/3ykdIy is far from being a lie, it's indeed an accurate example.

You can't claim that I artificially built the test suites of a lot of open
source
projects just to "support the RFC". The test suites (and the code being
tested)
grew quite "organically", I just run them to do some measurements:

and there it is: the same patterns from the "artificial" example were
detected
numerous times "on the wild".

--
Stas Malyshev
smalyshev@gmail.com

Márcio

9 years ago by Stanislav Malyshev — view source

unread

Hi!

related to the proposed RFC. But after some heuristics it was
noticeable that most warnings had a common cause. I parsed the output

It doesn't matter if it has common cause or not. If I have a system of
Wordpress-like size, I'm bound to get a lot of failures, that's what it
is telling me. And 27 separate failures are non-negligible number.

some code refactory that left function calls with residual parameters
behind. This could be fixed in less than 1 hour.

I think you are severely underestimating the cost of fixing it. Fixing
bugs is not only replacing the bytes.

The ZF2 test suite couldn't run because of fatal errors caused by PHP7
incompatibilities
or bugs that caused segfaults. Nothing related to the RFC itself.

We do not know if something there is related to RFC or not. That's not
evidence that RFC is OK, that's evidence that a) we could not obtain the
information and b) we already have a BC problem so severe that we can
not run ZF2, and probably not easily fixable (since, I assume, if it
were easily fixable you'd have done so). That's why I am reluctant to
add more BC breaks, especially ones that bring no new capabilities but
just add more error messages. Each new BC break adds migration barriers,
and in my opinion, it's not even linear.

If compilation is "magic"... we are all magicians here. Calling the
implementation "magic"
won't change the fact that it's just a compilation check :D

I didn't say compilation is "magic", you are strawmanning here. What I
said is that having different behavior of the compiler depending on
specific function calls made by the function being compiled is pretty
rare (not only in PHP but in other languages too) and non-obvious, and
thus suspicious.

If you plan to use methods|functions interchangeably then you should
make them compatible
from now on. That's one of the points of the RFC. It's not "unclean".

You basically just choose to ignore my argument here, recognizing that I
correctly identified the case where BC break would happen but saying you
don't care about it. Other people that do use such code may care.
And yes, I consider adding unnecessary parameters to a function just to
satisfy some warning "unclean".

BTW, the current PHP silent behavior should be considered even more
confusing otherwise we
wouldn't have these measurements:

https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world

I'm not sure how these prove current behavior is "confusing". Yes, there
are bugs in the code, and I'm sure this feature will expose some of
those (previously harmless) bugs. At cost of the BC break. And don't
think a handful of libraries represent the vast universe of PHP code,
most of which we can't even see because it's not published anywhere.

It basically means that even experienced package maintainers are
shooting on their own foot and
they are using code evaluation tools, static analyzers, etc. Now imagine
the common PHP programmer.

Yes, experienced programmers make bugs. Inexperienced programmers make
bugs too. Nobody ever claimed otherwise. The point is not about that,
the point is in this case the cure may be worse than the disease.

You can't claim that I artificially built the test suites of a lot of
open source
projects just to "support the RFC". The test suites (and the code being

Coincidentally, I also didn't claim that. Should we go and enumerate
more things that I can't claim and actually didn't, or we are ready to
proceed to discussing things I actually did claim?

and there it is: the same patterns from the "artificial" example were
detected
numerous times "on the wild".

No, your example talks about something completely different - removing a
parameter used by the code and the tests completely missing that even
though the API requires that parameter to be used, and then adding
another parameter in place of that previous parameter and giving it
completely different meaning and the tests missing it again. If some
framework in the wild has such code changes and such tests, they need to
seriously upgrade their testing game.

--
Stas Malyshev
smalyshev@gmail.com

9 years ago by Marcio Almada — view source

unread

Hi

2015-03-11 1:49 GMT-03:00 Stanislav Malyshev smalyshev@gmail.com:

Hi!

related to the proposed RFC. But after some heuristics it was
noticeable that most warnings had a common cause. I parsed the output

It doesn't matter if it has common cause or not. If I have a system of
Wordpress-like size, I'm bound to get a lot of failures, that's what it
is telling me. And 27 separate failures are non-negligible number.

It's just 27 function calls to fix, and you only need to remove a few
arguments that are being ignored
because the functions are not taking them into account... I don't think it
has a big cost to fix.
I fixed them myself in less than one hour!

This won't be an E_INSANELY frequent warning and fix is straightforward, as
the warning
message is usually very explicit about where the call happens and where the
function was declared.

some code refactory that left function calls with residual parameters
behind. This could be fixed in less than 1 hour.

I think you are severely underestimating the cost of fixing it. Fixing
bugs is not only replacing the bytes.

Same as up there.

The ZF2 test suite couldn't run because of fatal errors caused by PHP7
incompatibilities
or bugs that caused segfaults. Nothing related to the RFC itself.

We do not know if something there is related to RFC or not. That's not
evidence that RFC is OK, that's evidence that a) we could not obtain the
information and b) we already have a BC problem so severe that we can
not run ZF2, and probably not easily fixable (since, I assume, if it
were easily fixable you'd have done so).

That's why I am reluctant to

add more BC breaks, especially ones that bring no new capabilities but
just add more error messages. Each new BC break adds migration barriers,
and in my opinion, it's not even linear.

I really get your concerns, but this RFC will not increase the migration
barrier
as much as you are thinking (not even close). Just look at the other
projects
that I tested too ;)

That's why I tried to measure the BC breaks, I was expecting more BC breaks
but it turned out surprisingly sparse breaks and most are potential bugs.

When you say "the RFC brings no new capabilities" I disagree with you. The
RFC
makes PHP safer and easier for people to catch problems, that's a good
capability.

If compilation is "magic"... we are all magicians here. Calling the

implementation "magic"
won't change the fact that it's just a compilation check :D

I didn't say compilation is "magic", you are strawmanning here. What I
said is that having different behavior of the compiler depending on
specific function calls made by the function being compiled is pretty
rare (not only in PHP but in other languages too) and non-obvious, and
thus suspicious.

The bad design is on PHP which unfortunately decided that variadic
functions should
be determined by func_get_args usage. I'm just dealing with that oddity by
doing a
compile time check. It's not "suspicious". It's predictable and reliable.

We check function signatures, we check return by ref and many other things.
If
you think it really straight there is nothing really shocking on the
proposed solution.
You might not like the RFC, but I don't think this is a point for concerns.

If you plan to use methods|functions interchangeably then you should
make them compatible
from now on. That's one of the points of the RFC. It's not "unclean".

You basically just choose to ignore my argument here, recognizing that I
correctly identified the case where BC break would happen but saying you
don't care about it. Other people that do use such code may care.
And yes, I consider adding unnecessary parameters to a function just to
satisfy some warning "unclean".

I'll sincerely try to give some more thought on this subject. I don't think
it's 100% necessary
but I'll try to see if there is a possible alternative for this case.

BTW, the current PHP silent behavior should be considered even more
confusing otherwise we
wouldn't have these measurements:

https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world

I'm not sure how these prove current behavior is "confusing". Yes, there
are bugs in the code, and I'm sure this feature will expose some of
those (previously harmless) bugs.

"Harmless bugs" that one day might get promoted to heartbleed status
on some widely used package or CMS out there if you don't kill them ;)

At cost of the BC break. And don't
think a handful of libraries represent the vast universe of PHP code,
most of which we can't even see because it's not published anywhere.

Unfortunately we can't run each in house code out there to prove something
or not,
but I had to test drive the patch and picked a diverse number of packages
and
FOSS projects so everybody can repeat the tests too.

There is no other way to prove usefulness and measure BCB other than
running the test
suites (it's all green) and running real projects that anyone can have
access to reproduce the tests.
And you also have the patch to test things yourself if you really have the
willpower.

If you think that the current methodology used for measurements is not
valid, I'm afraid there is
nothing else I can do to make it better.

It basically means that even experienced package maintainers are
shooting on their own foot and
they are using code evaluation tools, static analyzers, etc. Now imagine
the common PHP programmer.

Yes, experienced programmers make bugs. Inexperienced programmers make
bugs too. Nobody ever claimed otherwise. The point is not about that,
the point is in this case the cure may be worse than the disease.

You can't claim that I artificially built the test suites of a lot of
open source
projects just to "support the RFC". The test suites (and the code being

Coincidentally, I also didn't claim that. Should we go and enumerate
more things that I can't claim and actually didn't, or we are ready to
proceed to discussing things I actually did claim?

You said that the example given was "artificially built" to prove a point.
I just said
that I couldn't be artificially building the test suites from all the FOSS
projects tested
as sampling. And the example given was based on the experience of testing
these projects
and getting the results (it's far from "artificial").

and there it is: the same patterns from the "artificial" example were
detected
numerous times "on the wild".

No, your example talks about something completely different - removing a
parameter used by the code and the tests completely missing that even
though the API requires that parameter to be used, and then adding
another parameter in place of that previous parameter and giving it
completely different meaning and the tests missing it again. If some
framework in the wild has such code changes and such tests, they need to
seriously upgrade their testing game.

I don't want to sound repetitive, but that's exactly what's happening on
some of the tested code.
Parameters were removed and nothing warns about the wrong method|function
calls because PHP
happily swallow the arguments... until somebody create another argument
that will
suddenly take the dormant values being passed into account - bug.

--
Stas Malyshev
smalyshev@gmail.com

Márcio

9 years ago by wp12173047-156224 wp12173047-156224 — view source

unread

Hi Stas,

Stanislav Malyshev smalyshev@gmail.com hat am 11. März 2015 um 05:49
geschrieben:

Hi!

related to the proposed RFC. But after some heuristics it was
noticeable that most warnings had a common cause. I parsed the output

It doesn't matter if it has common cause or not. If I have a system of
Wordpress-like size, I'm bound to get a lot of failures, that's what it
is telling me. And 27 separate failures are non-negligible number.

This RFC doesn't add failures. It only makes failures visible. The failures are
present without this this patch too but invisible.

some code refactory that left function calls with residual parameters
behind. This could be fixed in less than 1 hour.

I think you are severely underestimating the cost of fixing it. Fixing
bugs is not only replacing the bytes.

Same as above. The Bugs are there with and without this RFC.

The ZF2 test suite couldn't run because of fatal errors caused by PHP7
incompatibilities
or bugs that caused segfaults. Nothing related to the RFC itself.

We do not know if something there is related to RFC or not. That's not
evidence that RFC is OK, that's evidence that a) we could not obtain the
information and b) we already have a BC problem so severe that we can
not run ZF2, and probably not easily fixable (since, I assume, if it
were easily fixable you'd have done so). That's why I am reluctant to
add more BC breaks, especially ones that bring no new capabilities but
just add more error messages. Each new BC break adds migration barriers,
and in my opinion, it's not even linear.

The current fatals are not related to this patch. Sure, because of this it's
unknown if there are other errors related to this patch but it doesn't make this
RFC better or worse.

In my opinion this RFC only introduces a very very small BC break and only in
the case warnings are thrown as exceptions by the user (as in composer). This
RFC only makes currently invisible failures visible.

If compilation is "magic"... we are all magicians here. Calling the
implementation "magic"
won't change the fact that it's just a compilation check :D

I didn't say compilation is "magic", you are strawmanning here. What I
said is that having different behavior of the compiler depending on
specific function calls made by the function being compiled is pretty
rare (not only in PHP but in other languages too) and non-obvious, and
thus suspicious.

First of all please be consistent with your meaning ... on discussion about
"$this from incompatible context" you liked to change a function callable or not
if the function contains $this or not.

In the normal case also say the function body should not define anything about
how a function is callable but in this case the functions func_get_arg[s]
already do this.

BTW, the current PHP silent behavior should be considered even more
confusing otherwise we
wouldn't have these measurements:

https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world

I'm not sure how these prove current behavior is "confusing". Yes, there
are bugs in the code, and I'm sure this feature will expose some of
those (previously harmless) bugs. At cost of the BC break. And don't
think a handful of libraries represent the vast universe of PHP code,
most of which we can't even see because it's not published anywhere.

What do you mean here?

Again, introducing a warning isn't a real BC-break and as you says there are
bugs it helps nobody ignoring them. If you think so you can do that with all of
your own bugs by ignoring warnings/notices.

<snip>

Marc

9 years ago by Lester Caine — view source

unread

BTW, the current PHP silent behavior should be considered even more

confusing otherwise we
wouldn't have these measurements:

https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world

I'm not sure how these prove current behavior is "confusing". Yes, there
are bugs in the code, and I'm sure this feature will expose some of
those (previously harmless) bugs. At cost of the BC break. And don't
think a handful of libraries represent the vast universe of PHP code,
most of which we can't even see because it's not published anywhere.
What do you mean here?

Again, introducing a warning isn't a real BC-break and as you says there are
bugs it helps nobody ignoring them. If you think so you can do that with all of
your own bugs by ignoring warnings/notices.

I must say I was a little confused when I first saw this given the
number of errors I got when addressing the E_STRICT ones on existing
code. One ended up creating multiple functions with different arguments
to get a clean set of code, but I had perhaps not even realised that
adding extra arguments was not flagged. In practice my IDE setup is
flagging warnings where variables are not used, and it seems that
perhaps I'm in a 'safe place' because of the whole package rather than a
particular 'bug' in PHP.

Most of the examples being shown are examples of simple bad programming
practice that needs fixing anyway, and I would expect a proper code
review to have picked them up, so don't see that adding the check in PHP
is essential. It would however be a useful addition but in the E_STRICT
category ... not that I want to maintain that, but being able to ignore
those errors until such time as it is appropriate to fix them. The
poorly checked code is the problem rather than PHP here, and I'm with
Stas that it's probably being a bit heavy handed at this point in the
PHP7 process. Actually I would be rather annoyed if the problems
demonstrated have not already been cleaned up already? A tool to
identify problems and get them cleaned before the main test is switched
on helps everybody?

--
Lester Caine - G8HFL

Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

9 years ago by Marcio Almada — view source

unread

2015-03-11 6:27 GMT-03:00 Lester Caine lester@lsces.co.uk:

BTW, the current PHP silent behavior should be considered even more

confusing otherwise we
wouldn't have these measurements:

https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world

I'm not sure how these prove current behavior is "confusing". Yes,
there
are bugs in the code, and I'm sure this feature will expose some of
those (previously harmless) bugs. At cost of the BC break. And don't
think a handful of libraries represent the vast universe of PHP code,
most of which we can't even see because it's not published anywhere.
What do you mean here?

Again, introducing a warning isn't a real BC-break and as you says there
are
bugs it helps nobody ignoring them. If you think so you can do that with
all of
your own bugs by ignoring warnings/notices.

I must say I was a little confused when I first saw this given the
number of errors I got when addressing the E_STRICT ones on existing
code. One ended up creating multiple functions with different arguments
to get a clean set of code, but I had perhaps not even realised that
adding extra arguments was not flagged.

A lot of people don't know it too, and they are shooting themselves on the
foot without
realizing it because PHP is currently silent about it.

In practice my IDE setup is
flagging warnings where variables are not used, and it seems that
perhaps I'm in a 'safe place' because of the whole package rather than a
particular 'bug' in PHP.

Your IDE is a tool you are using to be safe from the current PHP misleading
behavior.
It's fine to use tooling, but that's a basic feature that shouldn't require
special tooling.

Most of the examples being shown are examples of simple bad programming
practice that needs fixing anyway, and I would expect a proper code
review to have picked them up, so don't see that adding the check in PHP
is essential. It would however be a useful addition but in the E_STRICT
category ... not that I want to maintain that, but being able to ignore
those errors until such time as it is appropriate to fix them.

I think this is a valid argument to keep the E_STRICT error level option
for the secondary voting.
That's a very useful information, thanks :)

The
poorly checked code is the problem rather than PHP here, and I'm with
Stas that it's probably being a bit heavy handed at this point in the
PHP7 process. Actually I would be rather annoyed if the problems
demonstrated have not already been cleaned up already?

The examples are not "poorly checked" code, they were retrieved on widely
used packages
we all rely, directly or indirectly. Both code and tests used in the
measurements are
maintained by experienced people and yet the problems found a way to lurk
(it's not a fault on the maintainers side).

I'd say it's enough information to improve the language, not to require
that people
should manually check their code more or use tool A or B.

A tool to
identify problems and get them cleaned before the main test is switched
on helps everybody?

--

Lester Caine - G8HFL

Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

--

Thanks,
Márcio

9 years ago by Patrick ALLAERT — view source

unread

Le mer. 11 mars 2015 à 22:44, Marcio Almada marcio.web2@gmail.com a
écrit :

2015-03-11 6:27 GMT-03:00 Lester Caine lester@lsces.co.uk:

Most of the examples being shown are examples of simple bad programming
practice that needs fixing anyway, and I would expect a proper code
review to have picked them up, so don't see that adding the check in PHP
is essential. It would however be a useful addition but in the E_STRICT
category ... not that I want to maintain that, but being able to ignore
those errors until such time as it is appropriate to fix them.

I don't really see how this favors E_STRICT over E_NOTICE as any of this
type of errors can be displayed/hidden independently.

I think this is a valid argument to keep the E_STRICT error level option
for the secondary voting.
That's a very useful information, thanks :)

It also depends on your perception of E_STRICT. This level has been
introduced in 5.0 without being part of E_ALL in order to, among other
things, avoid too much pain in the *** while migrating from 4.x to 5.x.
As of 5.4, E_ALL contains E_STRICT and the difference between E_STRICT and
E_NOTICE/E_WARNING is certainly not in terms of severity.
Using an undefined variable or property => notice.
Trying to get property of non-object => notice.
Use of undefined constant => notice

For this reason, I think we should use the standard notice/warning/error
levels as much as possible. You may take a look at Nikita's "Reclassify
E_STRICT RFC" for more info about it.
https://wiki.php.net/rfc/reclassify_e_strict

Cheers,
Patrick

9 years ago by Lester Caine — view source

unread

It also depends on your perception of E_STRICT. This level has been
introduced in 5.0 without being part of E_ALL in order to, among other
things, avoid too much pain in the *** while migrating from 4.x to 5.x.
As of 5.4, E_ALL contains E_STRICT and the difference between E_STRICT
and E_NOTICE/E_WARNING is certainly not in terms of severity.
Using an undefined variable or property => notice.
Trying to get property of non-object => notice.
Use of undefined constant => notice

For this reason, I think we should use the standard notice/warning/error
levels as much as possible. You may take a look at Nikita's "Reclassify
E_STRICT RFC" for more info about it.
https://wiki.php.net/rfc/__reclassify_e_strict
https://wiki.php.net/rfc/reclassify_e_strict

I think the main point here is just like the PHP4->5 conversion path,
SOME areas that need upgrading should be flagged by default while others
should be able to be hidden until they need to be addressed. Perhaps
E_STRICT7 off by default, but if all of the 4->5 conversion stuff is now
reclassified then E_STRICT should be available to serve the same purpose
it did back then?

--
Lester Caine - G8HFL

Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

9 years ago by Patrick ALLAERT — view source

unread

Le ven. 13 mars 2015 à 14:39, Lester Caine lester@lsces.co.uk a écrit :

It also depends on your perception of E_STRICT. This level has been
introduced in 5.0 without being part of E_ALL in order to, among other
things, avoid too much pain in the *** while migrating from 4.x to 5.x.
As of 5.4, E_ALL contains E_STRICT and the difference between E_STRICT
and E_NOTICE/E_WARNING is certainly not in terms of severity.
Using an undefined variable or property => notice.
Trying to get property of non-object => notice.
Use of undefined constant => notice

For this reason, I think we should use the standard notice/warning/error
levels as much as possible. You may take a look at Nikita's "Reclassify
E_STRICT RFC" for more info about it.
https://wiki.php.net/rfc/__reclassify_e_strict
https://wiki.php.net/rfc/reclassify_e_strict

I think the main point here is just like the PHP4->5 conversion path,
SOME areas that need upgrading should be flagged by default while others
should be able to be hidden until they need to be addressed. Perhaps
E_STRICT7 off by default, but if all of the 4->5 conversion stuff is now
reclassified then E_STRICT should be available to serve the same purpose
it did back then?

This is the subject I discussed this morning in the "Reclassify E_STRICT
notices" thread: http://marc.info/?l=php-internals&m=142623927703931

Yet another level would probably be a mistake. So I wish we could try
working on a way to easier the conversion path using a pattern that can be
repeated over the time.

9 years ago by Marcio Almada — view source

unread

Hi

2015-03-13 6:02 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Le mer. 11 mars 2015 à 22:44, Marcio Almada marcio.web2@gmail.com a
écrit :

2015-03-11 6:27 GMT-03:00 Lester Caine lester@lsces.co.uk:

Most of the examples being shown are examples of simple bad programming
practice that needs fixing anyway, and I would expect a proper code
review to have picked them up, so don't see that adding the check in PHP
is essential. It would however be a useful addition but in the E_STRICT
category ... not that I want to maintain that, but being able to ignore
those errors until such time as it is appropriate to fix them.

I don't really see how this favors E_STRICT over E_NOTICE as any of this
type of errors can be displayed/hidden independently.

I think this is a valid argument to keep the E_STRICT error level option
for the secondary voting.
That's a very useful information, thanks :)

It also depends on your perception of E_STRICT. This level has been
introduced in 5.0 without being part of E_ALL in order to, among other
things, avoid too much pain in the *** while migrating from 4.x to 5.x.
As of 5.4, E_ALL contains E_STRICT and the difference between E_STRICT and
E_NOTICE/E_WARNING is certainly not in terms of severity.
Using an undefined variable or property => notice.
Trying to get property of non-object => notice.
Use of undefined constant => notice

For this reason, I think we should use the standard notice/warning/error
levels as much as possible. You may take a look at Nikita's "Reclassify
E_STRICT RFC" for more info about it.
https://wiki.php.net/rfc/reclassify_e_strict

Cheers,
Patrick

I talked to Nikita earlier today, in order to try to align the strict arg
count RFC with https://wiki.php.net/rfc/reclassify_e_strict and the
conclusion was the following:

It's a good thing to reclassify E_STRICT as this simplifies PHP error model
and resolves the currently unclear role of strict standards notices. As a
supporter of Nikita's idea, I'm removing E_STRICT from the voting options.
Consider this my collaboration to help to unifiy the error level model and
go a bit farther from the current "error level buffet" state we got
ourselves historically, on PHP.

This leaves us with E_WARNING vs E_NOTICE and I'm sufficiently comfortable
to allow a secondary voting between these two error levels.

Thanks,
Márcio

9 years ago by Marco Pivetta — view source

unread

Hi Marcio/internals,

I just went through the RFC again (didn't check the discussion since our
last chat about it in Room 11).

The section about '"Flexible" Interface Implementations' mentions the
interface as an 'an acceptable “PHPism”'.

This is not just an "acceptable PHPism", it's a "life saving PHPism".

To give you an idea of the amount of code that was saved from a massive
refactoring when ZF1 => ZF2 code was ported by current ZF2 users, think
about all web apps that have form contextual validation.
Guess what? Almost all web applications have forms, and almost all apps
have something like a "confirm password" form.
Yep, ALL ZF2 apps are affected by this RFC, and it will likely prevent an
upgrade to PHP7 in the near future unless our users upgrade to ZF3, which
will be PHP 5.5 based, and therefore without variadic arguments.

This is not an interface that I'm proud of, but it is an interface that
saved a lot of time/money to a lot of users, and that allows flexible
handling of contextual validation where we didn't foresee one upfront (the
interface is indeed wrong, but we can't fix it in a stable release).

Until all of the framework and all of the users have migrated to variadic
interfaces, this particular RFC is a problem.

Getting this change into PHP7 would mean having a major part of the
framework community having a real hard time in upgrading.

Additionally to all these worries of mine, consider that mistakes like the
one with the ValidatorInterface may happen again in the future, and this
"PHPism" is actually saving our asses out there.

Don't get rid of it for the sake of clean code, because these "dirty hacks"
have already proven themselves REALLY useful.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

Hi

2015-03-13 6:02 GMT-03:00 Patrick ALLAERT patrickallaert@php.net:

Le mer. 11 mars 2015 à 22:44, Marcio Almada marcio.web2@gmail.com a
écrit :

2015-03-11 6:27 GMT-03:00 Lester Caine lester@lsces.co.uk:

Most of the examples being shown are examples of simple bad
programming
practice that needs fixing anyway, and I would expect a proper code
review to have picked them up, so don't see that adding the check in
PHP
is essential. It would however be a useful addition but in the
E_STRICT
category ... not that I want to maintain that, but being able to
ignore
those errors until such time as it is appropriate to fix them.

I don't really see how this favors E_STRICT over E_NOTICE as any of this
type of errors can be displayed/hidden independently.

I think this is a valid argument to keep the E_STRICT error level option
for the secondary voting.
That's a very useful information, thanks :)

It also depends on your perception of E_STRICT. This level has been
introduced in 5.0 without being part of E_ALL in order to, among other
things, avoid too much pain in the *** while migrating from 4.x to 5.x.
As of 5.4, E_ALL contains E_STRICT and the difference between E_STRICT
and
E_NOTICE/E_WARNING is certainly not in terms of severity.
Using an undefined variable or property => notice.
Trying to get property of non-object => notice.
Use of undefined constant => notice

For this reason, I think we should use the standard notice/warning/error
levels as much as possible. You may take a look at Nikita's "Reclassify
E_STRICT RFC" for more info about it.
https://wiki.php.net/rfc/reclassify_e_strict

Cheers,
Patrick

I talked to Nikita earlier today, in order to try to align the strict arg
count RFC with https://wiki.php.net/rfc/reclassify_e_strict and the
conclusion was the following:

It's a good thing to reclassify E_STRICT as this simplifies PHP error model
and resolves the currently unclear role of strict standards notices. As a
supporter of Nikita's idea, I'm removing E_STRICT from the voting options.
Consider this my collaboration to help to unifiy the error level model and
go a bit farther from the current "error level buffet" state we got
ourselves historically, on PHP.

This leaves us with E_WARNING vs E_NOTICE and I'm sufficiently comfortable
to allow a secondary voting between these two error levels.

Thanks,
Márcio

9 years ago by Stanislav Malyshev — view source

unread

Hi!

The section about '"Flexible" Interface Implementations' mentions the
interface as an 'an acceptable “PHPism”'.

This is not just an "acceptable PHPism", it's a "life saving PHPism".
/.../
Getting this change into PHP7 would mean having a major part of the
framework community having a real hard time in upgrading.

I get a certain vibe from this RFC that the author just says something
like "I like this idea so much that damn the BC, those people should
just rewrite their code for latest standards, it probably will just take
2 minutes or so" while ignoring what this means for actual production
applications in PHP (e.g: no upgrade, for years). People still running
5.3, massively, and we're building a huge wall here which would block
anybody from trying out 7 because their production app would just not
work. And the reaction of people whose app does not work is not "we
should spend next quarter rewriting our app", it's "ok, maybe we'll look
at that next year". Because it's probably was initially written
targeting 5.0 if not 4.x. I mean I get that some people want to be not
bound by the past and just do whatever they want because past is not
cool and making drastic changes is cool. But I don't see how it is going
to be a good thing for us.

And the worst of it - it doesn't actually allow to make anything
additional we couldn't do before, just produces more errors.

Stas Malyshev
smalyshev@gmail.com

9 years ago by Yasuo Ohgaki — view source

unread

Hi Stas,

On Mon, Mar 16, 2015 at 7:41 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:

The section about '"Flexible" Interface Implementations' mentions the
interface as an 'an acceptable “PHPism”'.

This is not just an "acceptable PHPism", it's a "life saving PHPism".
/.../
Getting this change into PHP7 would mean having a major part of the
framework community having a real hard time in upgrading.

I get a certain vibe from this RFC that the author just says something
like "I like this idea so much that damn the BC, those people should
just rewrite their code for latest standards, it probably will just take
2 minutes or so" while ignoring what this means for actual production
applications in PHP (e.g: no upgrade, for years). People still running
5.3, massively, and we're building a huge wall here which would block
anybody from trying out 7 because their production app would just not
work. And the reaction of people whose app does not work is not "we
should spend next quarter rewriting our app", it's "ok, maybe we'll look
at that next year". Because it's probably was initially written
targeting 5.0 if not 4.x. I mean I get that some people want to be not
bound by the past and just do whatever they want because past is not
cool and making drastic changes is cool. But I don't see how it is going
to be a good thing for us.

And the worst of it - it doesn't actually allow to make anything
additional we couldn't do before, just produces more errors.

I agree partly. It does not provide any additional feature.
However, it gives users ability to detect bugs. It's important gain for
users also.

Wrong code should be fixed anyway. The RFC could be more old code friendly
if E_DEPECATED is used.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Lester Caine — view source

unread

I agree partly. It does not provide any additional feature.
However, it gives users ability to detect bugs. It's important gain for
users also.

Wrong code should be fixed anyway. The RFC could be more old code friendly
if E_DEPECATED is used.

The problem here is simply that just what error's are enabled and
disabled is getting more difficult to decide? If when moving from a
currently clean environment which has every error displayed and only
shows something when any problem arises then moving to a new major
version do we have to switch everything off again since all types of
errors will now be thrown by the previously clean code.

This is the problem currently in the PHP5.2->5.4 dilema. Yes you can
switch errors off and the code runs, but then how do you address the
problems. Added to which something hidden by E_DEPECATED in 5.3 is now
no longer available in 5.4, but the code is still 5.2.

Adding the errors is not really the problem it's working out the order
they need to be cleared in :(

--
Lester Caine - G8HFL

Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

9 years ago by Marco Pivetta — view source

unread

The problem here is simply that just what error's are enabled and
disabled is getting more difficult to decide? If when moving from a
currently clean environment which has every error displayed and only
shows something when any problem arises then moving to a new major
version do we have to switch everything off again since all types of
errors will now be thrown by the previously clean code.

This is the problem currently in the PHP5.2->5.4 dilema. Yes you can
switch errors off and the code runs, but then how do you address the
problems. Added to which something hidden by E_DEPECATED in 5.3 is now
no longer available in 5.4, but the code is still 5.2.

We are talking about codebases with strict no-notice and no-warning
policies.

Disabling error reporting is not going to cut it here, as we all test our
stuff with E_ALL.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

9 years ago by Lester Caine — view source

unread

The problem here is simply that just what error's are enabled and
disabled is getting more difficult to decide? If when moving from a
currently clean environment which has every error displayed and only
shows something when any problem arises then moving to a new major
version do we have to switch everything off again since all types of
errors will now be thrown by the previously clean code.

This is the problem currently in the PHP5.2->5.4 dilema. Yes you can
switch errors off and the code runs, but then how do you address the
problems. Added to which something hidden by E_DEPECATED in 5.3 is now
no longer available in 5.4, but the code is still 5.2.

We are talking about codebases with strict no-notice and no-warning
policies.

Disabling error reporting is not going to cut it here, as we all test our
stuff with E_ALL.

Exactly my point!

But getting millions of lines of existing code TO that state requires a
little time, especially when the goal posts keep moving. Which is why
I've set the goal posts at 5.4 at the moment. The 5.2/3 stuff is finally
starting to get there, but I can't switch off the 5.2 services yet :(
Then I need the PHP5.4 base to run on the PHP7 without a problem ...
just how much time that will take is unknown as yet, so being able to
switch off errors that are non-urgent to fix is a useful tool, otherwise
why have different types of error at all? But this is why PHP5.7 might
have been a sensible stepping stone in the battle to GET a clean code base.

--
Lester Caine - G8HFL

Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

[RFC][DISCUSSION] Strict Argument Count

-- Lester Caine - G8HFL

Lester Caine - G8HFL

-- Lester Caine - G8HFL

And the worst of it - it doesn't actually allow to make anything additional we couldn't do before, just produces more errors.

-- Lester Caine - G8HFL

-- Lester Caine - G8HFL

--
Lester Caine - G8HFL

--
Lester Caine - G8HFL

And the worst of it - it doesn't actually allow to make anything
additional we couldn't do before, just produces more errors.

--
Lester Caine - G8HFL

--
Lester Caine - G8HFL