Hi,

On Thu, 2008-07-03 at 22:30 +0400, Dmitry Stogov wrote:
> I don't see big problems with closures. The patch is simple and
stable.
> It's main part isolated in zend_closures.c and it doesn't affect other
> parts of engine.

Changes too the languages always introduce some side effects and
therefore we have too be careful there. Even stuff which is from engine
point of view nice might have some effects on the userland which might
not be too nice, just see the ongoing discussions about details about
namespaces... But back to closures:

I spent the last weeks with less mail reading and am still 112 mails on
internals behind so maybe I missed something there but even short tests
showed me things which might need cleanup:

A tiny thing I saw is different naming in different places:

php -r 'is_callable((function () {}), true, $name); var_dump($name);'
string(6) "lambda"

vs.

php -r 'var_dump(function(){});'
object(Closure)#1 (0) {
}

Do we really want to use both names in userland? Is a string "lamda"
really the best represantation for callable_name? (this might even be an
issue if somwbody has a function called lambda ... for whatever reason,
oh just checked people even do:
http://google.com/codesearch?hl=en&lr=&q=lang%3Aphp+%22Function+lambda(%
22&sbtn=Search )

Another issue I found during my short testing period today is
reflection: In a previous position I had to deal with callbacks, for
making the code more robust I checked the callback using reflection
before accepting it, the code looks something like that:

   function setCallback($cb) {
       if (is_array($cb) {
           $r = new ReflectionMethod($cb);
       } elseif (is_String($cb)) {
           $r = new ReflectionFunction($cb);
       } else {
           throw new Exception();
       }

       if ($r->getNumberOfRequiredParameters() != 2) {
           throw new Exception();
       }
       /* ...  morre checks of that kind ... */
    }

Now such a check isn't possible, all reflection information on the
callback I can get is that it is an object of type Closure having a
method __invoke() with zero required parameters. There's no further
information on the function, so probably we need a ReflectionClosure
class or something

Both of them are tiny issues which can be fixed in a quite simple way
(or be documented as expected) but my concern is that we'll find way
more of them.

So the final question is: Do we want to go in a higher pace to get
things out and have other features ready which can be added early for a
new release cycle or do we want to go a bit slower with the risk of
stopping and probably going backwards, again.

I'd prefer the first way (even so I do really like closures and wanted
to have them for some loooong time...)

> I expect more problems with GC

GC can be problematic yes, but imo gc can either be disabled by default
and documented as "use only with care for  running your unit test suite"
or completely dropped, even in a later stage if there are problems, I
don't see there much potential for long going detail discussions. Even
if it's, technically, more critical.

johannes


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

Reply via email to