__invoke() weirdness View as Markdown

Hello,

On Mon, Dec 29, 2008 at 6:00 AM, David Zülke
david.zuelke@bitextender.com wrote:

Hi David,

Am Dienstag, den 23.12.2008, 17:02 +0100 schrieb David Zülke:
[...]

This gives a fatal "Call to undefined method DateTime::getAtom()"
$d = new DateTime();
$d->getAtom = Curry::create(array($d, 'format'), DATE_ATOM);
echo $d->getAtom();

This is the same as the following:

$obj = new stdClass();
$obj->method = function() {
return "foo";
};
$obj->method();

The first "method" is a property, the call to "method" is - well - a
method call. Because PHP separates methods and properties in the class
entry structure - and because we have magic methods like __set, __get
and __call, there is no efficient and logical way how to search for
properties with closure instances when a method is not found. We
discussed that before and decided to let closures go out into the wild
and think about adding a solution for prototype alike inheritance later.

I realize that, but I guess it's pretty inconsistent, as I can do

• $func();
• $func->__invoke();
• $obj->func->__invoke();
  but not
• $obj->func();

That was my point; I'm aware of potential reasons for this behavior (like
what Stas pointed out). Sorry for the confusion ;)

Is it going to be like this forever? Stas said "there is no way to
distinguish". Isn't that a parser issue that should be gone now with the
switch to re2c? Wouldn't it be possible to just do a lookup for a property
that is an object with __invoke if the method was not found?

