[RFC] [VOTE] Replace "Missing argument" warning with "Too few arguments" exception

8 years ago by Dmitry Stogov — view source

unread

Hi,

This mini RFC has been moved to "Voting" state. Voting
began on Jun 6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

I encourage everyone to read the RFC and cast your vote towards whichever
option you feel is the best for the language and the community.

Thanks. Dmitry.

8 years ago by Rowan Collins — view source

unread

Hi,

This mini RFC has been moved to "Voting" state. Voting
began on Jun 6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

I encourage everyone to read the RFC and cast your vote towards whichever
option you feel is the best for the language and the community.

I don't have a vote, but if I did I would vote against making this
change in a minor release.

The release process RFC [1] says that for a release such as 7.1
"Backward compatibility must be kept". This RFC gives no justification
for being an exception to this rule, simply acknowledging that it is a
deliberate break in compatibility.

It's a shame it didn't make it for 7.0, but in my opinion, this should
now wait for 8.0.

[1] https://wiki.php.net/rfc/releaseprocess

Regards,

Rowan Collins
[IMSoP]

8 years ago by Derick Rethans — view source

unread

This mini RFC has been moved to "Voting" state. Voting began on Jun
6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

I encourage everyone to read the RFC and cast your vote towards
whichever option you feel is the best for the language and the
community.

I don't have a vote, but if I did I would vote against making this
change in a minor release.

The release process RFC [1] says that for a release such as 7.1
"Backward compatibility must be kept". This RFC gives no justification
for being an exception to this rule, simply acknowledging that it is a
deliberate break in compatibility.

It's a shame it didn't make it for 7.0, but in my opinion, this should now
wait for 8.0.

I agree with that statement, and hence voted "No".

cheers,
Derick

8 years ago by Sebastian Bergmann — view source

unread

It's a shame it didn't make it for 7.0, but in my opinion, this should now
wait for 8.0.
I agree with that statement, and hence voted "No".

Same reason I voted no.

8 years ago by Christoph Becker — view source

unread

Rowan Collins wrote:

This mini RFC has been moved to "Voting" state. Voting
began on Jun 6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

I encourage everyone to read the RFC and cast your vote towards whichever
option you feel is the best for the language and the community.

I don't have a vote, but if I did I would vote against making this
change in a minor release.

The release process RFC [1] says that for a release such as 7.1
"Backward compatibility must be kept". This RFC gives no justification
for being an exception to this rule, simply acknowledging that it is a
deliberate break in compatibility.

In my opinion it would be good to more explicitly clarify what
constitutes an unacceptable BC break. Bug fixes are allowed, but even
those sometimes cause a BC break (as a programmer may rely on the buggy
behavior). After all, to me it comes down to whether a certain behavior
is documented – if it is not documented a user should not rely on it.

While I have not been able to find the behavior described in the PHP
manual, it is explicitly specified in the PHP language specification[2]:

| Otherwise, if the parameter has no corresponding argument and the
| parameter does not have a default value, the parameter variable is
| non-existent and no corresponding VSlot exists.

It's a shame it didn't make it for 7.0, but in my opinion, this should
now wait for 8.0.

I'm afraid I have to agree.

[1] https://wiki.php.net/rfc/releaseprocess

[2]
https://github.com/php/php-langspec/blob/PHP-7.0/spec/13-functions.md#user-content-function-definitions

--
Christoph M. Becker

8 years ago by Rowan Collins — view source

unread

Apologies if this ends up as a duplicate. The list server seems to be,
or have been, down, so trying to re-send and see what happens.

In my opinion it would be good to more explicitly clarify what
constitutes an unacceptable BC break. Bug fixes are allowed, but even
those sometimes cause a BC break (as a programmer may rely on the buggy
behavior).

Indeed, there is the famous xkcd cartoon highlighting that every change
breaks somebody's expectations, and there is sometimes a grey area
between clear bugs and unintended behaviour that is nonetheless unwise
to change. There are also times when breaking compatibility is justified
because the cost of doing so is low - e.g. the behaviour being changed
is a rare edge case; or because the cost of not changing is high -
e.g. fixing a major security flaw.

