Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] Saner string to number comparisons »

[RFC] Saner string to number comparisons

php.internals

Nikita Popov

7 years ago
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. Regards, Nikita

Girgias

7 years ago
On Tue, 26 Feb 2019 at 13:27, 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. > > Regards, > Nikita >
I am in favor of this change however I do think having the Trailing Whitespaces numerics RFC would be better for this change as I do agree with you that IMHO 42 == "42 " should return true. I'm also not sure if you maybe should point this out sooner but as it is currently seems fine to me. However small nitpick on your precision section $float = 1.75; ini_set <http://www.php.net/ini_set>('precision', 14); // Defaultvar_dump <http://www.php.net/var_dump>($float < "1.75abc");// Behaves likevar_dump <http://www.php.net/var_dump>("1.75" < "1.75abc"); // true is wrong because var_dump($float < "1.75abc"); returns false: https://3v4l.org/G56Vj I think you meant to use the <= comparison operator maybe? Also the NAN behavior is a bit surprising (to me atleast) but it does follow the IEEE-754 However I don't really understand why it always returns 1 with the TIE operator. And finally as to how to do the transition I don't really have a strong opinion about it. An INI setting seems like this behavior is going to be supported for a long time. But it doesn't seem like its possible to throw deprecation notices to alert of this change. Best regards George P. Banyard

Rowan Collins

7 years ago
On Tue, 26 Feb 2019 at 12:27, Nikita Popov <nikita.ppv@gmail.com> wrote:
> 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 >
Hi Nikita, Thanks for tackling this; I think if we can improve this, we'll be answering a lot of language critics (I'm sure they'll find something else to complain about, but that's life!). However, I'm concerned that it doesn't go far enough, when you say that the following will still return true: 0 == "0e214987142012" "0" == "0e214987142012" I think the cases where this is useful are vastly outweighed by the cases where it's completely unexpected, and potentially dangerous (e.g. in a hash comparison). If this is not fixed, the "dogma to always avoid non-strict comparisons" you refer to will remain. If I understand it right, this arises from the fact that "0e214987142012" is considered a "well-formed numeric string", which is cast to int(0) or float(0). Is it feasible to also narrow this definition as part of the same change? Regards,
-- Rowan Collins [IMSoP]

Nikita Popov

7 years ago
On Tue, Feb 26, 2019 at 2:06 PM Rowan Collins <rowan.collins@gmail.com> wrote:
> On Tue, 26 Feb 2019 at 12:27, Nikita Popov <nikita.ppv@gmail.com> wrote: > > > 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 > > > > > Hi Nikita, > > Thanks for tackling this; I think if we can improve this, we'll be > answering a lot of language critics (I'm sure they'll find something else > to complain about, but that's life!). > > However, I'm concerned that it doesn't go far enough, when you say that the > following will still return true: > > 0 == "0e214987142012" > "0" == "0e214987142012" > > I think the cases where this is useful are vastly outweighed by the cases > where it's completely unexpected, and potentially dangerous (e.g. in a hash > comparison). If this is not fixed, the "dogma to always avoid non-strict > comparisons" you refer to will remain. > > If I understand it right, this arises from the fact that "0e214987142012" > is considered a "well-formed numeric string", which is cast to int(0) or > float(0). Is it feasible to also narrow this definition as part of the same > change? >
Yes, that's right. However, it's probably worth mentioning that string to string comparisons are subject to additional constraints beyond the well-formedness requirement. Since PHP 5.4.4 there are additional overflow protections in place, which prevent numeric comparison from applying when both sides are integers that overflow to double and become equal as a result of that. This means that "100000000000000000000" == "100000000000000000001" returns false rather than true since PHP 5.4.4. I'm mentioning this, because it is a precedent for tweaking the string to string numeric comparison rules to prevent unexpected and possibly security critical equalities. I think we could add similar special handling for the "0eNNNN" == "0eMMMM" case, as this is another "catastrophic" case when it comes to comparisons of hashes that happen to start with 0e, for example. It might be better to discuss such a change separately from this proposal though (it's much more minor, and something that can also conceivable go into a minor version, given that the previous change was applied in a patch release). Regards, Nikita

Rowan Collins

