Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added switchable warning for comparison numbers and strings #292

Closed
wants to merge 10 commits into from

Conversation

i582
Copy link
Contributor

@i582 i582 commented Aug 18, 2021

In PHP 8, the logic for comparing numbers and strings has changed, so we need a way to find all such places, while not including warning at all, as this will lead to a huge number of warnings.

In order to enable the warning, the set_migration_php8_warning function has been added, with which you can enable or disable the warning. This function accepts a bitset, in which only the first right bit is used so far, but the rest will be used in the future.

<?php
$b = 0;

set_migration_php8_warning(0b001);

if ($b == "") {} // warning

set_migration_php8_warning(0b000);

if ($b == "") {} // no warning

At the end of each request, the warning is automatically disabled.

Changes in

The operators <=>, ==, !=, >, >=, <, and <=.
The functions in_array(), array_search() and array_keys() with $strict set to false (which is the default).
The sorting functions sort(), rsort(), asort(), arsort() and array_multisort() with $sort_flags set to SORT_REGULAR (which is the default).

Links

RFC: https://wiki.php.net/rfc/string_to_number_comparison

In documentation: String to Number Comparison

The following table shows how the result of some simple comparisons changes (or doesn't change) under this RFC:

Comparison Before After
0 == "0" true true
0 == "0.0" true true
0 == "foo" true false
0 == "" true false
42 == " 42" true true
42 == "42foo" true false

See full table in tests:

TEST(number_string_comparison_test, test_comp_int) {

Note

We are already comparing infinities as in PHP 8.
See Special values part of RFC.

Related changes (not implemented in this PR, see #298)

In https://wiki.php.net/rfc/saner-numeric-strings (implemented in PHP 8):

42 == "42"; // true
42 == "   42"; // true
42 == "42   "; // true

Before

42 == "42"; // true
42 == "   42"; // true
42 == "42   "; // false

See VKCOM/kphp-polyfills#25
#290

@i582 i582 added the PHP8 PHP8 feature label Aug 18, 2021
@i582 i582 changed the title added switchable warning for comparison numbers and strings WIP: added switchable warning for comparison numbers and strings Aug 18, 2021
@i582 i582 changed the title WIP: added switchable warning for comparison numbers and strings added switchable warning for comparison numbers and strings Aug 18, 2021
@i582 i582 changed the title added switchable warning for comparison numbers and strings WIP: added switchable warning for comparison numbers and strings Aug 18, 2021
@i582 i582 changed the title WIP: added switchable warning for comparison numbers and strings added switchable warning for comparison numbers and strings Aug 23, 2021
runtime/mixed.inl Outdated Show resolved Hide resolved
runtime/mixed.inl Outdated Show resolved Hide resolved
tests/python/tests/migration_php8/test_comparison.py Outdated Show resolved Hide resolved
@tolk-vm tolk-vm added this to the next milestone Aug 25, 2021
i582 and others added 3 commits August 29, 2021 22:39
set_migration_php8_warning
- Now this function accepts a bit mask where each bit
sets the warnings to be triggered (currently for
comparisons only)
- Format strings are now created using template functions
@i582
Copy link
Contributor Author

i582 commented Sep 14, 2021

Closed due #298
Thшы two PRs are merged into one since the second depends on the first.

@i582 i582 closed this Sep 14, 2021
@i582 i582 deleted the pmakhnev/add_warning_for_number_string_comparisons branch September 15, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP8 PHP8 feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants