Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:91998
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.161.172 as permitted sender)
MIME-Version: 1.0
Date: Wed, 30 Mar 2016 00:21:46 +0200
Message-ID: <CAF+90c95i=u8rE_vA9WOuZ7VjSkWLxFPRoRykWAm2337mV=JeA@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=94eb2c07b5166b0abb052f37746b
Subject: Forbid binding methods to incompatible $this
From: nikita.ppv@gmail.com (Nikita Popov)

--94eb2c07b5166b0abb052f37746b
Content-Type: text/plain; charset=UTF-8

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

--94eb2c07b5166b0abb052f37746b--