I think a case could be made for this change falling into the "cost of
change is low" category, but am disappointed that the RFC itself makes
no such case, and nor did there seem to be much engagement when it was
raised on the pre-vote thread.

I'm also not convinced that such a justification would be in the spirit
of the release process RFC. If the intent of the "no BC breaks in minor
releases" policy is to make upgrades smoother, then producing fatal
errors on code that previously ran without error seems like exactly what
we should be avoiding.

I know that people SHOULD be fixing these incidences where they are
producing Warnings currently, but that doesn't mean we can assume that
they DO. This change has the same kind of implications as the removal of
call-time pass-by-reference in 5.4, and I thought that was exactly what
we didn't want to repeat.

Frankly, if this passes, as it currently looks set to do, I think the BC
policy in the Release Process RFC should be officially abandoned or
rewritten, because we will no longer be able to tell users "if it runs
under 7.0, it will run under 7.1".

Regards,

Rowan Collins
[IMSoP]

8 years ago by Fleshgrinder — view source

unread

I don't have a vote, but if I did I would vote against making this
change in a minor release.

The release process RFC [1] says that for a release such as 7.1
"Backward compatibility must be kept". This RFC gives no justification
for being an exception to this rule, simply acknowledging that it is a
deliberate break in compatibility.

It's a shame it didn't make it for 7.0, but in my opinion, this should
now wait for 8.0.

[1] https://wiki.php.net/rfc/releaseprocess

Regards,

I completely agree. The policies are completely unclear. We have patch
and minor releases with BCs and at the same time clean-up of various
stuff is forbidden because of possible BCs in the next major version.
This makes absolutely no sense!

--
Richard "Fleshgrinder" Fussenegger

8 years ago by Rowan Collins — view source

unread

Hi,

This mini RFC has been moved to "Voting" state. Voting
began on Jun 6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

Hi,

The more I think about this RFC, the less I agree with it being included
in 7.1.

I realise this is now officially late in the approval process to raise
these points, but note that the discussion period was just 5 days, when
a language change would normally require 2 weeks.

On the one hand, it doesn't stop a function's author from having to
check for missing data, as claimed.

As far as the receiving function is concerned, there is no difference
between a missing parameter and an explicit null; both are currently
allowed by default, and both are prohibited if a (non-nullable) type
hint is present. Any unhinted parameter can accept nulls, so a defensive
programmer must always check for them.

Like strict_types mode, the change only actually affects the caller of
the function. We can posit two types of user: those who read and fix all
warnings, and those who ignore them. To a user who reads all warnings,
this change makes no difference: they were already avoiding this
behaviour by fixing the warnings. To a user who doesn't, it is a
breaking change, which may prevent their application from running on PHP
7.1.

On the other hand, there is a real risk of people delaying adoption of
7.1 because of these new errors.

On the face of it, it seems like it would be trivial to fix code not to
trigger this error, but there are more complex cases, such as callbacks.
A simple example:

function foo($a, $b, $mode) { }
$callback = 'foo';
$data = [1,2];
usort($data, $callback);

This code will run under all current versions of PHP, but throw an Error
under 7.1 if this RFC passes. If the definition of foo(), the origin of
$callback, and the usort line, are all in different places, it may not
be trivial to determine the correct fix for this.

There is an assumption from those who have spoken in favour of the RFC
that such warnings will rarely be ignored in real world code, and
therefore nobody will actually be affected by this. Here, we are
pitching anecdote against anecdote; my feeling is that many people
routinely ignore warnings, and will consider their code to be "running
fine" under 7.0, then see it "crash" under 7.1.

I was pleased when the "minor versions will retain BC" policy was
adopted, because I thought it would avoid the problems we had with PHP
5.3 and 5.4, and allow wider adoption of the latest version. Allowing
this change would mean that promise has been broken.

Regards,

Rowan Collins
[IMSoP]

8 years ago by Bishop Bettini — view source

unread

On Mon, Jun 13, 2016 at 9:07 AM, Rowan Collins rowan.collins@gmail.com
wrote:

This mini RFC has been moved to "Voting" state. Voting
began on Jun 6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

The more I think about this RFC, the less I agree with it being included
in 7.1.