Well, this is not a parser problem (so re2c, which actually is the
lexer, won't help).
Additionally: looking for a property when a method is not found would
break code relying on __call() to be called on those cases.

Finally, note that you can do that in userland using something looking like:
public function __call($name, $args) { return
call_user_func_array(array($this->$name, "__invoke"), $args); }

Regards,

• David
  --

--
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal

Hello Lars,

Wednesday, December 31, 2008, 6:59:08 PM, you wrote:

Hi Markus,

have you measured the performance impact in a class with - say - ten
methods? And what to do with __get() and __call()? How are the
prioritized in the method resolve order?

Translated into user code we now have:

public function __zend_call($name, $args) {
// Added property lookup
if (isset($this->$name)) { // may call __isset
$callable = $this->$name; // may call __get
if (is_callable($callable)) {
return call_user_func_array($callable, $args);
}
}
// Original behavior:
// Check for __call
if (method_exists($this, '__call')) {
return $this->__call($name, $args);
}
// error
error_log('Function ' . CLASS . '::' . $name . ' not found');
return NULL;
}

As to the performance impact. We add one additional hash-lookup per
method call on a default class for a non found function. So whenever
we would normally call __call we add an additional lookup.

cu, Lars

Am Mittwoch, den 31.12.2008, 17:38 +0100 schrieb Marcus Boerger:

Hello David,

Tuesday, December 23, 2008, 5:02:43 PM, you wrote:

Hi folks,

I played with __invoke today:

class Curry
{
protected $callable;
protected $args;

public static function create($callable)
{
$curry = new self($callable, array_slice(func_get_args(), 1));
return $curry;
}

protected function __construct($callable, $args)
{
$this->callable = $callable;
$this->args = $args;
}

public function __invoke()
{
return call_user_func_array($this->callable, array_merge($this-

args, func_get_args()));
}
}

However, it doesn't work consistently.

This works fine:
$d = new DateTime();
$getAtom = Curry::create(array($d, 'format'), DATE_ATOM);
echo $getAtom();

This gives a fatal "Call to undefined method DateTime::getAtom()"
$d = new DateTime();
$d->getAtom = Curry::create(array($d, 'format'), DATE_ATOM);
echo $d->getAtom();

Is that intentional?

So far it is. Yet I as much as you do not like the inconsistency. So I
spend a little bit on providing the following patch that should do what
you were looking for.

The disadvantage: Calling properties is case sensitive while calling
methods isn't. But since this has nothign to do with this patch and the
patch only increases consistency I am all for applying it.

Comments? Lukas/Johannes?

Oh I hate that case insensitivity.... and inconsistency....

Cheers,

David

Best regards,
Marcus

Best regards,
Marcus

Hello Johannes,

Friday, January 2, 2009, 7:16:32 PM, you wrote:

Hi,

So far it is. Yet I as much as you do not like the inconsistency. So I
spend a little bit on providing the following patch that should do
what
you were looking for.

I thought bout this issue for one day or so, there are three things to
consider here:

• the language design question
• implementation-related things
• getting 5.3 out

For the first question I have to say that this is a major change to
language. Bringing this feature in makes it a mixture of a class-based
OO model and JavaScript-like class-less OO model. As I tend to read way
more code than I write I, personally, prefer the clearer class based
approach.

Nonetheless there seems to be large support for this, so let's lake a
deeper look:

The disadvantage: Calling properties is case sensitive while calling
methods isn't. But since this has nothign to do with this patch and
the patch only increases consistency I am all for applying it.

That's one of the symptoms of the underlying problem we're having: In
our object implementation methods and properties are kept completely
separated. Using properties now like methods will have side effects, one
is described above. I found another one today:

<?php
class A {
public $p;
public function __construct() {
$this->p = function() { echo 1; };
}
}

class B extends A {
private function p() {
echo 2;
}
}

$a = new A();
$b = new B();

$a->p();
$b->p();
?>>

What do you expect? Well, the first call, to $a->p(); works, the second
one, to $b->p();, doesn't since where accessing the private method
B::p().

$ sapi/cli/php test.php
1
Fatal error: Call to undefined method B::p() in test.php on line 16

Depending on your view on this that result can be correct or wrong, it
might even be wrong in different ways, maybe overwriting $a->p with
$b->p should be forbidden, maybe it should treat private elements like
non-existent ones.

The obvious thing is that it feels like the "is-a" relationship isn't
enforced anymore.

First, for properties we already have that anyway. Remember you can add
properties to object instances at any time. So being able to add methods
at any time just reduces one more inconsistency between things. Also
since properties follow a mixture of class based and prototype based
object model we have to respect prototype semantics for the parts where
proptotype model is used. This means when you dynamically add a closure
then this is done in the manner of prototype semantics and needs to follow
it. Thus with a slightly different code the result is perfectly correct:

<?php
class A {
// public $p;
}

class B extends A {
private function p() {
echo 2;
}
}

$a = new A();
$b = new B();

$a->p = function() { echo 1; };
$a->p();
$b->p();
?>

So this raises the question what the status quo without the patch is:
class Test {
public $func;
function __construct() {
$this->func = function() {
echo METHOD . "()\n";
};
}
private function func() {
echo METHOD . "()\n";
}
}

$o = new Test;
$f = $o->func;
$f(); // Test::{closure}()
$o->func(); // Error call to private method...

So apparently we already have the exact issue at hand and we take
precedence in the actual function which in my opinion is a good
choice. And this also is exactly the behavior your example with the
patch applies shows.

To summarize this from my point of view we only get a bit more of
consistency. But for people with a JavaScript background we get a
lot more. As we now support what they are used to and that actually
is a pretty big advantage.

However if that is the intention, then something we might want to look
into in order to make it easier for those people is to fix something
that propbably looks wrong to them. That is binding a closure to a
property outside of the scope of the object does not bind the object
to the this_ptr as it would in case the assignment is done inside the
object scope (a class method). But once again this is somethign to be
fixed whether or not we apply the patch.

Additionally there are other areas with such conflicts: Reflection
(ReflectionMethod doesn't work, ReflectionProperty has no hint it's
executable, ...) and other meta-functionality are obvious, others might
(and will) be quite hidden.

Reflection now has closure support.

I guess the only solid way to implement that feature would be by merging
the property and method tables into an "element" table so we reduce such
conflicts -- while that's a major engine and language change. Every
other approach will have side-effects.

That would mean a major change in language and we should not go that route.

This leads us to the third consideration: 5.3 is around 1.5 years in
development now with lots of new features. It was announced that we
wanted to go to a beta status to get it out soon ("release early,
release often" ...)

I'd say it would be good to concentrate on making 5.3 stable and then
see how new features are accepted. If users really demand such a
functionality, when using closures for "real life" development, we can
still add it, but that should be done with the time needed to identify
side-effects and other consequences, not in a rush during holiday season
after a feature freeze has been announced.

Stable also means stable features and semantics. And that includes trying
to get rid of inconsistencies.

marcus

Best regards,
Marcus