Hi Marcus,

>> "Convoluted"? "Mess"? Are you kidding me? It's standard usage of access 
>> handlers.
> 
> It is a mess right now. You assign a closure to another method and get
> access to the original owners private members. That is not only unexpected
> and contradicting anything that any oyther language ever but but also
> violates our very own basic ideas. That is what I call a mess.

You could also call Javascript's behaviour confusing. A closure is per
definition a function that encloses its scope at creation time. E.g. if
you have the following (in Javascript, PHP, doesn't matter):

var foo;
var closure = function () {
        alert (foo);
};

The current scope variable foo is inherited. The question is: Why
shouldn't the same also happen for the variable this? In the closures
implementatios Dmitry and I designed for PHP, it does. Admittedly, $this
is a special variable because it's implicitly available in normal
methods and thus we decided that for closures you don't need to do "use
($this)" either.

So the question is: Why does Javascript change the pointer to the this
variable upon calling a method? The answer is simple: Because there is
NO OTHER WAY to define object methods in Javascript. You *always* have
to use object.method = function () { }; or Something.prototype.method =
... in order to define a callable method. There is no other way. Because
of that, Javascript defines the behaviour with $this.

PHP, on the other hand, since it already does have a method for creating
normal class methods (simply define them in the class { } block), does
not need such a mechanism for normal OOP.

Also, implementing this in PHP may give quite some headaches. Take for
example the following code:

interface Some_Filter {
  public function accept ($value);
}

class Closure_Filter implements Some_Filter {
  private $closure;
  public function __construct (Closure $closure) {
    $this->closure = $closure;
  }
  public function accept ($value) {
    // Or something similar, see below
    return call_user_func ($this->closure, $value);
  }
}

class Foo {
  private $min, $max;

  public function bar () {
    $filter = new Closure_Filter (function ($value) {
      return $value >= $this->min && $value <= $this->max;
    });
    $data = $something->doSomethingElse ($filter, $data);
  }
}

Now, basically, the idea behind this code should be clear: We want to
define a filter, there's an interface for that filter that any class may
define and there's a simple wrapper class for Closures for filters that
are supposed to be extremely simple. I don't think this example is
convoluted, one could easily imagine a similar design in the real word
(probably a bit more complex, but nevertheless).

The filter closure is now defined in the class Foo. Thus one would
assume that the closure is bound to the $this of the Foo object (once
created). But since that closure is passed to the constructor of the
Closure_Filter class, the $this would be rebound according to your
proposal. Thus, when invoking the method, the closure would now try to
access the ->min and ->max properties of Closure_Filter class - which is
clearly not the intention.

Of course, there are possibilites to circumvent that: 1) Copy the min
and max properties to local scope and bind them with use() to the
closure. Or 2) Don't store the closure directly inside a property of the
Closure_Filter class but in an array so $this doesn't get rebound. But
clearly, in my eyes, that is some kind of hackish workaround that really
sucks.

Also, with the implementation Dmitry and I wrote, it is very clear what
the semantics are $this: It is always bound at creation time and that's
it. Just to make a comparison:

// Variant 1:
return call_user_func ($this->closure, $value);
// Variant 2:
$closure = $this->closure;
return $closure ($value);
// Variant 3:
return $this->closure->__invoke ($value);
// Variant 4:
return $this->closure ($value);

Now, the original implementation:
  Variant 1: $this bound to Foo class
  Variant 2: $this bound to Foo class
  Variant 3: $this bound to Foo class
  Variant 4: doesn't work, because methods and properties have a
             different namespace

Now, an implementation where *all* four variants bind to the
Closure_Filter class:
  Variant 1: $this bound to Closure_Filter class
      -> Inconsistent: call_user_func ($this->normal_method) will first
                       cause "undefined property" and then "invalid
                       callback" errors
  Variant 2: $this bound to Closure_Filter class
      -> Hmm, so this basically allows for the following code:
               $closure = function ...;
               $object->closure = $closure; // MAGIC happens!
               $closure = $object->closure; // $closure changed - WTF?!
  Variant 3: $this bound to Closure_Filter class
      -> Ok, this really doesn't matter either way, using __invoke
         directly looks a bit weird anyway.
  Variant 4: $this bound to Closure_Filter class
      -> Calling properties directly will certainly cause resolution
         order headaches. Since that was the MAIN point on the list that
         was discussed before my posting you will have to admit it at
         least is not obvious what the resolution order should be.

