[RFC] Increment/Decrement Fixes

  108799
March 1, 2020 21:23 rowan.collins@gmail.com (Rowan Tommins)
Hi all,

Following my thread a couple of weeks ago, I would like to put forward 
an RFC to fix some inconsistencies with the behaviour of the ++ and -- 
operators:

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

Note that this RFC focusses on three specific cases which I think could 
be considered bugs, but require breaking compatibility:

1) Bring the behaviour of decrementing null in line with subtracting 1 
(and with the equivalent increment operator)
2) Bring the behaviour of incrementing or decrementing true and false in 
line with adding or subtracting 1
3) Throw the same error when incrementing or decrementing an array as 
when adding or subtracting 1

I believe the behaviour of strings, objects, and resources, each raise 
sufficient extra questions that they should be left to future RFCs where 
we can focus on each in more detail.

Please read the RFC for more detail on what is and isn't proposed.

Regards,

-- 
Rowan Tommins (né Collins)
[IMSoP]
  108801
March 2, 2020 09:22 drealecs@gmail.com (=?UTF-8?Q?Alexandru_P=C4=83tr=C4=83nescu?=)
Hi Rowan,

Thank you for the effort, very good unification of behavior across
increment and decrement operations.
I would say that all 3 points are most probably code issues and potential
bugs.
I agree with all of them.

For point 3, the new behavior is an error. The problem will be caught
earlier in any production code.

For point 1 and 2, both the old and new behavior is silent. Can we think of
a safer way to migrate?
I'm thinking about having a special notice that can be used before changing
the behavior and code running an old version could have some handler to
catch it and at least be aware of the change.

Have a mechanism like this ever been proposed on internals list?
The "special notice" I mentioned could be something like an information
debug behavior that can configuration for each debugging type statically in
ini settings, used before parsing and have no impact on production code
when disabled.
New debugging types can be added in patch versions for older PHP version
(now 7.2, 7.3 and 7.4), preparing for minor/major BC changes and removed
after migration (now 8.0).
When upgrading production applications, I always wished to have something
like this available for BC changes. We even consider recompiling PHP with
some changes here and there where it would have been harder to find the
issue statically but we didn't go through with it.
We can even do it back for BC changes 7.2->7.3 and 7.3->7.4.
Let me know if this would be a good idea and I can start a topic on this as
it has nothing to do with this RFC.

Regards,
Alex








On Sun, Mar 1, 2020 at 11:23 PM Rowan Tommins collins@gmail.com>
wrote:

> Hi all, > > Following my thread a couple of weeks ago, I would like to put forward > an RFC to fix some inconsistencies with the behaviour of the ++ and -- > operators: > > https://wiki.php.net/rfc/increment_decrement_fixes > > Note that this RFC focusses on three specific cases which I think could > be considered bugs, but require breaking compatibility: > > 1) Bring the behaviour of decrementing null in line with subtracting 1 > (and with the equivalent increment operator) > 2) Bring the behaviour of incrementing or decrementing true and false in > line with adding or subtracting 1 > 3) Throw the same error when incrementing or decrementing an array as > when adding or subtracting 1 > > I believe the behaviour of strings, objects, and resources, each raise > sufficient extra questions that they should be left to future RFCs where > we can focus on each in more detail. > > Please read the RFC for more detail on what is and isn't proposed. > > Regards, > > -- > Rowan Tommins (né Collins) > [IMSoP] > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
  108803
March 2, 2020 14:08 rowan.collins@gmail.com (Rowan Tommins)
Hi Alexandru,


On 2 March 2020 09:22:57 GMT+00:00, "Alexandru Pătrănescu"
<drealecs@gmail.com> wrote:
>For point 1 and 2, both the old and new behavior is silent. Can we >think of >a safer way to migrate?
That's a very good question, and one I don't have a particularly good answer to. The problem with this kind of behaviour change is that there's no way for PHP to know whether a particular piece of code has been changed to expect the new behaviour or not, and therefore whether to raise a deprecation notice. An opt-in notice on *all* usages could allow the user to build up a list of places in their code to check, but they'd still need to manually track which they'd already looked at, so I'm not sure how useful it would be. What I will look into is how well static analysis tools such as PHPStan and Psalm could build a similar list, as I think that's generally a more useful approach. Regards, -- Rowan Tommins [IMSoP]
  108804
March 2, 2020 14:14 marandall@php.net (Mark Randall)
On 02/03/2020 14:08, Rowan Tommins wrote:
> The problem with this kind of behaviour change is that there's no way > for PHP to know whether a particular piece of code has been changed to > expect the new behaviour or not
Lump it in with editions maybe?
  108802
March 2, 2020 14:00 ocramius@gmail.com (Marco Pivetta)
Hey Rowan,