7 years ago
On 26 February 2019 13:26:24 GMT+00:00, Nikita Popov <nikita.ppv@gmail.com> wrote:
>I'm mentioning this, because it is a precedent for tweaking the string >to >string numeric comparison rules to prevent unexpected and possibly >security >critical equalities. I think we could add similar special handling for >the >"0eNNNN" == "0eMMMM" case, as this is another "catastrophic" case when >it >comes to comparisons of hashes that happen to start with 0e, for >example.
That makes sense. Personally, I find the treatment of strings in this e-notation problematic in all contexts - it makes is_numeric() much less useful for validation, for instance - but realise we have to balance compatibility here.
>It might be better to discuss such a change separately from this >proposal >though (it's much more minor, and something that can also conceivable >go >into a minor version, given that the previous change was applied in a >patch >release).
I think keeping it to a separate RFC is fine, but it would be nice to target the same release. "We've made == safer" is something that we can shout about, even if it's composed of a bunch of small tweaks. It also gives one upgrade where people need to look for subtle breaks, rather than two. Regards,
-- Rowan Collins [IMSoP]

Dmitry Stogov

7 years ago
Hi Nikita, Yeah, this is a big BC break, but I think, it's a good time to make some "cleanup" in PHP-8. The only thing, I don't like is a difference between leading and trailing spaces. They should behave in the same way. Thanks. Dmitry. ________________________________ From: Nikita Popov <nikita.ppv@gmail.com> Sent: Tuesday, February 26, 2019 3:27:23 PM To: PHP internals Subject: [PHP-DEV] [RFC] Saner string to number comparisons 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. Regards, Nikita

Kingsquare.nl - Robin Speekenbrink

7 years ago
Op di 26 feb. 2019 om 13:27 schreef Nikita Popov <nikita.ppv@gmail.com>:
> 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 > > ... > > Regards, > Nikita >
Dear All, From a user-space POV this is a very good way forward and I (personally) really like the proposal of having a transitional period (7.4 anyone?) As of the 0 == "" bit: I do think that an empty string is widespread regarded as falsey-string... Thus 0 == "" sould IMHO return true... Just my 2 cents, keep up the excellent Nikita! (and everyone else ofcourse) Regards, Robin

Arvids Godjuks

7 years ago
ср, 27 февр. 2019 г. в 10:06, Kingsquare.nl - Robin Speekenbrink < robin@kingsquare.nl>:
> Op di 26 feb. 2019 om 13:27 schreef Nikita Popov <nikita.ppv@gmail.com>: > > > 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 > > > > ... > > > > Regards, > > Nikita > > > > Dear All, > > From a user-space POV this is a very good way forward and I (personally) > really like the proposal of having a transitional period (7.4 anyone?) > > As of the 0 == "" bit: I do think that an empty string is widespread > regarded as falsey-string... Thus 0 == "" sould IMHO return true... > > Just my 2 cents, keep up the excellent Nikita! > (and everyone else ofcourse) > > Regards, > Robin >
It's a bad idea to leave 0 == "" - remember - we are dealing with strings that come in via HTTP, so when you handle form input a 0 can be a valid input value, but an empty string is not. Sure, a === is better used here or empty(), but we should not leave edge cases like these. This is a string to a number comparison - it should trigger notice or error or whatever is designed to happen when "abc" == 123 is being done.
-- Arvīds Godjuks +371 26 851 664 arvids.godjuks@gmail.com Skype: psihius Telegram: @psihius https://t.me/psihius

Claude Pache

7 years ago
> Le 27 févr. 2019 à 09:06, Kingsquare.nl - Robin Speekenbrink <robin@kingsquare.nl> a écrit : > > As of the 0 == "" bit: I do think that an empty string is widespread > regarded as falsey-string... Thus 0 == "" sould IMHO return true... >
0 == "" evaluating to true has been a footgun for me in the past; something like that: ```php $state = $_GET['state'] ?? null; // ... switch ($state) { case 0: // when $state === "", this branch is incorrectly chosen // ... } ``` where the `state` parameter comes from <select name="state"> and is supposed to provide an optional filter criterion for some list. The fact that both values are falsy doesn’t mean that they should be equal (hopefully, `array() == 0` evaluates to false). —Claude

Zeev Suraski

7 years ago
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

Nikita Popov

7 years ago
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

Niklas Keller

7 years ago
Am Mo., 4. März 2019 um 18:00 Uhr schrieb Nikita Popov <nikita.ppv@gmail.com>:
> > 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
Most warnings will not come from framework code, but code interfacing with browsers, I guess. Regards, Niklas

Nikita Popov

6 years ago
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. Regards, Nikita

Nikita Popov

6 years ago
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. >
Thinking about the detailed behavior of the RFC, I think it might be better to change the semantics to:
> If one of the operands is NaN, return uncomparable (1). > Otherwise, convert the int/float operand to string with precision=-1 and
compare both operands as strings (with the usual "smart" logic). This differs from what the RFC currently specifies only in the behavior of comparison between a float and a non-numeric strings (because the string representation of the float currently depends on precision), but I think specifying it this way simplifies the mental model behind it, and makes the intended semantics more obvious. The actual implementation would of course be closer to what is specified in the RFC right now. Regards, Nikita

Nikita Popov

6 years ago
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

Björn Larsson

6 years ago
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

Nikita Popov

6 years ago
On Tue, Jul 14, 2020 at 11:47 PM Björn Larsson <bjorn.x.larsson@telia.com> wrote:
> 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? >
Thanks, I've updated the link to point to the new proposal! Nikita

Brent

6 years ago
Hi Nikita Is the ini setting available in current 7.4 builds? Is it documented somewhere? I'd like to test this change in some of our projects. Kind regards Brent

Nikita Popov

6 years ago
On Wed, Jul 15, 2020 at 10:49 AM Brent Roose <brendt@stitcher.io> wrote:
> Hi Nikita > > Is the ini setting available in current 7.4 builds? Is it documented > somewhere? I'd like to test this change in some of our projects. >
We did not introduce an ini setting in PHP 7.4, I only used it for my own experiments. The implementation is available at https://github.com/php/php-src/pull/3917. I could rebase that to current 7.4 if that would be useful. Nikita

Brent

6 years ago
Hi Nikita Yes that would be nice, if it's not too much of a hassle. I'm only able to test this in one or two large Laravel projects, so it would still be a limited test. Kind regards Brent

Gabriel Caruso

7 years ago
Em ter, 26 de fev de 2019 às 09:28, Nikita Popov <nikita.ppv@gmail.com> escreveu:
> 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. > > Regards, > Nikita >
Hi Nikita, There's only one case that I'm not sure about
> Comparison | Before | After > ------------------------------ > 0 == "" | true | false
I know that a cast of am empty and non-empty string results into 0 ( https://3v4l.org/W0P3Z), but still, not sure about this one in specific. Best,
-- Gabriel Caruso