Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92150
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.161.177 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAF+90c95i=u8rE_vA9WOuZ7VjSkWLxFPRoRykWAm2337mV=JeA@mail.gmail.com>
References: <CAF+90c95i=u8rE_vA9WOuZ7VjSkWLxFPRoRykWAm2337mV=JeA@mail.gmail.com>
Date: Fri, 8 Apr 2016 07:42:33 +0900
Message-ID: <CAF+90c9Th=bnmu=NcVOcGsh6k_cEhhGs4eZr7_7RtP7JfZiu7g@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1141e7f44c2a3d052feccbb5
Subject: Re: Forbid binding methods to incompatible $this
From: nikita.ppv@gmail.com (Nikita Popov)

--001a1141e7f44c2a3d052feccbb5
Content-Type: text/plain; charset=UTF-8

On Wed, Mar 30, 2016 at 7:21 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> Hi internals!
>
> Currently, inside instance methods the following invariant holds:
>
>     assert(is_null($this) || is_object($this))
>
> This is a problem. I'd like to guarantee the following invariant instead:
>
>     assert(is_null($this) || $this instanceof self)
>
> That is, ensure that if $this is not null, then it must be an object
> "compatible" with the class, i.e. be in its inheritance hierarchy.
>
> The absence of this guarantee, apart from being a major WTF in itself,
> also prevents us from optimizing operations involving $this. In particular,
> the following optimizations are not possible:
>
> a) If typed properties land, we will not be able to use the type of
> $this->typedProperty for optimization, because $this->typedProperty might
> actually be referencing a property of a totally unrelated class.
> b) We are not able to early-bind arguments to private or final method
> calls, i.e. determine at compile-time whether an argument will use by-value
> or by-reference argument passing and optimize accordingly.
> c) We are not able to inline calls to private or final methods. (The
> possibility of null is an issue in this instance as well, though a lesser
> one.)
>
> The good news is that, as of PHP 7.0, we are *very* close to guaranteeing
> that "$this instanceof self" already. There exists only one little known
> loophole: Obtaining a closure object wrapping the method using
> ReflectionMethod::getClosure() and binding to an unrelated $this using
> Closure::bindTo().
>
> In PHP 7.0 we already heavily cut down [1] on what bindTo() is to allowed
> to do on fake method closures. Namely, it was completely forbidden to
> rebind the scope of methods, as this was already interacting badly with
> optimizations in 7.0. We did not forbid binding to an incompatible $this
> because it was not yet causing immediate issues at the time.
>
> I'd like to remedy this oversight now and restrict $this binding of fake
> method closures to compatible contexts only, i.e. for a method A::foo()
> allow only instances of A or one of its descendant classes to be bound.
> This limitation already exists for fake method closures targeting internal
> methods.
>
> Are there any objections to this change? If not, I'll merge the patch [2].
> If yes, I'll write an RFC, as this change, while of no direct consequence
> for PHP programmers, is important for the PHP implementation.
>
> Regards,
> Nikita
>
> [1]: http://markmail.org/message/sjihplebuwsmdwex
> [2]:
> https://github.com/php/php-src/compare/master...nikic:forbigIncompatThis
>

As the discussion was positive, this is now merged as
https://github.com/php/php-src/commit/75af8150f58fb55637ac12b33d469b27adef9d76
.

Nikita

--001a1141e7f44c2a3d052feccbb5--