Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116589
MIME-Version: 1.0
References: <CANzZr6kgZCF4DRKOnMTQr5gZ1LRQ2mMFn1OottwQR87qL=t25g@mail.gmail.com>
 <CAF+90c9i90FLNaL=bvN4=L0iJbb4RxGrfU-vt7Q09ZvG+gYsjA@mail.gmail.com>
 <CANzZr6=g3fYiLd5Wu-rfVSJEScC+NJnSKmtYzDULysJbe9qOnA@mail.gmail.com> <CAF+90c-uSTqkJg2hSvCvyvg+KxV8p4Asne7mLG_hb-MyUOZ2Nw@mail.gmail.com>
In-Reply-To: <CAF+90c-uSTqkJg2hSvCvyvg+KxV8p4Asne7mLG_hb-MyUOZ2Nw@mail.gmail.com>
Date: Wed, 8 Dec 2021 11:01:43 -0500
Message-ID: <CANzZr6=rnHzZix0pnDLM+d=i+EkrF_KiijX3-7vgwxNp0Ecb0A@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000e1acc805d2a49b80"
Subject: Re: [PHP-DEV] Automatic implementation of Stringable may conflict
 with old, untyped arginfo declarations
From: jmikola@gmail.com (Jeremy Mikola)

--000000000000e1acc805d2a49b80
Content-Type: text/plain; charset="UTF-8"

On Fri, Nov 12, 2021 at 7:00 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

>
> With the introduction of Stringable PHP also started automatically adding
> the string result type to __toString(), specifically for compatibility with
> the interface. As such, it should be safe to add the string return type
> everywhere for PHP >= 8.0. (If you use stubs with legacy arginfo, that
> would automatically give you the right behavior: types on PHP 8, no types
> on old versions.)
>

We haven't explored using stubs, but I'll keep that in mind. Andreas has
definitely mentioned that in the past and I believe it's on the list of
ideas to implement once we revisit typing across the board for the
extension (which we plan to do since we finally dropped support for PHP
7.1).

I previously added explicit string return types to all of our toString
methods (including interfaces), which I understand is a small BC break for
any userland classes implementing those interfaces (as rare as that is).

Just yesterday, I had an idea to use tentative return type info on the
interface toString methods ([mongodb/mongo-php-driver#1283](
https://github.com/mongodb/mongo-php-driver/pull/1283)), which seems to
require no changes at all to userland classes. ReturnTypeWillChange
attributes are not even required on PHP 8.1+ since the Stringable behavior
will automatically add the return type info if it's omitted. For PHP 8.0
and earlier, we map the ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX
macro to ZEND_BEGIN_ARG_INFO_EX -- so these methods will continue to have
no return types there (also avoiding a BC break).

I tested the change (i.e. tentative return type info for interfaces) with
PHP's master branch to cover the warning you added in [86379b6](
https://github.com/php/php-src/commit/86379b6710f972e0d4a11c89ce28d5768d9824d3).
There seems to be no conflict, which seems reasonable considering that
return type is only "tentative" from the context of userland (and only
until PHP 9.0, when it becomes enforced). Tentative return types are still
considered the actual return type for internal purposes and satisfies the
check for Stringable compatibility.

Are we likely to run into issues with this approach? If so, I'll consider
your last suggestion to define non-tentative return type info for
interfaces on PHP 8.0+ (where Stringable will add it to userland classes
automatically), and leave return type info absent for PHP 7.x. But if both
are viable solutions, I think I prefer the tentative return type info
approach.

-- 
jeremy mikola

--000000000000e1acc805d2a49b80--