Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108806
MIME-Version: 1.0
References: <3f615d82-a84b-697b-5c02-8d915270f92c@gmail.com> <CADyq6sJ5ErMVg2EjUcVM17omtN+Tx_bZ69RB_4b44n-Y=oxzSw@mail.gmail.com>
In-Reply-To: <CADyq6sJ5ErMVg2EjUcVM17omtN+Tx_bZ69RB_4b44n-Y=oxzSw@mail.gmail.com>
Date: Mon, 2 Mar 2020 14:39:30 +0000
Message-ID: <CALKiJKqx4a8=i0XEB=xM4Nn_Nvhn=ZVfvjMoni8GXJWRZ5h0Fg@mail.gmail.com>
To: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000580e78059fe02818"
Subject: Re: [PHP-DEV] [RFC] Increment/Decrement Fixes
From: rowan.collins@gmail.com (Rowan Tommins)

--000000000000580e78059fe02818
Content-Type: text/plain; charset="UTF-8"

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]

--000000000000580e78059fe02818--