Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:106547
MIME-Version: 1.0
References: <CAF+90c99ftVY4KndJAH_851sEAF===4iyUgSqrc3nEsL833BhQ@mail.gmail.com>
 <CAESVnVqvSt_U8g=cwRu0pfEVrByYVmec_LHDa8jHtem95E=i9g@mail.gmail.com>
 <CAF+90c98JVkbOO7wrD5=ESDeD0wC8ChWo4WvC0by+MMjiMry3g@mail.gmail.com>
 <CAESVnVrd7zmKr=CAOw0OQ1ojDCOCFAnnYbo-AMD8qtcWSZevAg@mail.gmail.com> <CAF+90c-kYuR0VE7FBZLhOfPTE4Yd-g2QS=W8+B0uXbaPJ6Twiw@mail.gmail.com>
In-Reply-To: <CAF+90c-kYuR0VE7FBZLhOfPTE4Yd-g2QS=W8+B0uXbaPJ6Twiw@mail.gmail.com>
Date: Sun, 11 Aug 2019 16:29:41 +0200
Message-ID: <CAOWwgpk_6G2s0-XrAh2UQfE-Een5oxNjikjYgdBjxV1nHsZRLg@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Sara Golemon <pollita@php.net>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000009d833b058fd83d6a"
Subject: Re: [PHP-DEV] Adding return types to internal methods (PHP 8)
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--0000000000009d833b058fd83d6a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Le dim. 11 ao=C3=BBt 2019 =C3=A0 15:24, Nikita Popov <nikita.ppv@gmail.com>=
 a =C3=A9crit :

> On Sun, Aug 11, 2019 at 3:01 PM Sara Golemon <pollita@php.net> wrote:
>
> > On Sun, Aug 11, 2019 at 7:51 AM Nikita Popov <nikita.ppv@gmail.com>
> wrote:
> >
> >> On Sun, Aug 11, 2019 at 2:37 PM Sara Golemon <pollita@php.net> wrote:
> >> > P.S. - Perhaps a way to the middle might be to introduce implicit
> return
> >> > type hints.  If a child method is implemented with no return type
> >> specified
> >> > on a parent method which has one, then the parent's type is assumed =
at
> >> bind
> >> > time, then we provide a better runtime error if that assumption is
> >> violated
> >> > in a strict_types=3D1 context which is the only place where strict
> >> covariance
> >> > of return types matters to PHP.
> >> >
> >>
> >> This is an interesting idea. There's still a BC break here, but it's
> more
> >> limited and only applies to code that is actively violating type
> contracts
> >> (that were previously implicit). Imho this should not be bound to
> >> strict_types though: strict_types controls coercion behavior of scalar
> >> type
> >> annotations, and nothing else. It should not be overloaded with this
> >> additional meaning (and I don't think the BC break is nowhere near lar=
ge
> >> enough to justify this overloading).
> >>
> >> Fair.  Just trying to tread as light as possible, but there is a
> > vanishing point.
> >
> >
> >> Do you have this in mind as something for use by internal classes only=
,
> or
> >> as a general language feature? I can see the use as a general language
> >> feature (see for example the debacle where PHPUnit added "void"
> >> annotations
> >> to some methods). The migration problem of adding return types is
> >> certainly
> >> not limited to internal classes. On the other hand it does seem somewh=
at
> >> weird that an (inherited) return type will be enforced that was not
> >> explicitly given in the source.
> >>
> >> I agree that it would be as useful to userspace classes as internal
> > ones.  It would allow library authors to embrace strong typing without
> > breaking consumers who are already conforming to docblock types.
> >
> > It's probably worth proof-of-concepting this and making an RFC
> > irrespective of the push to annotate all the things.  If feasible and
> > accepted, I'd be much more on board with including return types for
> > internal methods in that plan however.
> >
>
> After thinking about this a bit, this is going to be pretty hard to
> implement. Currently return type checks are performed using a separate
> opcode, which means that we need to know the return type at compile-time.
> At the point where inheritance happens and we know the parent return type=
,
> we are no longer able to modify the opcodes of the function. We'd have to
> integrate the return type check into the main return logic, with a possib=
le
> negative impact on performance...
>

Coincidentally, we're working on exactly this in SYmfony: we want to add
return-type declarations in v5 (due in November) and we want to provide a
continuous upgrade path so that apps that are using v4.4 are warned with a
deprecation asking them to add the return type.

We're doing this using `@return` annotation in the docblocks: when one is
found in base Symfony class, child classes that don't declare their return
type get the notice.

Because we don't want Symfony to trigger the deprecation when extending
itself, we silence it when a child class has an explicit `@return`
annotation. This is the way to opt-out from the notice, while telling our
userland what they should do, before we do.

The related PR is here if you want to have a look:
https://github.com/symfony/symfony/pull/30323

I don't know if you'll find applicable, but an idea for PHP itself could be
that internal classes expose the planned return-type via a docblock,
accessible by `ReflectionMethod::getDocComment()`. This way we could build
userland tools to leverage it and trigger deprecations.

In general on this thread, I agree with Nikita it would be desireable to
add return types, and I also agree with Sara that it should not be a hard
BC break.

I hope this helped a bit :)

NIcolas

--0000000000009d833b058fd83d6a--