Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92015
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.178 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: Wed, 30 Mar 2016 10:29:04 +0100
Message-ID: <CABHuAWwFYOya0yqzMqOKwpxDp4UoR4Xi5QVByrGO8N2t8y2hKQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11c3c6d2e75535052f40c6a8
Subject: Re: [PHP-DEV] Forbid binding methods to incompatible $this
From: t.carnage@gmail.com (Chris Riley)

--001a11c3c6d2e75535052f40c6a8
Content-Type: text/plain; charset=UTF-8

On 29 March 2016 at 23:21, 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
>


Hi,

Will this patch break any use cases similar to:

class Bar {
private $a = 'hidden';
}

$x = new Bar();
$y = function() { return $this->a);
$z = $x->bindTo($x, $x);
echo $z(); //hidden

?

If not, I have no objections.

--001a11c3c6d2e75535052f40c6a8--