Overall against the RFC: `++` and `--` (prefix and suffix) are supposed to
be used with numeric values, not with other types, as that makes everything
only more confusing.

Code written (intentionally) to use `++` and `--` against non-numeric
values should **NOT** pass a code review, and I am sorry for those that
have to maintain it if that happens.

Changing the current behavior, regardless in which way, is a BC break:
might as well make the BC break useful:

RFC Proposal: `$a = null; $a--; $a === -1`. Let's make this an explicit
TypeError instead.
RFC Proposal: `$a = null; --$a; $a === -1`. Let's make this an explicit
TypeError instead.

RFC Proposal: `$a = true; $a++; $a === 2`. Let's make this an explicit
TypeError instead.
RFC Proposal: `$a = true; ++$a; $a === 2`. Let's make this an explicit
TypeError instead.
RFC Proposal: `$a = true; $a--; $a === 0`. Let's make this an explicit
TypeError instead.
RFC Proposal: `$a = true; --$a; $a === 0`. Let's make this an explicit
TypeError instead.

RFC Proposal: `$a = false; $a++; $a === 1`. Let's make this an explicit
TypeError instead.
RFC Proposal: `$a = false; ++$a; $a === 1`. Let's make this an explicit
TypeError instead.
RFC Proposal: `$a = false; $a--; $a === -1`. Let's make this an explicit
TypeError instead.
RFC Proposal: `$a = false; --$a; $a === -1`. Let's make this an explicit
TypeError instead.

In **addition** to that, we may propose removal of `++`, `--` and similar
from non-numeric types (will lead to TypeError) like Andrea started in
https://wiki.php.net/rfc/invalid_strings_in_arithmetic

It makes no sense to keep a landmine there: let's get rid of it, instead of
empowering it further (and breaking BC too, while doing so).

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/


On Sun, Mar 1, 2020 at 10:23 PM Rowan Tommins collins@gmail.com>
wrote:

> Hi all, > > Following my thread a couple of weeks ago, I would like to put forward > an RFC to fix some inconsistencies with the behaviour of the ++ and -- > operators: > > https://wiki.php.net/rfc/increment_decrement_fixes > > Note that this RFC focusses on three specific cases which I think could > be considered bugs, but require breaking compatibility: > > 1) Bring the behaviour of decrementing null in line with subtracting 1 > (and with the equivalent increment operator) > 2) Bring the behaviour of incrementing or decrementing true and false in > line with adding or subtracting 1 > 3) Throw the same error when incrementing or decrementing an array as > when adding or subtracting 1 > > I believe the behaviour of strings, objects, and resources, each raise > sufficient extra questions that they should be left to future RFCs where > we can focus on each in more detail. > > Please read the RFC for more detail on what is and isn't proposed. > > Regards, > > -- > Rowan Tommins (né Collins) > [IMSoP] > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
  108806
March 2, 2020 14:39 rowan.collins@gmail.com (Rowan Tommins)
Hi Marco,

On Mon, 2 Mar 2020 at 14:00, Marco Pivetta <ocramius@gmail.com> wrote:

