Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:111017 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 65569 invoked from network); 15 Jul 2020 10:18:37 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 15 Jul 2020 10:18:37 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 920C7180508 for ; Wed, 15 Jul 2020 02:11:14 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail-lj1-f170.google.com (mail-lj1-f170.google.com [209.85.208.170]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 15 Jul 2020 02:11:14 -0700 (PDT) Received: by mail-lj1-f170.google.com with SMTP id r19so1687333ljn.12 for ; Wed, 15 Jul 2020 02:11:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=90sqk6OFtmom14/P2Cc67t7Q1W2d3QssNQ3DjXmKdoQ=; b=YBSsT2gaal2c0hb6htxXjjJwtEmAvka74mwX2uP9x9eT3+xsquNFbaOaeVGaPGfL+Q fOGCwkF6M86y5VlEpZJ6s6jGXscTKrgmjnw1/krYsVh8/t+QSIu+kTVnoSBYcIweNchT Uf1NFnyxOshiLBE9Is9uJx2FXVsTbt8f9u4dGFjn72N3R0Yl0xEMchFJf58qGm18JRs+ fbcbnOJGkOnLITrwcEIvrFqcb9RRGrDdFhsH+TWeN/P4CbvSaSuPCSYjan8AHAXVBthf AsB7isP8jDK6dc4o8yMExTmSK9dKjvF0S5viKCYomgXmQSl+xwJhO21BlPF2zLxGReKY EZQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=90sqk6OFtmom14/P2Cc67t7Q1W2d3QssNQ3DjXmKdoQ=; b=VDcGeDOTixeBbh76V4F342V/JpVmhPLD7KnTWlhLP72Z4NBpTwoir48p4yvFkITUXN 7n16JzyRzv7KYWiYB+51ZheXvx+GBMR+LGzkB8jIxkmARUnrM39Ej5RAbgCiHh5pQoLU YqrTBe8/HV8/DM2MDshbQ15nH0ZWG/byRQXxYkwLOgqHJoWX37VHNe2h7WLRcm6I2inM Th++RMnZWe5QdZ0p37sOARfd6/tzLflI2l3yuryz4G2R/wUtcHhUPgfxB8BbtlQULMuD 3BDv6c3SRjATekQ1ZDoeagaAfBJv5qDuzzahiiZnbe+LLGVUa187wrRxFzVduwHoVAVh QIbw== X-Gm-Message-State: AOAM531fKsoaOtHNMQfcZ8DMEhF28fdmfSuOgQJzyJrwd3dTrxH1M45K +Y/s9PtK3f1hifLcztIRyuf66tAQhf1i7Lsl8Ds= X-Google-Smtp-Source: ABdhPJzNlYNTLOVttQkBaGt3p8wnCy2PVY3xkY5wLuc2Rm2nWfSW2RmiA/HFQrjPzVhKN17//b12qTyw7jpQKc3XPRY= X-Received: by 2002:a2e:8199:: with SMTP id e25mr3980294ljg.307.1594804271035; Wed, 15 Jul 2020 02:11:11 -0700 (PDT) MIME-Version: 1.0 References: <529e7a72-8bd7-b4f4-a987-0e88d37e47b5@telia.com> <44637D3E-5F0B-4AEF-A154-21C8FF20ED91@stitcher.io> <35A1BC6F-D147-4692-8A12-A13B5ABF78D5@stitcher.io> In-Reply-To: <35A1BC6F-D147-4692-8A12-A13B5ABF78D5@stitcher.io> Date: Wed, 15 Jul 2020 11:10:55 +0200 Message-ID: To: Brent Roose Cc: PHP internals Content-Type: multipart/alternative; boundary="0000000000000fb8bb05aa774e6a" Subject: Re: [PHP-DEV] [RFC] Saner string to number comparisons From: nikita.ppv@gmail.com (Nikita Popov) --0000000000000fb8bb05aa774e6a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Jul 15, 2020 at 10:56 AM Brent Roose wrote: > Hi Nikita > > Yes that would be nice, if it's not too much of a hassle. I'm only able t= o > 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 > On 15 Jul 2020, at 10:53, Nikita Popov wrote: > > On Wed, Jul 15, 2020 at 10:49 AM Brent Roose 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 > >> On 15 Jul 2020, at 10:28, Nikita Popov wrote: >> >> On Tue, Jul 14, 2020 at 11:47 PM Bj=C3=B6rn Larsson > > >> wrote: >> >> Den 2020-07-14 kl. 15:48, skrev Nikita Popov: >> >> On Thu, Jul 2, 2020 at 10:09 AM Nikita Popov >> >> wrote: >> >> >> On Mon, Mar 4, 2019 at 6:00 PM Nikita Popov >> >> wrote: >> >> >> On Wed, Feb 27, 2019 at 10:23 AM Zeev Suraski wrote: >> >> >> On Tue, Feb 26, 2019 at 2:27 PM Nikita Popov >> wrote: >> >> Hi internals, >> >> I think it is well known that =3D=3D 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 =3D=3D in PH= P >> make >> it so dangerous. The biggest WTF factor is probably that 0 =3D=3D >> >> "foobar" >> >> returns true. >> >> I'd like to bring forward an RFC for PHP 8 to change the semantics >> >> of =3D=3D >> >> 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 >> >> >> > --0000000000000fb8bb05aa774e6a--