Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110999
MIME-Version: 1.0
References: <CAF+90c821boRO6YsO0SH0JbDb8E8eTHfEK=e4Y5_z9P7+OOnUg@mail.gmail.com>
 <CAJHtzny=SX_ojsohApAY8Xe7c_EMAKGbNe110QQVh7i5yGFm_A@mail.gmail.com>
 <CAF+90c-6iEBPjWuzA6=bAr=85N=JNV0+GqL3_maak3yqVP-e=Q@mail.gmail.com> <CAF+90c9DxVz2Ro3yJ0-jDuPXg5Ffe3F9ERE8wHt=7ERjOm01Sw@mail.gmail.com>
In-Reply-To: <CAF+90c9DxVz2Ro3yJ0-jDuPXg5Ffe3F9ERE8wHt=7ERjOm01Sw@mail.gmail.com>
Date: Tue, 14 Jul 2020 15:48:30 +0200
Message-ID: <CAF+90c_vyLakwP=1QrWUJh2rwJtcsMtPSYzyqxqOeXecGYcQTg@mail.gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000f0657605aa671082"
Subject: Re: [PHP-DEV] [RFC] Saner string to number comparisons
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000f0657605aa671082
Content-Type: text/plain; charset="UTF-8"

On Thu, Jul 2, 2020 at 10:09 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> On Mon, Mar 4, 2019 at 6:00 PM Nikita Popov <nikita.ppv@gmail.com> wrote:
>
>> On Wed, Feb 27, 2019 at 10:23 AM Zeev Suraski <zeev@php.net> wrote:
>>
>>>
>>>
>>> On Tue, Feb 26, 2019 at 2:27 PM Nikita Popov <nikita.ppv@gmail.com>
>>> wrote:
>>>
>>>> Hi internals,
>>>>
>>>> I think it is well known that == in PHP is a pretty big footgun. It
>>>> doesn't
>>>> have to be. I think that type juggling comparisons in a language like
>>>> PHP
>>>> have some merit, it's just that the particular semantics of == in PHP
>>>> make
>>>> it so dangerous. The biggest WTF factor is probably that 0 == "foobar"
>>>> returns true.
>>>>
>>>> I'd like to bring forward an RFC for PHP 8 to change the semantics of ==
>>>> and other non-strict comparisons, when used between a number and a
>>>> string:
>>>>
>>>> https://wiki.php.net/rfc/string_to_number_comparison
>>>>
>>>> The tl;dr is that if you compare a number and a numeric string, they'll
>>>> be
>>>> compared as numbers. Otherwise, the number is converted into a string
>>>> and
>>>> they'll be compared as strings.
>>>>
>>>> This is a very significant change -- not so much because the actual BC
>>>> breakage is expected to be particularly large, but because it is a
>>>> silent
>>>> change in core language semantics, which makes it hard to determine
>>>> whether
>>>> or not code is affected by the change. There are things we can do about
>>>> this, for example the RFC suggests that we might want to have a
>>>> transition
>>>> mode where we perform the comparison using both the old and the new
>>>> semantics and warn if the result differs.
>>>>
>>>> I think we should give serious consideration to making such a change.
>>>> I'd
>>>> be interested to hear whether other people think this is worthwhile, and
>>>> how we could go about doing it, while minimizing breakage.
>>>>
>>>
>>> I generally like the direction and think we should seriously consider it.
>>>
>>> I think that before we make any decisions on this, or even dive too deep
>>> into the discussion - we actually need to implement this behavior,
>>> including the proposed INI setting you mentioned we might add in 7.4 - and
>>> see what happens in some real world apps, at least in terms of potential
>>> danger (as you say, figuring out whether there's actual breakage would
>>> require a full audit of every potentially problematic sample.  Ultimately,
>>> I think there's no question that if we were to start from scratch, we'd be
>>> going for something along these lines.  But since we're not starting from
>>> scratch - scoping the level of breakage is key here.
>>>
>>> Zeev
>>>
>>
>> Totally agree that assessing the amount of breakage in real code is key
>> here. I have now implemented a warning for PHP 7.4 (for now unconditional,
>> no ini setting) that is thrown whenever the result of a comparison is going
>> to change under the currently proposed rules:
>> https://github.com/php/php-src/pull/3917
>>
>> I've done a few initial tests by running this against the Laravel,
>> Symfony and pear-core. The warning was thrown 2 times for Laravel, 1 times
>> for Symfony and 2 times for pear-core. (See PR for the results.)
>>
>> Both of the warnings in pear-core pointed to implementation bugs. The
>> Symfony warning was due to trailing whitespace not being allowed in numeric
>> strings (something we should definitely change). One of the Laravel
>> warnings is ultimately a false-positive (does not change behavior), though
>> code could be improved to avoid it. I wasn't able to tell whether the other
>> one is problematic, as it affects sorting order.
>>
>> I have to say that this is a lot less warnings than I expected. Makes me
>> wonder if I didn't make an implementation mistake ^^
>>
>> Regards,
>> Nikita
>>
>
> As we're moving closer to PHP 8 feature freeze, I want to give this RFC a
> bump. I've updated the text to account for some changes that have happened
> in the meantime, such as the removal of locale-sensitivity for float to
> string conversions.
>
> It's been quite a while since we discussed this last, and back then the
> discussion was fairly positive. Some experiments with a warning mode also
> showed that the impact, at least in framework/library code, appears to be
> fairly low in practice, contrary to what one might intuitively expect.
>
> Now would be the time to decide whether or not we want to pursue this
> change for PHP 8.
>

And then there was silence...

I think I'll just put this up for vote on Friday, and we'll see what people
think :)

Nikita

--000000000000f0657605aa671082--