[RFC] Saner string to number comparisons

6 years ago by Nikita Popov — view source — reply

unread

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

6 years ago by G. P. B. — view source — reply

unread

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

6 years ago by Rowan Collins — view source — reply

unread

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]

6 years ago by Nikita Popov — view source — reply

unread

On Tue, Feb 26, 2019 at 2:06 PM Rowan Collins rowan.collins@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

6 years ago by Rowan Collins — view source — reply

unread

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]

6 years ago by Dmitry Stogov — view source — reply

unread

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.

6 years ago by Kingsquare.nl - Robin Speekenbrink — view source — reply

unread

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

6 years ago by Arvids Godjuks — view source — reply

unread

ср, 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

6 years ago by Claude Pache — view source — reply

unread

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:

$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

6 years ago by Zeev Suraski — view source — reply

unread

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

6 years ago by Gabriel Caruso — view source — reply

unread

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

6 years ago by Nikita Popov — view source — reply

unread

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

5 years ago by Nikita Popov — view source — reply

unread

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

5 years ago by Nikita Popov — view source — reply

unread

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

5 years ago by Nikita Popov — view source — reply

unread

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

5 years ago by bjorn.x.larsson@telia.com — view source — reply

unread

Den 2020-07-14 kl. 15:48, skrev Nikita Popov:

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

5 years ago by Nikita Popov — view source — reply

unread

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

5 years ago by Brent Roose — view source — reply

unread

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

On Tue, Jul 14, 2020 at 11:47 PM Björn Larsson <bjorn.x.larsson@telia.com mailto: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 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

5 years ago by Nikita Popov — view source — reply

unread

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

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

5 years ago by Brent Roose — view source — reply

unread

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

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 https://github.com/php/php-src/pull/3917. I could rebase that to current 7.4 if that would be useful.

Nikita

On Tue, Jul 14, 2020 at 11:47 PM Björn Larsson <bjorn.x.larsson@telia.com mailto: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 mailto:nikita.ppv@gmail.com>
wrote:

On Mon, Mar 4, 2019 at 6:00 PM Nikita Popov <nikita.ppv@gmail.com mailto:nikita.ppv@gmail.com>
wrote:

On Tue, Feb 26, 2019 at 2:27 PM Nikita Popov <nikita.ppv@gmail.com mailto: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 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 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 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

5 years ago by Nikita Popov — view source — reply

unread

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

Done! https://github.com/php/php-src/pull/3917 is now based on current 7.4
HEAD. Note that it just unconditionally throws a warning, without a way to
disable it.

Nikita

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

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

5 years ago by Arnold Daniels — view source — reply

unread

Hi Nikita,

One of the problems with numeric string comparisons is that it might
interpret a hexadecimal value as scientific notation

$red = '990000';
$purple = '9900e2';
$red == $purple; // true

I suggest only interpreting a number formatted with a sign ("1e+100") or
with decimals ("1.0e100") as scientific notation as part of this RFC. These
are the notations that languages use when casting a float to a string,
never "1e100"

PHP "1.0E+100"
JavaScript "1e+100"
Python "1e+100"
Ruby "1.0e+100"
Java "1.0E100"

Arnold

5 years ago by Nikita Popov — view source — reply

unread

On Wed, Jul 15, 2020 at 5:12 PM Arnold Daniels arnold.adaniels.nl@gmail.com
wrote:

Hi Nikita,

One of the problems with numeric string comparisons is that it might
interpret a hexadecimal value as scientific notation
$red = '990000';
$purple = '9900e2';
$red == $purple; // true
I suggest only interpreting a number formatted with a sign ("1e+100") or
with decimals ("1.0e100") as scientific notation as part of this RFC. These
are the notations that languages use when casting a float to a string,
never "1e100"

PHP "1.0E+100"
JavaScript "1e+100"
Python "1e+100"
Ruby "1.0e+100"
Java "1.0E100"

Arnold

Hey Arnold,

I don't want to include this change in this RFC (because the proposal does
not otherwise touch string-to-string comparison at all), but I do think
this is a very good idea. I was not aware of this particular variant of the
issue (i.e., that it also occurs with prefixes other than "0e") and this
seems like a pragmatic way to resolve that particular ambiguity. A question
here would be whether to make this something specific to string-to-string
comparisons, or make a general change to numeric string recognition (i.e.,
no longer treat "1e100" as a numeric string, only "1.0e100" or "1e+100".)

Regards,
Nikita

5 years ago by Brent Roose — view source — reply

unread

Hey Nikita

Thanks for the rebase. I just tested this on one of our most largest projects (after verifying that the warning does show in a dummy test case), and all is fine. So from my point of view, there is a theoretical chance of breaking code, but I believe this won't have a large impact, at least not on modern-day projects.

Kind regards
Brent

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

Done! https://github.com/php/php-src/pull/3917 https://github.com/php/php-src/pull/3917 is now based on current 7.4
HEAD. Note that it just unconditionally throws a warning, without a way to
disable it.

Nikita

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 https://github.com/php/php-src/pull/3917. I could rebase that to current
7.4 if that would be useful.

Nikita

On Tue, Jul 14, 2020 at 11:47 PM Björn Larsson <bjorn.x.larsson@telia.com mailto: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 mailto:nikita.ppv@gmail.com>

wrote:

On Mon, Mar 4, 2019 at 6:00 PM Nikita Popov <nikita.ppv@gmail.com mailto:nikita.ppv@gmail.com>

wrote:

On Tue, Feb 26, 2019 at 2:27 PM Nikita Popov <nikita.ppv@gmail.com mailto: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 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 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 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

5 years ago by Nikita Popov — view source — reply

unread

Hey Nikita

Thanks for the rebase. I just tested this on one of our most largest
projects (after verifying that the warning does show in a dummy test case),
and all is fine. So from my point of view, there is a theoretical chance of
breaking code, but I believe this won't have a large impact, at least not
on modern-day projects.

Kind regards
Brent

Great to hear, thanks for testing!

Regards,
Nikita

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

Done! https://github.com/php/php-src/pull/3917 is now based on current 7.4
HEAD. Note that it just unconditionally throws a warning, without a way to
disable it.

Nikita

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

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

6 years ago by Niklas Keller — view source — reply

unread

Am Mo., 4. März 2019 um 18:00 Uhr schrieb 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

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