I realise this is now officially late in the approval process to raise
these points, but note that the discussion period was just 5 days, when a
language change would normally require 2 weeks.
<snip>
There is an assumption from those who have spoken in favour of the RFC
that such warnings will rarely be ignored in real world code, and therefore
nobody will actually be affected by this. Here, we are pitching anecdote
against anecdote; my feeling is that many people routinely ignore warnings,
and will consider their code to be "running fine" under 7.0, then see it
"crash" under 7.1.

I was pleased when the "minor versions will retain BC" policy was adopted,
because I thought it would avoid the problems we had with PHP 5.3 and 5.4,
and allow wider adoption of the latest version. Allowing this change would
mean that promise has been broken.

Rowan, spot on. We're breaking promises, *when we don't need to: *just
accelerate releases of major versions.

In my mind, a version makes a concise statement about what changes to
expect. It's like a changelog, but with less reading. But that only works
when the major, minor, and patch numbers have stable meanings.

I would rather see us establish and hold firm on the membership rules (what
kind of change can go in what version) and quickly release queued
implementations, than destabilize the meaning of version numbers.

For example, our promise might read like:

"Patches do not change the language. Minors may add non-breaking language
features. Majors may change language features. Releases will be made as
soon as there is a critical mass of queued implementations."

What's in a release, when a release goes out, and when it's no longer
maintained is all arbitrary. Amidst all this arbitrariness, let's at least
give our users version numbers with reliable meaning.

bishop

8 years ago by Patrick ALLAERT — view source

unread

Le lun. 6 juin 2016 à 09:23, Dmitry Stogov dmitry@zend.com a écrit :

Hi,

This mini RFC has been moved to "Voting" state. Voting
began on Jun 6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

I encourage everyone to read the RFC and cast your vote towards whichever
option you feel is the best for the language and the community.

Thanks. Dmitry.

I'm all for the feature, but for the same reason than a few here, I voted
"No".

I guess that a vote on a RFC that violates the Release Process makes it
void, whatever the result.

Is there anything against proposing this for PHP 8.0? I guess we can
perfectly vote now on a 8.0 feature, right?

Kind regards,
Patrick

8 years ago by Pierre Joye — view source

unread

Hi,

Hi,

This mini RFC has been moved to "Voting" state. Voting
began on Jun 6 and will close on June 16.

You can find the full RFC at: https://wiki.php.net/rfc/too_few_args

I encourage everyone to read the RFC and cast your vote towards whichever
option you feel is the best for the language and the community.

I voted no for bc reasons.

I do think it is a relatively small thing but still bc.

The other point that makes me vote no is the short discussion period. The
rule is a minimum discussion time, not a "let choose randomly depending on
what or who". This is worrisome.

My take on it is that the RMs should veto it (it is part of their roles).

Thanks
Pierre

8 years ago by Lester Caine — view source

unread

Do we really trust our future selves so little that we are incapable of
planning more than 6 months ahead?

A practical example of the problems of upgrading is perhaps the time it
takes for public services to get updated. Many of my council customers
were still trying to get through their testing phase to switch to W7
when XP was end of lifed and some systems still run XP simply because
money is not available to replace the perfectly functional hardware. We
need long term stability for big installations.

One can not 'plan' for the replacement of working systems, and as has
been indicated migration to PHP7 is an ongoing process. BC is
essentially a matter of can my current code run without change on the
new server. The easy answer is NO since much of it still can't run on
PHP5.4 so moving to any PHP7 system is academic. It is pointless my
testing every new build of PHP7 as I still have to finish the migration
to a later PHP5.x but once the current migration has been completed then
one would expect that having tested on a current PHP7.0.x one does not
have to worry too much about retesting on every new version? Many of the
BC breaks being discussed do only relate to 'new' developments or edge
cases which will not have the same impact that the PHP5.4 changes did,
but they should all be packaged in the one stage rather than being
spread across several builds?

While PHP7 is still 'unstable', I think I am still sensible working
legacy code up to PHP5.6 and eliminated all previous versions before
starting the migration to PHP7?

--
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] [VOTE] Replace "Missing argument" warning with "Too few arguments" exception

Regards,

Regards,

Regards,

-- Lester Caine - G8HFL

--
Lester Caine - G8HFL