On 29 March 2016 at 23:21, Nikita Popov <nikita....@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.
