Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105180
MIME-Version: 1.0
References: <CAF+90c_Xcz6rTq_CdAFdGBhW2eLDZOVEkLRgKgJZTwRQSsTTBA@mail.gmail.com>
 <3BCEA463-BD9B-468B-AF26-E0AD75C5F559@gmail.com> <CA+kxMuSyV1srU1RDuEMG=sNSA09MCMwdKohKE9pHWW7vVwDNJQ@mail.gmail.com>
 <16a029d2430.27c1.08be835b7d1a2c2edb4c4286afe1a236@gmail.com>
In-Reply-To: <16a029d2430.27c1.08be835b7d1a2c2edb4c4286afe1a236@gmail.com>
Date: Tue, 9 Apr 2019 09:35:24 -0600
Message-ID: <CAFMT4Nqt4NTM-o5GD3-M10jitxmxASdd9o9+zw2gsWTTtTc21w@mail.gmail.com>
To: Gabriel O <gadelat@gmail.com>
Cc: Dan Ackroyd <Danack@basereality.com>, PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] [RFC] Always generate fatal error for incompatible
 method signatures
From: levim@php.net (Levi Morrison)

On Tue, Apr 9, 2019 at 9:00 AM Gabriel O <gadelat@gmail.com> wrote:
>
> I believe rfc deals with reverse situation
>
> On 9 April 2019 4:47:50 PM Dan Ackroyd <Danack@basereality.com> wrote:
>
> > On Tue, 9 Apr 2019 at 11:58, Gabriel O <gadelat@gmail.com> wrote:
> >
> >> And this RFC conveniently shows only big LSP violation examples like array
> >> -> int, but not widely used narrowing like mixed/object -> specific instance.
> >
> >
> > Type narrowing or contravariant parameters is properly supported for
> > PHP 7.4:
> > https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters
> > , which is why this RFC doesn't need to cover them.
> >
> > cheers
> > Dan
> > Ack

If you want the reverse to be true, then your code has bugs waiting to
show themselves. The earlier we can catch these bugs, the better.

One thing to note is that return type information does permit
optimizations in some cases. For instance, I believe if a private
method has a return type constraint of int, then the optimizer can
rule out certain edge cases, and in some cases use type specialized
handlers. We can do these sorts of optimizations in more places if the
LSP issues are always errors, instead of warnings.

So for both correctness and performance, I support this change.