Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92017
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 65.55.169.122 as permitted sender)
To: Antony Dovgal <antony.dovgal@gmail.com>, Nikita Popov
	<nikita.ppv@gmail.com>
CC: PHP internals <internals@lists.php.net>
Thread-Topic: [PHP-DEV] Forbid binding methods to incompatible $this
Thread-Index: AQHRigl+6xTV22juY0yTf1tMtTF7859xuSXt
Date: Wed, 30 Mar 2016 09:34:31 +0000
Message-ID: <CY1PR0201MB1786E14B3CBEE673D5EF795BBF980@CY1PR0201MB1786.namprd02.prod.outlook.com>
References: <CAF+90c95i=u8rE_vA9WOuZ7VjSkWLxFPRoRykWAm2337mV=JeA@mail.gmail.com>
In-Reply-To: <CAF+90c95i=u8rE_vA9WOuZ7VjSkWLxFPRoRykWAm2337mV=JeA@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:23
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Mar 2016 09:34:31.7079
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 32210298-c08b-4829-8097-6b12c025a892
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0201MB1788
Subject: Re: [PHP-DEV] Forbid binding methods to incompatible $this
From: dmitry@zend.com (Dmitry Stogov)

I agree, this ability is a dirty and annoying hack, but I'm sure, some peop=
le use it.

Tony, you don't use this in the new runkit replacement? :)

Thanks. Dmitry.

________________________________________
From: Nikita Popov <nikita.ppv@gmail.com>
Sent: Wednesday, March 30, 2016 01:21
To: PHP internals
Subject: [PHP-DEV] Forbid binding methods to incompatible $this

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