> > Overall against the RFC: `++` and `--` (prefix and suffix) are supposed to > be used with numeric values, not with other types, as that makes everything > only more confusing. >
While I'm generally sympathetic to that principle, it could also be said of the + and - operators, which currently silently handle both nulls and booleans. I would be open to changing *all* arithmetic behaviour, particularly on booleans, but don't think a vague ambition for that justifies inconsistent behaviour between different arithmetic operators.
> Code written (intentionally) to use `++` and `--` against non-numeric > values should **NOT** pass a code review, and I am sorry for those that > have to maintain it if that happens. >
I think you're taking simplified examples too literally here, and assuming that all affected code would be obvious at a glance. Take for instance this innocent-looking patch: function foo($bar) { - $bar -= 1; + $bar--; // more code here } With the current behaviour, any inadvertent calls to the function passing null, true, false, or an array will change behaviour when this patch is merged. In particular, the variable $bar would previously have been guaranteed to be an integer, and will now retain the original type passed in. I completely agree that such code has a bug, but the difference in behaviour is still a massive WTF. The asymmetry of ++ and -- is even more confusing. Consider this patch: function repeatThing($extraTimes) { - for ( $i = 0; $i <= $extraTimes ; $i ++ ) { + for ( $i = $extraTimes ; $i >= 0; $i -- ) { doThing(); } } Again, this looks like a safe change - but if passed a value of NULL, the old code will call doThing() once, and the new code will enter an infinite loop. This asymmetry is the primary problem I want to address with this RFC.
> Changing the current behavior, regardless in which way, is a BC break: > might as well make the BC break useful: > > RFC Proposal: `$a = null; $a--; $a === -1`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = null; --$a; $a === -1`. Let's make this an explicit > TypeError instead. >
And what about `$a = null; $a++;` / `$a = null; ++$a;`, which currently has behaviour consistent with other arithmetic operations (in particular, is identical to $a+=1) and this RFC does not propose to change? If we change the ++ case to throw a TypeError, would we also change null+1 to throw one? And would we then also examine all the other operators and utility functions which silently coerce null to zero or an empty string? I'd rather not go down that rabbit hole to fix something which is seemingly just a bug in the implementation. If an RFC passes that makes all arithmetic on NULL an error in PHP 8.0 (i.e. with no deprecation period), my RFC will be redundant; but that seems unlikely. I am less precious about the changes to boolean, which is why I propose three separate votes. I would be open to an alternative RFC removing all implicit casts on boolean, object, and resource; but unless and until undefined variable behaviour changes, null is necessarily a special case. Regards, -- Rowan Tommins [IMSoP]
  108808
March 2, 2020 14:46 ocramius@gmail.com (Marco Pivetta)
Hey Rowan,

On Mon, Mar 2, 2020 at 3:39 PM Rowan Tommins collins@gmail.com>
wrote:

> Hi Marco, > > On Mon, 2 Mar 2020 at 14:00, Marco Pivetta <ocramius@gmail.com> wrote: > > > > > Overall against the RFC: `++` and `--` (prefix and suffix) are supposed > to > > be used with numeric values, not with other types, as that makes > everything > > only more confusing. > > > > > While I'm generally sympathetic to that principle, it could also be said of > the + and - operators, which currently silently handle both nulls and > booleans. I would be open to changing *all* arithmetic behaviour, > particularly on booleans, but don't think a vague ambition for that > justifies inconsistent behaviour between different arithmetic operators. >
Yes, I'm indeed not suggesting to change them: my comment is explicitly about turning the BC break (in this RFC) into an useful and *loud* BC break.
> > Code written (intentionally) to use `++` and `--` against non-numeric > > values should **NOT** pass a code review, and I am sorry for those that > > have to maintain it if that happens. > > > > > I think you're taking simplified examples too literally here, and assuming > that all affected code would be obvious at a glance. > > Take for instance this innocent-looking patch: > > function foo($bar) { > - $bar -= 1; > + $bar--; > // more code here > } > > With the current behaviour, any inadvertent calls to the function passing > null, true, false, or an array will change behaviour when this patch is > merged. In particular, the variable $bar would previously have been > guaranteed to be an integer, and will now retain the original type passed > in. > > I completely agree that such code has a bug, but the difference in > behaviour is still a massive WTF. >
The code in the above example cannot be accepted without an `int` type declaration, or an `(int)` cast (or similar number) applied before the first arithmetic happens. For older code that needs to respect BC due to LSP, a docblock type declaration would suffice. The code above is NOT going to pass a code review. The asymmetry of ++ and -- is even more confusing. Consider this patch:
> > function repeatThing($extraTimes) { > - for ( $i = 0; $i <= $extraTimes ; $i ++ ) { > + for ( $i = $extraTimes ; $i >= 0; $i -- ) { > doThing(); > } > } > > Again, this looks like a safe change - but if passed a value of NULL, the > old code will call doThing() once, and the new code will enter an infinite > loop. >
Same as above: types are not optional, and `$extraTimes` is not usable in its form. This code cannot pass a code review either.
> > > > Changing the current behavior, regardless in which way, is a BC break: > > might as well make the BC break useful: > > > > RFC Proposal: `$a = null; $a--; $a === -1`. Let's make this an explicit > > TypeError instead. > > RFC Proposal: `$a = null; --$a; $a === -1`. Let's make this an explicit > > TypeError instead. > > > > > And what about `$a = null; $a++;` / `$a = null; ++$a;`, which currently > has behaviour consistent with other arithmetic operations (in particular, > is identical to $a+=1) and this RFC does not propose to change? >
Leave unchanged. If a BC break is to be introduced, make it go with a bang (throw error) If we change the ++ case to throw a TypeError, would we also change null+1
> to throw one? And would we then also examine all the other operators and > utility functions which silently coerce null to zero or an empty string? >
Can also not change it: I'd throw an error, if I were to propose it, but the BC break must bring active value. Right now, psalm & phpstan catch these, luckily. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/
  108810
March 2, 2020 15:10 rowan.collins@gmail.com (Rowan Tommins)
On Mon, 2 Mar 2020 at 14:46, Marco Pivetta <ocramius@gmail.com> wrote:

> > The code above is NOT going to pass a code review. >
It may not pass a code review *from you*, but I don't think your high standards are a good benchmark for the way PHP is used throughout the world. Let's put it this way: if my RFC passes, and the following day an RFC makes it redundant by changing integer coercion in general, I will be happy. If an alternative is proposed which makes -- an error on arguments where both -1 and ++ are silently accepted, I will voice my dissent (I don't have a formal vote). If you feel that the BC break is too great, even in the null decrement case, feel free to vote No, and keep the current WTF in the language. Regards, -- Rowan Tommins [IMSoP]
  108807
March 2, 2020 14:40 cschneid@cschneid.com (Christian Schneider)
Am 02.03.2020 um 15:00 schrieb Marco Pivetta <ocramius@gmail.com>:
> Overall against the RFC: `++` and `--` (prefix and suffix) are supposed to > be used with numeric values, not with other types, as that makes everything > only more confusing. > > Code written (intentionally) to use `++` and `--` against non-numeric > values should **NOT** pass a code review, and I am sorry for those that > have to maintain it if that happens.
There are still those who find $wordcount[$word]++; useful enough to support both ++ on null as well as usage of uninitialised array indices. Please respect this kind of code, it is out there and fully functional.
> Changing the current behavior, regardless in which way, is a BC break: > might as well make the BC break useful: > > RFC Proposal: `$a = null; $a--; $a === -1`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = null; --$a; $a === -1`. Let's make this an explicit > TypeError instead. > > RFC Proposal: `$a = true; $a++; $a === 2`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = true; ++$a; $a === 2`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = true; $a--; $a === 0`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = true; --$a; $a === 0`. Let's make this an explicit > TypeError instead. > > RFC Proposal: `$a = false; $a++; $a === 1`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = false; ++$a; $a === 1`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = false; $a--; $a === -1`. Let's make this an explicit > TypeError instead. > RFC Proposal: `$a = false; --$a; $a === -1`. Let's make this an explicit > TypeError instead.
I don't care too much about -- but *if* you want to discourage ++ on those types then make it E_STRICT, E_NOTICE or even E_WARNING. Similar for non-numeric increments.
> In **addition** to that, we may propose removal of `++`, `--` and similar > from non-numeric types (will lead to TypeError) like Andrea started in > https://wiki.php.net/rfc/invalid_strings_in_arithmetic > > It makes no sense to keep a landmine there: let's get rid of it, instead of > empowering it further (and breaking BC too, while doing so).
I see your point and I can live with E_* but breaking well-defined and functioning code because you don't like $null++ is wrong. - Chris
  108901
