Re: PATCH: Implementing closures in PHP

Hi! Thanks for reading through! > 1. There appear to be some spurious whitespace insertions in this > version of the patch. Oh, that's probably my editor, I'll fix that. > 2. The terms "lamba" and "anonymous function" are being used > interchangeably. If we're going to introduce new terminology, it > would be good to pick one name and use it consistently. I don't > have a preference for which one is ultimately chosen. Well, create_function uses an already-existing EG(lambda_count) and names the function __lambda_$counter so I thought I'd use CG(compiled_lambda_count) and name them __compiled_lambda_... But since anonymous functions aren't REAL lambdas, I named them anonymous elsewhere. But you're right, introducing duplicate terminology is a bad idea, I'll change everything to lambda for consistency, even though it's technically not 100% correct. > The term "lexical" could also be considered a competing term as > its used in part of the patch. 'lexical' is only used for the variables that are passed into the closure, not for the closure itself. > 3. The "is_anonymous" flags could be zend_bool values instead of bare > integers, although that breaks the precedent started by some > related flags (such as "is_method"). You're right, zend_bool is a better idea. Since PHP 5.3 is going to break binary compability anyway, would it do any harm changing the types of the existing flags, too? > 4. This part of the zend_vm_def.h diff looks wrong (a stray "f"): > > -/* > +f/* WTF? I thought I had already fixed that. Hmm, obviously I hadn't... > Looks great overall! Thanks! Merry Christmas, Christian