Ok, you could say *ONLY* variant 4 should be added with dynamic and
temporary (i.e. call-time and for the duration of the call) rebinding of
the $this context. But that would also cause confusion: Why should $this
for the *same* property on some occasions a first object and on some
another? It doesn't make any sense!

Now, I would call this certainly a bigger mess than the inconsistency
wrt. Javascript. Since PHP is already different from Javascript (access
modifiers, "normal" object methods, different namespaces for methods and
properties), I don't really see the point in forcing something which
will create problems with the concepts PHP already has.

----------------------- snip ------------------

However, having said that, I may have a solution which will make
everybody (more or less at least) happy:

Since the main problem of re-binding closures to different objects is
the obscure magic just by assignment, why not make that magic "public"?

I.e.: Add a method bindTo() to the Closure class which returns an
identical copy of the closure (same bound variables etc.) with the
EXCEPTION that the new variables are already bound. This allows for
prototyping in the way that you want it BUT ensures that no silent magic
occurs that creates problems.

Example:
$func = function ($a, $b) {
  return $a + $b + $this->bias;
};
$object->add = $func->bindTo ($object);

Additionally, a static method bind() could be added for direct assignments:

$object->add = Closure::bind ($object, function ($a, $b) {
  return $a + $b + $this->bias;
});

The bindTo/bind functions could also check the current class scope and
ensure that the class scope of the newly bound closure has the same
access level to the object as the current class - that would enforce
access modifiers. Whether or not we want that is probably a matter of
debate.

Another idea which *could* be discussed (I'm not sure about it myself)
is that if you add variant 4 from above (i.e. calling object properties
like methods directly) - which you probably want to do if you want to
allow $this rebinding - and we have finally figured out some sane
resolution rules, one could also add a E_WARNING message when the $this
pointer of the closure does not match the $this pointer of the object
for which the closure is called as a method. So, for example:

$closure = function () { return $this->value; };
$object->get1 = $closure;
$object->get2 = $closure->bindTo ($object);

call_user_func ($object->get1);
call_user_func ($object->get2);
$f = $object->get1; $f ();
$f = $object->get2; $f ();
$object->get1->__invoke ();
$object->get2->__invoke ();
  // -> No warnings
  //       BECAUSE (!) this syntax makes it clear to the user that
  //       he is calling *some* closure, but not necessarily a closure
  //       bound to the specific object
$object->get1 ();
  // -> Warning: Closure called directly but object scope does
  //    not match object closure is being called for!
  // -> HOWEVER, execute the closure and DON'T rebind it
$object->get2 ();
  // -> No warning (match)

The check should of course be intelligent enough to detect the
following correctly:

$this->foo = function () { ... };
$this->foo ();
  // -> No warning (match, since assigned from inside the own object)

------------------ snip again --------------------

So, to sum up my posting:

1. Stance on direct method calling $foo->closure_property ()

       - As long as you figure out some sane and consistent resolution
         rules: Fine with me.

2. Stance on *automatic* re-binding of $this

       - Really big convoluted mess
       - PHP already does things differently from Javascript, has
         different concepts.
           => I'm against it.

3. Alternative proposal for making re-binding possible but making
   sure this is done explicitly:

       - Figure out some sane resolution rules
       - Closure::bind and Closure->bindTo

4. Additional proposal to 3:

       - Enforce class scope and thus access modifiers when using
         Closure::bind / Closure->bindTo

5. Additional propsoal to 3:

       - Warning when using direct-calling and the closure is not
         bound to the object for which the closure is called.

Regards,
Christian

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to