Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:111035 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 43993 invoked from network); 16 Jul 2020 08:28:41 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 16 Jul 2020 08:28:41 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 507791804B3 for ; Thu, 16 Jul 2020 00:21:29 -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,HTML_MESSAGE,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from srv015.mail.ichtushosting.com (srv015.mail.ichtushosting.com [159.69.182.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Thu, 16 Jul 2020 00:21:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=stitcher.io ; s=default; h=References:To:Cc:In-Reply-To:Date:Subject:Mime-Version: Content-Type:Message-Id:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=cLrHQiHvfbcU74KP59HXBNMo5Jvlb0jbYSKC6Hhtg4g=; b=b7yFFzvByZ+eDmHd28LgTcVGWh zTk8cWNmZabvdjeKzRTx0bfUKcdBUCpi80oKrWe+MYl4Rced74VOifMZYoYyr+kEpTa049vXQvgPp pzkNAb7dz6xXm/3kz1FMSqqg6pCJyrXlCm9xHhLIntpY8rRusVcCtI5wxjHNBv2YcmPrumn85qmuj g8r8XLetgYqXYPzeDCbzrAcO8WYTvpQdNK8n0suMOHPoImmaEkCpFxpyaR52gFPbirzZdfipcarQL /iQ+hytcSOjuem65bhWOzePbT3d7M/upecoT23PFP0RYWPBYH6BFbVi3Pb8ZezxW1wzIDyq3U0tem fE+Cx4vA==; Received: from srv020.mail.ichtushosting.com ([78.46.213.219]) by srv015.mail.ichtushosting.com stage1 with esmtp (Exim MailCleaner) id 1jvyCr-0005Gi-IV from ; Thu, 16 Jul 2020 09:21:25 +0200 Received: from [192.168.1.23] (d54C4E0D4.access.telenet.be [84.196.224.212]) (Authenticated sender: brendt@stitcher.io) by srv020.mail.ichtushosting.com (Postfix) with ESMTPSA id EB3353EACF; Thu, 16 Jul 2020 09:21:23 +0200 (CEST) X-MailCleaner-SPF: pass Message-ID: Content-Type: multipart/alternative; boundary="Apple-Mail=_506CB81F-E59A-4C9A-B7A8-9FB6711CC2A6" Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) Date: Thu, 16 Jul 2020 09:21:23 +0200 In-Reply-To: Cc: PHP internals To: Nikita Popov References: <529e7a72-8bd7-b4f4-a987-0e88d37e47b5@telia.com> <44637D3E-5F0B-4AEF-A154-21C8FF20ED91@stitcher.io> <35A1BC6F-D147-4692-8A12-A13B5ABF78D5@stitcher.io> X-Mailer: Apple Mail (2.3608.80.23.2.2) X-MailCleaner-TrustedIPs: Ok Subject: Re: [PHP-DEV] [RFC] Saner string to number comparisons From: brendt@stitcher.io (Brent Roose) --Apple-Mail=_506CB81F-E59A-4C9A-B7A8-9FB6711CC2A6 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 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 > On 15 Jul 2020, at 11:10, Nikita Popov wrote: >=20 > On Wed, Jul 15, 2020 at 10:56 AM Brent Roose > wrote: >=20 >> Hi Nikita >>=20 >> 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. >>=20 >> Kind regards >> Brent >>=20 >=20 > 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. >=20 > Nikita >=20 >> On 15 Jul 2020, at 10:53, Nikita Popov > wrote: >>=20 >> On Wed, Jul 15, 2020 at 10:49 AM Brent Roose > wrote: >>=20 >>> Hi Nikita >>>=20 >>> 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. >>>=20 >>=20 >> 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. >>=20 >> Nikita >>=20 >>> On 15 Jul 2020, at 10:28, Nikita Popov > wrote: >>>=20 >>> On Tue, Jul 14, 2020 at 11:47 PM Bj=C3=B6rn Larsson = >>>>=20 >>> wrote: >>>=20 >>> Den 2020-07-14 kl. 15:48, skrev Nikita Popov: >>>=20 >>> On Thu, Jul 2, 2020 at 10:09 AM Nikita Popov > >>>=20 >>> wrote: >>>=20 >>>=20 >>> On Mon, Mar 4, 2019 at 6:00 PM Nikita Popov > >>>=20 >>> wrote: >>>=20 >>>=20 >>> On Wed, Feb 27, 2019 at 10:23 AM Zeev Suraski > wrote: >>>=20 >>>=20 >>> On Tue, Feb 26, 2019 at 2:27 PM Nikita Popov > >>> wrote: >>>=20 >>> Hi internals, >>>=20 >>> 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 PHP >>> make >>> it so dangerous. The biggest WTF factor is probably that 0 =3D=3D >>>=20 >>> "foobar" >>>=20 >>> returns true. >>>=20 >>> I'd like to bring forward an RFC for PHP 8 to change the semantics >>>=20 >>> of =3D=3D >>>=20 >>> and other non-strict comparisons, when used between a number and a >>> string: >>>=20 >>> https://wiki.php.net/rfc/string_to_number_comparison = >>>=20 >>> The tl;dr is that if you compare a number and a numeric string, >>>=20 >>> they'll >>>=20 >>> be >>> compared as numbers. Otherwise, the number is converted into a = string >>> and >>> they'll be compared as strings. >>>=20 >>> This is a very significant change -- not so much because the actual >>>=20 >>> BC >>>=20 >>> 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 >>>=20 >>> about >>>=20 >>> 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. >>>=20 >>> 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, >>>=20 >>> and >>>=20 >>> how we could go about doing it, while minimizing breakage. >>>=20 >>> I generally like the direction and think we should seriously = consider >>>=20 >>> it. >>>=20 >>>=20 >>> I think that before we make any decisions on this, or even dive too >>>=20 >>> deep >>>=20 >>> into the discussion - we actually need to implement this behavior, >>> including the proposed INI setting you mentioned we might add in 7.4 >>>=20 >>> - and >>>=20 >>> see what happens in some real world apps, at least in terms of >>>=20 >>> potential >>>=20 >>> danger (as you say, figuring out whether there's actual breakage = would >>> require a full audit of every potentially problematic sample. >>>=20 >>> Ultimately, >>>=20 >>> I think there's no question that if we were to start from scratch, >>>=20 >>> we'd be >>>=20 >>> going for something along these lines. But since we're not starting >>>=20 >>> from >>>=20 >>> scratch - scoping the level of breakage is key here. >>>=20 >>> Zeev >>>=20 >>> 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 >>>=20 >>> unconditional, >>>=20 >>> no ini setting) that is thrown whenever the result of a comparison = is >>>=20 >>> going >>>=20 >>> to change under the currently proposed rules: >>> https://github.com/php/php-src/pull/3917 = >>>=20 >>> 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 >>>=20 >>> times >>>=20 >>> for Symfony and 2 times for pear-core. (See PR for the results.) >>>=20 >>> Both of the warnings in pear-core pointed to implementation bugs. = The >>> Symfony warning was due to trailing whitespace not being allowed in >>>=20 >>> numeric >>>=20 >>> strings (something we should definitely change). One of the Laravel >>> warnings is ultimately a false-positive (does not change behavior), >>>=20 >>> though >>>=20 >>> code could be improved to avoid it. I wasn't able to tell whether = the >>>=20 >>> other >>>=20 >>> one is problematic, as it affects sorting order. >>>=20 >>> I have to say that this is a lot less warnings than I expected. = Makes >>>=20 >>> me >>>=20 >>> wonder if I didn't make an implementation mistake ^^ >>>=20 >>> Regards, >>> Nikita >>>=20 >>> As we're moving closer to PHP 8 feature freeze, I want to give this = RFC >>>=20 >>> a >>>=20 >>> bump. I've updated the text to account for some changes that have >>>=20 >>> happened >>>=20 >>> in the meantime, such as the removal of locale-sensitivity for float = to >>> string conversions. >>>=20 >>> 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 >>>=20 >>> also >>>=20 >>> showed that the impact, at least in framework/library code, appears = to >>>=20 >>> be >>>=20 >>> fairly low in practice, contrary to what one might intuitively = expect. >>>=20 >>> Now would be the time to decide whether or not we want to pursue = this >>> change for PHP 8. >>>=20 >>> And then there was silence... >>>=20 >>> I think I'll just put this up for vote on Friday, and we'll see what >>>=20 >>> people >>>=20 >>> think :) >>>=20 >>> Nikita >>>=20 >>>=20 >>> Seems like a very good idea!! Especially in conjunction with the = RFC: >>> - https://wiki.php.net/rfc/saner-numeric-strings = >>>=20 >>> Btw, in the RFC there is a reference to the "Trailing whitespace in >>> numeric >>> strings" RFC. Update to reference "Saner numeric strings" RFC = instead? >>>=20 >>>=20 >>> Thanks, I've updated the link to point to the new proposal! >>>=20 >>> Nikita --Apple-Mail=_506CB81F-E59A-4C9A-B7A8-9FB6711CC2A6--