March 8, 2020 19:31 rowan.collins@gmail.com (Rowan Tommins)
On 01/03/2020 21:23, Rowan Tommins wrote:


Hi all,

I have just posted a substantially rewritten version of the above RFC.

* The proposal to change boolean behaviour has been dropped.
* More justification has been included for treating nulls differently.
* The suggestion of raising errors just for the currently broken 
operators has been explicitly addressed.

I would urge anyone who reacted instinctively to the original proposal 
to read this version, and engage with the reasoning it presents, even if 
they disagree with my conclusions.

Thanks,

-- 
Rowan Tommins (né Collins)
[IMSoP]
  108902
March 8, 2020 20:01 marandall@php.net (Mark Randall)
On 08/03/2020 19:31, Rowan Tommins wrote:
> On 01/03/2020 21:23, Rowan Tommins wrote: >> https://wiki.php.net/rfc/increment_decrement_fixes
It reads better than before. IMO option c) is definitely the absolutely superior option for the long-term health of the ecosystem, of that I have absolutely no doubt at all. Simply, if it's arithmatic on something that's not a number, and isn't an overloaded object, it should throw. But that seems like it's too far away... so I could *maybe* get behind subtracting from null becoming -1, but I really don't like it. It just feels so wrong, but the only thing that feels worse is that we probably couldn't pass removing null++, at least in the base version, and that may set up null-- as the least-worst option. -- Mark Randall
  115910
September 1, 2021 08:13 nikita.ppv@gmail.com (Nikita Popov)
On Sun, Mar 8, 2020 at 8:31 PM Rowan Tommins collins@gmail.com>
wrote:

> On 01/03/2020 21:23, Rowan Tommins wrote: > > https://wiki.php.net/rfc/increment_decrement_fixes > > > Hi all, > > I have just posted a substantially rewritten version of the above RFC. > > * The proposal to change boolean behaviour has been dropped. > * More justification has been included for treating nulls differently. > * The suggestion of raising errors just for the currently broken > operators has been explicitly addressed. > > I would urge anyone who reacted instinctively to the original proposal > to read this version, and engage with the reasoning it presents, even if > they disagree with my conclusions. >
For the record, increment/decrement on arrays, resources and objects now throws due to https://wiki.php.net/rfc/arithmetic_operator_type_checks. Personally I think it's fine to change the behavior of "$null--", to make it consistent with "$null - 1" and "$null++". Making "$null--" into a TypeError should imho only be done if we also make other arithmetic on null a TypeError (e.g. $null + 1, $null * 2, etc.) Regards, Nikita