Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:111008
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
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>
 <CAF+90c_vyLakwP=1QrWUJh2rwJtcsMtPSYzyqxqOeXecGYcQTg@mail.gmail.com>
Message-ID: <529e7a72-8bd7-b4f4-a987-0e88d37e47b5@telia.com>
Date: Tue, 14 Jul 2020 23:47:33 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <CAF+90c_vyLakwP=1QrWUJh2rwJtcsMtPSYzyqxqOeXecGYcQTg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-GB
Subject: Re: [PHP-DEV] [RFC] Saner string to number comparisons
From: bjorn.x.larsson@telia.com (=?UTF-8?Q?Bj=c3=b6rn_Larsson?=)

Den 2020-07-14 kl. 15:48, skrev Nikita Popov:
> 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

Seems like a very good idea!! Especially in conjunction with the RFC:
- https://wiki.php.net/rfc/saner-numeric-strings

Btw, in the RFC there is a reference to the "Trailing whitespace in numeric
strings" RFC. Update to reference "Saner numeric strings" RFC instead?

r//Björn L