Re: [RFC] Closure::call and Function Referencing as Closures

Hi! Sorry for the slow response, I’ve been on holiday. On 8 Aug 2014, at 01:32, Josh Watzman <jwatzman@fb.com> wrote: > The RFC goes a long way to fixing this, but one important place it misses is with function > references to private and protected methods. The crux of the issue is that allowing an unbound > closure to escape can lead to very unexpected and unwanted results, as opposed to forcing the > closure to be bound to a particular variable. Consider: > > <?php > class C { > private function priv() { /* ... */ } > public function pub() { > // ... do stuff ... > // Return a closure for the caller to call priv at a later date: > return &self::priv; > } > } > > While this example is somewhat contrived, I've certainly wanted to do something similar > before. The intent of this code is for the closure returned by "pub" to only ever be > called on what was $this inside "pub" -- but not only can you change that with bind() on > the closure, "pub" itself cannot even pre-bind it to the intended $this. Or maybe it > technically could by calling bind() on the result of &self::priv before returning it, but > that's very cumbersome for what is the common case for what you want to do with "function > pointer" to a non-static method. Unfortunately, yes, such closures are not pre-bound. However invoking it when it’s unbound will produce the same error as calling any method statically (E_STRICT or E_ERROR), so the issue can be noticed and fixed easily. It’s possible to bind with Closure::bind(&Foo::bar, ‘Foo’); or something like that. I could make it pre-bind, but I’m unwilling to as :: never binds when calling, so it shouldn’t when referencing for consistency. > However, importantly, the Hack typechecker *does* constrain a lot of this messiness, and so the > underlying implementation isn't nearly as important for Hack right now. But straightening this > out in the runtime would be a big win of having real first class function references, and I think > your RFC is missing out on making the semantics here a whole lot more in line with what programmers > are going to expect. I think &Foo::bar does what you’d expect IMO, as just like Foo::bar(), it’s unbound, even for self. It’s unfortunate, but it is very simple to simply bind it. > Orthogonal to the above issue, something else you may want to consider, since you're > revisiting first class functions, is using Closure for this might not be what you want. (This might > be partially what Stas was getting at as well?) If you're thinking about these as real > first-class functions, the fact that they are objects, that you can add dynamic properties or > re-bind() them is kind of weird. It makes sense for methods. If you reference the method itself, you’d need to bind it to call it. It makes less sense for global functions, but they’re static anyway, you can’t bind them. Using the ‘Closure’ class is unfortunate, but I don’t really want to make unnecessary new Function/Method/etc. classes given they’d all share the same implementation anyway. Maybe, should this pass, we could rename Closure to Function or something. > You can't do any of those things to the "real" function, so it's unclear if > it makes sense to be able to do it to the callable reference. Well, closures are immutable, to be fair. bind and bindTo actually produce *new* functions, and they don’t even work on static closures anyway. You’re not really doing these things to the “real” function. -- Andrea Faulds http://ajf.me/