Hello, internals!
Just noticed this thread and want to clarify some things with getClosure()
method. If i understood correctly, ReflectionFunctionAbstract::getClosure()
should return non-rebindable closure ONLY for internal functions, but for
userland functions binding should be still possible, right?
E.g.
class Foo {private $a=1;}
function test() {
var_dump($this);
}
$closure = (new \ReflectionFunction('test'))->getClosure();
$object = new Foo;
$closure = $closure->bindTo($object, get_class($object));
$closure();
(unfortunately, it's broken: https://3v4l.org/YeDBj#v700rc5)
I know, that binding functions to objects is tricky, but this can be useful
for utility classes. But if you insist on this, ok, it's very rare case,
just one single example.
What is more important for me is ability to work with methods as closures.
So my main concern is about restrictions of ReflectionMethod->getClosure()
and rebinding.
See this example:
class Foo {private $a=1;}
class Bar {
function test() {
var_dump($this);
}
}
$closure = (new \ReflectionMethod('Bar','test'))->getClosure(new Bar);
$object = new Foo;
$closure = $closure->bindTo($object, get_class($object));
$closure();
This kind of rebinding is heavily used by Go! Aspect-Oriented Framework and
now it will be broken (see https://3v4l.org/cZtbI#v700rc5). I think there
will be some more frameworks/libraries that use reflection and closure
rebinding via reflections. So this introduced patch for RC5 is definitely a
BC break for existing code.
Could you please provide a workaround or alternative ways to keep it
working? Maybe, restrict rebinding only for internal objects/methods will
be enough? Or only for ReflectionFunctionAbstract->getClosure(), but not
for ReflectionMethod->getClosure()
Thanks!
2015-10-13 10:27 GMT+03:00 Anatol Belski <anatol....@belski.net>:
> Hi,
>
> > -----Original Message-----
> > From: Xinchen Hui [mailto:larue...@php.net]
> > Sent: Tuesday, October 13, 2015 4:23 AM
> > To: Anatol Belski <anatol....@belski.net>
> > Cc: Nikita Popov <nikita....@gmail.com>; Dmitry Stogov <dmi...@zend.com
> >;
> > PHP internals <internals@lists.php.net>
> > Subject: Re: [PHP-DEV] Re: Forbid rebinding scope of closures created by
> > ReflectionFunctionAbstract::getClosure()
> >
> > Hey:
> >
> > On Tue, Oct 13, 2015 at 6:12 AM, Anatol Belski <anatol....@belski.net>
> > wrote:
> >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Nikita Popov [mailto:nikita....@gmail.com]
> > > > Sent: Monday, October 12, 2015 10:33 PM
> > > > To: Anatol Belski <anatol....@belski.net>
> > > > Cc: Dmitry Stogov <dmi...@zend.com>; PHP internals <
> > > internals@lists.php.net>
> > > > Subject: Re: [PHP-DEV] Re: Forbid rebinding scope of closures
> > > > created by
> > > > ReflectionFunctionAbstract::getClosure()
> > > >
> > > > On Mon, Oct 12, 2015 at 10:22 PM, Anatol Belski
> > > > <anatol....@belski.net>
> > > > wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Nikita Popov [mailto:nikita....@gmail.com]
> > > > > > Sent: Monday, October 12, 2015 8:57 PM
> > > > > > To: Dmitry Stogov <dmi...@zend.com>
> > > > > > Cc: PHP internals <internals@lists.php.net>; Andrea Faulds
> > > > > > <a...@ajf.me>;
> > > > > Stas
> > > > > > Malyshev <smalys...@sugarcrm.com>; Bob Weinand
> > <bwo...@php.net>;
> > > > > > Anatol Belski <anatol....@belski.net>
> > > > > > Subject: [PHP-DEV] Re: Forbid rebinding scope of closures
> > > > > > created by
> > > > > > ReflectionFunctionAbstract::getClosure()
> > > > > >
> > > > > > > It would be great, if we stop any commits into PHP-7.0 except
> > > > > > > for
> > > > > > critical fixes now
> > > > > >
> > > > > > Maybe keep PHP-7.0 open (or as open as release branches usually
> > > > > > are),
> > > > > but from
> > > > > > now on only cherry-pick critical fixes into PHP-7.0.0 (instead
> > > > > > of merging everything)?
> > > > > >
> > > > > I commit myself to Dmitry's words. What matters today and
> > > > > especially after
> > > > > RC5 is the stability. Today we should invest into testing and bug
> > > > > fixes more than into improvements (aka fixes to something that is
> > > > > not broken). It really matters for the quality of the final.
> > > > > That's the message to convey probably.
> > > > >
> > > >
> > > > To rephrase my question: Should we treat PHP-7.0 the same way we
> > > > treat
> > > > PHP-5.6 and other release branches (i.e. all kinds of bug fixes are
> > > okay, even if
> > > > it's just improving a test or fixing some inconsequential behavior),
> > > > or
> > > do you
> > > > want to limit the PHP-7.0 branch to actually critical fixes now?
> > > > From
> > > what you
> > > > say, I assume the former rather than the latter?
> > > >
> > > Talking about "critical" I meant usual definitions as something that
> > > causes memory corruptions, data loss, big functional impact, security
> > > flaws, etc.
> > >
> > agree, bugs
> >
> > >
> > > The tricky point with 7.0 right now is that it's a lot of new stuff
> > > with very high expectations standing short before final. Coupling this
> > > with the "critical" from above, the definition I would make were - it
> > > is a) critical and b) can be fixed properly and cannot wait until
> after the final
> > release.
> > > The dev time spent to fix something after 7.0 final is indeed lost for
> > > the
> > > 7.0 final. Any new code short before final increases the instability
> > > risk of the final.
> > >
> > > Fe small functional regressions are probably not always critical. If a
> > > (even small) functional regression breaks a lot, it is critical. If a
> > > functional regression breaks a rare use case - it is not critical. If
> > > improving a test helps to cover some critical code - so yes, it is
> > > critical as well. Anything that is critical can involve anything
> > > you've mentioned like adding tests or code.
> > >
> > hmm, I understand your ideas, but I think maybe we should forbid such
> things as
> > well for this moment, because it make things complicated to judge(if it
> is critical
> > or not), such things can goes to 7.1
> >
> > let's only allows bugs fix (as you mentioned above) to be committed into
> > PHP-7.0 before the final release of its
> >
> Yeah, such things are quite stretchable, as we don't really assign tickets
> to milestones. Concentrating on hard weight things only were probably the
> best strategy, as you say. I'll be mentioning this yet in the next
> announcement as well.
>
> Regards
>
> anatol
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
