Re: svn: /php/php-src/ branches/PHP_5_4/Zend/zend_compile.c trunk/Zend/zend_compile.c

On 18 Nov 2011, at 15:36, Felipe Pena wrote: > 'Be generous with whitespace' is the opposite of what you want to do. > I.e. generous != stingy. :P I was obviously to imprecise in my request: Index: Zend/zend_compile.c =================================================================== --- Zend/zend_compile.c (revision 319492) +++ Zend/zend_compile.c (working copy) @@ -3815,30 +3815,45 @@ static void zend_add_magic_methods(zend_class_entry* ce, const char* mname, uint mname_len, zend_function* fe TSRMLS_DC) /* {{{ */ { - if (!strncmp(mname, ZEND_CLONE_FUNC_NAME, mname_len)) { - ce->clone = fe; fe->common.fn_flags |= ZEND_ACC_CLONE; - } else if (!strncmp(mname, ZEND_CONSTRUCTOR_FUNC_NAME, mname_len)) { + if (!strncmp(mname, ZEND_CLONE_FUNC_NAME, mname_len)) { + fe->common.fn_flags |= ZEND_ACC_CLONE; + ce->clone = fe; + } + else if (!strncmp(mname, ZEND_CONSTRUCTOR_FUNC_NAME, mname_len)) { + fe->common.fn_flags |= ZEND_ACC_CTOR; if (ce->constructor) { - zend_error(E_COMPILE_ERROR, "%s has colliding constructor definitions coming from traits", ce->name); + zend_error(E_COMPILE_ERROR, + "%s has colliding constructor definitions coming from traits", + ce->name); } - ce->constructor = fe; fe->common.fn_flags |= ZEND_ACC_CTOR; - } else if (!strncmp(mname, ZEND_DESTRUCTOR_FUNC_NAME, mname_len)) { - ce->destructor = fe; fe->common.fn_flags |= ZEND_ACC_DTOR; - } else if (!strncmp(mname, ZEND_GET_FUNC_NAME, mname_len)) { - ce->__get = fe; - } else if (!strncmp(mname, ZEND_SET_FUNC_NAME, mname_len)) { - ce->__set = fe; - } else if (!strncmp(mname, ZEND_CALL_FUNC_NAME, mname_len)) { - ce->__call = fe; - } else if (!strncmp(mname, ZEND_UNSET_FUNC_NAME, mname_len)) { - ce->__unset = fe; - } else if (!strncmp(mname, ZEND_ISSET_FUNC_NAME, mname_len)) { - ce->__isset = fe; - } else if (!strncmp(mname, ZEND_CALLSTATIC_FUNC_NAME, mname_len)) { - ce->__callstatic = fe; - } else if (!strncmp(mname, ZEND_TOSTRING_FUNC_NAME, mname_len)) { - ce->__tostring = fe; - } else if (ce->name_length + 1 == mname_len) { + ce->constructor = fe; + } + else if (!strncmp(mname, ZEND_DESTRUCTOR_FUNC_NAME, mname_len)) { + fe->common.fn_flags |= ZEND_ACC_DTOR; + ce->destructor = fe; + } + else if (!strncmp(mname, ZEND_GET_FUNC_NAME, mname_len)) { + ce->__get = fe; + } + else if (!strncmp(mname, ZEND_SET_FUNC_NAME, mname_len)) { + ce->__set = fe; + } + else if (!strncmp(mname, ZEND_CALL_FUNC_NAME, mname_len)) { + ce->__call = fe; + } + else if (!strncmp(mname, ZEND_UNSET_FUNC_NAME, mname_len)) { + ce->__unset = fe; + } + else if (!strncmp(mname, ZEND_ISSET_FUNC_NAME, mname_len)) { + ce->__isset = fe; + } + else if (!strncmp(mname, ZEND_CALLSTATIC_FUNC_NAME, mname_len)) { + ce->__callstatic= fe; + } + else if (!strncmp(mname, ZEND_TOSTRING_FUNC_NAME, mname_len)) { + ce->__tostring = fe; + } + else if (ce->name_length + 1 == mname_len) { char *lowercase_name = emalloc(ce->name_length + 1); zend_str_tolower_copy(lowercase_name, ce->name, ce->name_length); lowercase_name = (char*)zend_new_interned_string(lowercase_name, ce->name_length + 1, 1 TSRMLS_CC); The only thing you can argue about that is, that else is not on the same line as the closing brace of the if. However, you will see, if you remove it, that it becomes hard to parse the code. At least for me. So I would put in the break to emphasize the else if condition. Anyway, that is what I mean with tabling the related parts. I don't see how that violates the style guide, but IMHO, it makes quite a difference when it comes to readability of such involved if/else if/else if/else if/else if/else if/else if/else if/else if/else if constructs. Best regards Stefan -- Stefan Marr Software Languages Lab Vrije Universiteit Brussel Pleinlaan 2 / B-1050 Brussels / Belgium http://soft.vub.ac.be/~smarr Phone: +32 2 629 2974 Fax: +32 2 629 3525