Re: [PHP-DEV] [Windows] Error Building PHP 5.4 NTS Debug Version, mysqlnd
Hi Pierre, Can reproduce it now, can you open a bug please and assign it to mysql? Done: https://bugs.php.net/bug.php?id=60863 Thanks, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [Windows] Error Building PHP 5.4 NTS Debug Version, mysqlnd
Hi Pierre, I cannot reproduce the error, mysqlnd builds just fine here. What's your configure line? cscript /nologo configure.js --disable-phar --disable-ipv6 --disable-zts --enable-cgi --disable-bcmath --disable-calendar --disable-odbc --disable-tokenizer --disable-xmlreader --disable-xmlwriter --without-wddx --enable-debug --enable-cli-win32 --enable-pdo --with-openssl --with-php-build=..\deps --with-libxml --with-pdo-sqlite This is done in a Windows SDK 6.1 shell with setenv /debug /x86 /xp [*] and VC9 Pro SP1 is used. The same configure line works fine for PHP 5.3.9. Thanks, Christian [*] But also setenv /release /x86 /xp has the same issue. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [Windows] Error Building PHP 5.4 NTS Debug Version, mysqlnd
Hi, I was compiling PHP 5.4 SVN for Windows and noticed that the NTS Debug version does not compile correctly if mysqlnd is enabled. The ZTS Debug, ZTS Release and NTS Release versions work without any problems. Also, PHP 5.3 in all 4 variants (Debug/Release, NTS/ZTS) has no problems. I've attached the error messages I got as a text file, I apologize in advance for the German error messages, the Visual Studio version I have I have no idea how to make it output English messages. (There appears to be no LC_ALL=C in Windows.) However, it should be easy to search for the corresponding error message IDs if you have trouble understanding the messages. To summarize the error, it chokes on the following statements: mysqlnd_driver.c, 100: DBG_ENTER(mysqlnd_error_list_pdtor); mysqlnd_driver.c, 104: DBG_VOID_RETURN; Since I have no idea whatsoever about the internals of mysqlnd, I'm posting this to the list. Regards, Christian d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2143: Syntaxfehler: Es fehlt ';' vor 'Typ' d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2275: 'uint64_t': Ungültige Verwendung dieses Typs als Ausdruck d:\php-sdk\php54dev\vc9\x86\php5.4-svn\win32\php_stdint.h(87): Siehe Deklaration von 'uint64_t' d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2146: Syntaxfehler: Fehlendes ';' vor Bezeichner '__dbg_prof_start' d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2065: '__dbg_prof_start': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2275: 'zend_bool': Ungültige Verwendung dieses Typs als Ausdruck d:\php-sdk\php54dev\vc9\x86\php5.4-svn\zend\zend_types.h(25): Siehe Deklaration von 'zend_bool' d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2146: Syntaxfehler: Fehlendes ';' vor Bezeichner 'dbg_skip_trace' d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2065: 'dbg_skip_trace': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2065: 'dbg_skip_trace': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2065: '__dbg_prof_tp': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : warning C4133: 'Funktion': Inkompatible Typen - von 'int *' zu 'timeval *' d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2065: '__dbg_prof_start': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2065: '__dbg_prof_tp': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2224: Der linke Teil von '.tv_sec' muss eine Struktur/Union sein d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2065: '__dbg_prof_tp': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(100) : error C2224: Der linke Teil von '.tv_usec' muss eine Struktur/Union sein d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(104) : error C2065: '__dbg_prof_tp': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(104) : warning C4133: 'Funktion': Inkompatible Typen - von 'int *' zu 'timeval *' d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(104) : error C2065: '__dbg_prof_tp': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(104) : error C2224: Der linke Teil von '.tv_sec' muss eine Struktur/Union sein d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(104) : error C2065: '__dbg_prof_tp': nichtdeklarierter Bezeichner d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(104) : error C2224: Der linke Teil von '.tv_usec' muss eine Struktur/Union sein d:\php-sdk\php54dev\vc9\x86\php5.4-svn\ext\mysqlnd\mysqlnd_driver.c(104) : error C2065: '__dbg_prof_start': nichtdeklarierter Bezeichner -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Closures and $this: Please vote!
Hi, My suggestion is to wait until the 15th of January (that's one month since I started this thread) and that should have been enough time of everybody. So, it's 18th - are we moving forward with this? Yes, Sorry, I've been extremely busy for the last two months, I'll get back to this in the next few days. Since my last posting in mid-December nobody else commented on this. My overview back then still holds. The majority of people (internals@ and otherwise) was in favor of (A+), so I'll implement that and put details of the implementation and the discussion into the Wiki. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Execution point of session save handler on shutdown
Hello, [CC to pecl-dev since this is APC-related.] I have been investigating a problem occuring when using APC in combination with userspace session save handlers. This has been reported multiple times both in the APC and PHP bugtrackers, bugs related to this issue include (but are most likely not limited to): http://bugs.php.net/bug.php?id=48787 http://bugs.php.net/bug.php?id=48686 http://pecl.php.net/bugs/bug.php?id=16721 http://pecl.php.net/bugs/bug.php?id=16745 Using a debugger I have found the source of the problem. In my eyes, this is a design problem in PHP's session extension. Rationale: Currently, the session module calls php_session_flush() in its RSHUTDOWN function. This makes sure that the session handler is always called at the very end of execution. However, APC uses its RSHUTDOWN function to remove all class entries from the class table, since they are only shallow copies of memory structures of its cache and the Zend engine should not destroy the class entries by itself. The problem now occurs when the session save handler uses non-internal classes in some way. Since APC, as a dynamic extension, is always loaded AFTER the session module (always a static extension), and the module de-initialization is in reverse order, APC will unload all internal classes *before* the session save handler is executed. So when it is finally the turn of the the session save handler, the classes will no longer be present in the class table, causing the problems described above. Note that APC is not the only extension for which there is a problem with regards to de-initialization. The Spidermonkey extension destroys the current Javascript execution context in the RSHUTDOWN function, so if should any save handler use the Spidermonkey extension, it will not work. In theory, even the date extension could cause minor problems (since the default timezone is reset in RSHUTDOWN), but it gets away since it is always loaded *before* session and thus de-inititialized afterwards. I consider this to be a design problem in PHP's session extension. It tries to use the PHP engine to execute PHP code (the custom session save handler, possibly also the serialization handlers of objects etc.) during a phase where at least some other extension that may be used in that code are already de-initialized. The only reason why it got unnoticed so long is that the RSHUTDOWN code of most - but not all - extensions doesn't really have any really nasty side-effects on its use. It would not surprise me at all, however, if there were memory leaks and other strange this occurring due to this problem. My immediate fix for this problem is to make php_request_shutdown() execute the session save handler just after the userspace shutdown functions but before the RSHUTDOWN functions of the other modules. I have attached short patches to this mail for PHP 5.2, 5.3 and 6.0 that accomplish this. It's not very elegant but it is the least-invasive fix I could come up with. Not existing tests are broken in either of the three branches. Unfortunately, I was not able to create a unit test for this using core PHP alone, since all of the core extensions hide the problem. I wanted to get some feedback first. If nobody objects, however, I'll apply these patches to PHP 5.3 and PHP 6 tomorrow. As for PHP 5.2: I was extremely busy the last two months so I didn't really follow the list. Is it okay if I apply it? I do think this patch should go into the last release of PHP 5.2 (which will be used by quite a few people for a while) since this bug prevents people from using a custom session save handler in combination with APC. On a side note: For PHP6 we may think of adding an additional hook for modules that is executed at the very beginning of the shutdown phase for precisely this purpose. Regards, Christian Index: ext/session/session.c === --- ext/session/session.c (revision 295251) +++ ext/session/session.c (working copy) @@ -36,6 +36,7 @@ #include php_ini.h #include SAPI.h +#include php_main.h #include php_session.h #include ext/standard/md5.h #include ext/standard/sha1.h @@ -2012,7 +2013,9 @@ static PHP_RINIT_FUNCTION(session) /* {{{ */ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */ { - php_session_flush(TSRMLS_C); + /* do not flush session, the flush method is assigned to + php_session_pre_shutdown_function in the MINIT, so it will be called + BEFORE execution has ended! */ php_rshutdown_session_globals(TSRMLS_C); return SUCCESS; @@ -2044,6 +2047,10 @@ static PHP_MINIT_FUNCTION(session) /* {{{ */ #ifdef HAVE_LIBMM PHP_MINIT(ps_mm) (INIT_FUNC_ARGS_PASSTHRU); #endif + + /* Flush the session just before shutdown. */ + php_session_pre_shutdown_function = php_session_flush; + return SUCCESS; } /* }}} */ @@ -2059,6 +2066,8 @@ static PHP_MSHUTDOWN_FUNCTION(session) /* {{{ */
Re: [PHP-DEV] Execution point of session save handler on shutdown
Hi, I've used APC + custom session handler for ages without any problems. Probably just under different circumstances. And couldn't this be fixed without such nasty hooks..? I'm open for other suggestions... Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Execution point of session save handler on shutdown
Hi, I have been investigating a problem occuring when using APC in combination with userspace session save handlers. This has been reported multiple times both in the APC and PHP bugtrackers, bugs related to this issue include (but are most likely not limited to): This has been documented feature for several years. You should explicitly call session_write_close() when your custom session handler is a userspace object. I would consider this a work-around to a design issue - but not a solution. With other extensions, I could imagine even crashing PHP in this way if a suitable extension is loaded - assume such an extension has some global state initialized on RINIT and deinitialized on RSHUTDOWN, and then after de-initialization the user-space session handler kicks in and tries to do something with the extension - all kinds of weird things could happen. If you say: ok, we want to force the user to use session_write_close(), then we should make sure php_session_flush() is only called at the end of the script if no userspace handler is installed, else emit a warning or whatever (if that's even possible at that point because output was already flushed completely). But even then you still have the problem with serialization. If you put an object into your $_SESSION data that has a __sleep function - and that __sleep function uses some kind of other functionality other than returning a simple array, you will run into problems. And that is the case regardless of whether you use a userspace session handler. I don't think that ANY extension should use the engine to execute userspace code in RSHUTDOWN - at all. If an extension does such a thing, I'd consider the resulting behaviour undefined and prone to all sorts of trouble. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] php for android
Hi, If you remove that, there are only two parts that seem to be really ARM-specific: 2) a patch for zend_float.h Both of them are not clear to me, would you care to explain why are they needed? Since I'm responsible for zend_float.h: When running ./configure directly without telling it to use a cross compiler and later manually patching the build system, exactly this kind of thing becomes necessary. If the local CPU is either x86 or x86_64, which ./configure thinks is the target, checks for the presence of certain FPU instructions will indicate that they are there, thus configure sets different constants. On ARM, however, the FPU is different [2] and with the constant defined, compilation will fail. When doing this properly with a cross compiler [1], ./configure checks should be able to determine that the FPU doesn't have these kind of instructions (and doesn't need them, they are only required for x86 / x86_64 without SSE) and the constants will not be set automatically. So the patch for zend_float.h is *not* necessary for ARM when using cross-compilation correctly or when compiling it on ARM natively... Regards, Christian [1] Something like: CC=path to CC LD=path to ld ./configure --target=arm-linux ... --target=arm-linux is necessary to tell configure that its using a cross compiler. [2] In some ARMs at least it's completely software based. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Closures and $this: Please vote!
Hello again, A quick summary of the votes so far (since 15th December): internals@ votes: 0: (1) Alexey Zakhlestin A: (1) Hannes Magnusson C: (0) - D: (0) - A+: (2) Christian Seiler, Joey Smith AS: (1) Stanislav Malyshev --- 5 votes [AS is Stas' version with bind/bindTo but no implicit $obj-x = function () { ... } ; $obj-x(); without userland implementing it in __call] non-internals@ votes: 0: (0) - A: (1) Victor Bolshov C: (0) - D: (0) - A+: (4) Ionut G. Stan, Richard Quadling, Mike Robinson, Larry Garfield AS: (0) - --- 5 votes Total tally: 0: 1 A: 2 C: 0 D: 0 A+: 6 AS: 1 --- 10 votes My suggestion is to wait until the 15th of January (that's one month since I started this thread) and that should have been enough time of everybody. Regards, Christian PS: As a side note, since somebody brought it up in the past few days: It is not really *technical* issue with the engine which variant we implement, it's simply a design choice. We didn't drop $this support in closures in 5.3 for technical reasons but for the simple reason that there was no agreement in sight on *how* to implement it. And since the current code for closures is a bit of a mess in PHP6 because of the back and forth due to the numerous discussions, I'll have to touch it again anyway. (I only cleaned up 5.3 for the release.) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Closures and $this
Hello again, Discuss away! I'm a little disappointed by the non-outcome of this debate. Very few people have responded and most of them seem to agree proposal (A) should be implemented, perhaps with the additional bind/bindTo as in my proposal and perhaps not. The problem here is: (A) was exactly the thing that was implemented and that people started to have a problem with as soon as the first or second beta of PHP 5.3 was released. And because the feedback came in so late, Lukas Johannes decided to remove $this support from closures for PHP 5.3 in order to be able to decide that later on. So basically we're at the same point where we were a little more year ago: There's an RFC for this (the semantics of $this support were discussed in the original closures RFC!) and the people who read it on internals@ support it. However, I predict that if we implement exactly the semantics that the RFC proposes, we will get into the same discussion we had with PHP 5.3 just before the release of PHP 6... (or 5.4 should there be one). So: What now? Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Closures and $this: Please vote!
Hi Lukas, Call for a vote. This time around people cannot claim to not have had time to review the issue. Also back then we tried to play it safe because of the short time before we were to release. This time there is more time for this to mature if needed inside svn. Ok, so then I call for a vote. Again, here are the options: (0): No $this in closures, keep it that way. (keep PHP 5.3 behavior) (A): Original closures implementation: $this is always the object context at closure creation. No possibility to do $someObject-closureProperty(...) and thus no possibility to extend objects! (C): Javascript-like behaviour: Bind $this only when calling the closure as object method, else $this is undefined. (D): JS-like behaviour on top of (A). Please look at the RFC as to why I consider it to be a *REALLY*, *REALLY* bad idea. (A+): (A) + Closure::bind Closure-bindTo for rebinding if this is wanted the possibility to call a closure as an object method. (See last section of RFC for details) My vote: (A+) Regards, Christian PS: Note that I removed (B) from the possible options since I believe it to be an EXTREMELY bad idea if one thinks it through. It was only added to the RFC in order to give an overview over what was discussed previously. Unless someone can make an extremely compelling case why I'm wrong in this respect, I will refuse to implement (B). -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Closures and $this: Please vote!
Hi, Ok, so then I call for a vote. Again, here are the options: A+, no direct method calling (get/call problem would be messy) I don't quite follow: Why A+ if no direct method calling? What would be the point of allowing bindTo() if $obj-closureProp() doesn't work anyway? If you don't want $obj-closureProp() I'd expect you to vote for A and not A+... (If you want to stick to that it's fine, I'm just a little stupefied.) Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Closures and $this
Hi, since a few months have passed since the last discussion on this topic and perhaps people had time to gather some experience with the current closure implementation in PHP 5.3 I'd like to restart the debate on $this in closures and object extension. Foreword: The discussion should center on the future behaviour of PHP 6 ONLY. Whether or not a backport is even possible (binary compability etc.) or should even be done should be topic of a separate discussion and should *NOT* influence the decision. The ONLY goal of this discussion should be to agree on a SANE way of implementing $this for closures for PHP 6. I've updated the original RFC I wrote a tiny bit (I didn't change much): http://wiki.php.net/rfc/closures/object-extension The basic outline is the following: * In the first section I explain the general issue. * In the second section I show the proposals that were made on internals@ before I wrote the RFC. * In the third section I compare the approaches and explain why the approaches (B) and (D) are inconsistent. * In the fourth section I propose a new approach (bindTo) that was only briefly discussed after the original RFC. Please read the complete RFC and try to understand the points I'm trying to make. If something is unclear, *please* ask first. In the past discussion I had the impression that a lot of people understood only partial aspects of the problem which made the discussion extremely noisy. I believe that is a disservice to the issue. However, now we have the huge advantage of NOT having an immanent deadline for a release. I hope this will enable a consensus on this issue. Discuss away! Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] 5.3.0 stable release
Hi Lukas, If issues are found/fixed please send the patches to internals for review. Based on the importance and risk of the patch will then be applied, however the next 2 days should really be focused on testing to make sure we do not have critical issues, minor issues can always be fixed in 5.3.1 and we better release with known minor issues than big unknown issues caused by a last minute fix. I have a minor issue with regard to spl_autoload_unregister / spl_autoload_functions and closures in my backlog. I already committed the patch tests to HEAD [1]. I see this as extremely minor (since spl_autoload_register was fixed a while ago and that is the only *really* important use case in my eyes) so I'm perfectly happy to wait until after 5.3.0 to merge it from HEAD. But just in case I've attached the patch for 5.3 (NEWS file update not included in patch). [1] http://news.php.net/php.cvs/58900 Regards, Christian Index: ext/spl/php_spl.c === RCS file: /repository/php-src/ext/spl/php_spl.c,v retrieving revision 1.52.2.28.2.17.2.38 diff -u -p -r1.52.2.28.2.17.2.38 php_spl.c --- ext/spl/php_spl.c 13 Jun 2009 17:30:50 - 1.52.2.28.2.17.2.38 +++ ext/spl/php_spl.c 22 Jun 2009 18:14:56 - @@ -509,10 +509,10 @@ PHP_FUNCTION(spl_autoload_register) alfi.closure = zcallable; Z_ADDREF_P(zcallable); - lc_name = erealloc(lc_name, func_name_len + 2 + sizeof(zcallable-value.obj.handle)); - memcpy(lc_name + func_name_len, (zcallable-value.obj.handle), - sizeof(zcallable-value.obj.handle)); - func_name_len += sizeof(zcallable-value.obj.handle); + lc_name = erealloc(lc_name, func_name_len + 2 + sizeof(zend_object_handle)); + memcpy(lc_name + func_name_len, Z_OBJ_HANDLE_P(zcallable), + sizeof(zend_object_handle)); + func_name_len += sizeof(zend_object_handle); lc_name[func_name_len] = '\0'; } @@ -579,6 +579,7 @@ PHP_FUNCTION(spl_autoload_unregister) { char *func_name, *error = NULL; int func_name_len; + char *lc_name = NULL; zval *zcallable; int success = FAILURE; zend_function *spl_func_ptr; @@ -604,10 +605,20 @@ PHP_FUNCTION(spl_autoload_unregister) efree(error); } - zend_str_tolower(func_name, func_name_len); + lc_name = safe_emalloc(func_name_len, 1, sizeof(long) + 1); + zend_str_tolower_copy(lc_name, func_name, func_name_len); + efree(func_name); + + if (Z_TYPE_P(zcallable) == IS_OBJECT) { + lc_name = erealloc(lc_name, func_name_len + 2 + sizeof(zend_object_handle)); + memcpy(lc_name + func_name_len, Z_OBJ_HANDLE_P(zcallable), + sizeof(zend_object_handle)); + func_name_len += sizeof(zend_object_handle); + lc_name[func_name_len] = '\0'; + } if (SPL_G(autoload_functions)) { - if (func_name_len == sizeof(spl_autoload_call)-1 !strcmp(func_name, spl_autoload_call)) { + if (func_name_len == sizeof(spl_autoload_call)-1 !strcmp(lc_name, spl_autoload_call)) { /* remove all */ zend_hash_destroy(SPL_G(autoload_functions)); FREE_HASHTABLE(SPL_G(autoload_functions)); @@ -616,16 +627,16 @@ PHP_FUNCTION(spl_autoload_unregister) success = SUCCESS; } else { /* remove specific */ - success = zend_hash_del(SPL_G(autoload_functions), func_name, func_name_len+1); + success = zend_hash_del(SPL_G(autoload_functions), lc_name, func_name_len+1); if (success != SUCCESS obj_ptr) { - func_name = erealloc(func_name, func_name_len + 1 + sizeof(zend_object_handle)); - memcpy(func_name + func_name_len, Z_OBJ_HANDLE_P(obj_ptr), sizeof(zend_object_handle)); + lc_name = erealloc(lc_name, func_name_len + 2 + sizeof(zend_object_handle)); + memcpy(lc_name + func_name_len, Z_OBJ_HANDLE_P(obj_ptr), sizeof(zend_object_handle)); func_name_len += sizeof(zend_object_handle); - func_name[func_name_len] = '\0'; - success = zend_hash_del(SPL_G(autoload_functions), func_name, func_name_len+1); + lc_name[func_name_len] = '\0'; + success = zend_hash_del(SPL_G(autoload_functions), lc_name, func_name_len+1); } } - } else if (func_name_len ==
Re: [PHP-DEV] bug 48541 - critical for PHP 5.3
Hi Greg, I can do it if someone can answer this question: how do closures uniquely identify themselves? spl_autoload_register is mistakenly treating all closures as if they were a single copy of the static method Closure::__invoke, and so only the first registered closure is ever called (plus it leaks the other closures at shutdown). If the system can be made to better identify the closure, then spl_autoload_register can use that (and also properly free the refcount on a dupe). A closure can only be uniquely identified by the object storage id the object has. You cannot assume any identity with regard to, for example, file name and line where it was defined, since one could imagine the following: foreach ($dirs as $dir) { spl_autoload_register (function ($class) use ($dir) { include $dir.'/'.$class.'.php'; }); } (or something like it) If nobody else does it I'll fix this later today. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] bug 48541 - critical for PHP 5.3
Hi Greg, (I meant commit when I said patch, sorry :) http://news.php.net/php.cvs/58696 and http://news.php.net/php.cvs/58697 Thanks, looks fine to me. However, I noticed that closures don't work with spl_autoload_unregister() and spl_autoload_functions(). Also, classes that define __invoke work with spl_autoload_functions() but not with spl_autoload_unregister() - unless array ($o, '__invoke') is given to spl_autoload_register() directly, then the same will work for unregister. Or, to sum it up: i) Real Closures $c = function ($class) { var_dump ('foo'); } spl_autoload_register ($c); var_dump (spl_autoload_functions ()); // '{closure}' spl_autoload_unregister ($c); // no effect ii) Invokables WORKS: ?php class Autoloader { private $dir; public function __construct ($dir) { $this-dir = $dir; } public function __invoke ($class) { var_dump ({$this-dir}/$class.php); } } $al1 = new Autoloader ('d1'); $al2 = new Autoloader ('d2'); spl_autoload_register (array ($al1, '__invoke')); spl_autoload_register (array ($al2, '__invoke')); var_dump (spl_autoload_functions ()); spl_autoload_unregister (array ($al1, '__invoke')); $x = new Test; ? SEMI-WORKS: ?php class Autoloader { private $dir; public function __construct ($dir) { $this-dir = $dir; } public function __invoke ($class) { var_dump ({$this-dir}/$class.php); } } $al1 = new Autoloader ('d1'); $al2 = new Autoloader ('d2'); spl_autoload_register ($al1); spl_autoload_register ($al2); var_dump (spl_autoload_functions ()); // gives array ($object, '__invoke') instead of // directly $object - but that's at least equivalent spl_autoload_unregister ($al1); // no effect spl_autoload_unregister (array ($al1, '__invoke')); // no effect $x = new Test; ? Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] float ops and _controlfp_s
Hi Pierre, (Btw. you got the wrong Christian as CC. ;-)) There is still a significant performance impact due to _controlfp_s usage. Huh? After the modification I made this should only called once (!) at PHP startup Or did we miss something? Anyway, calling it once should not cause any performance problems at all. Can you explain why we need it and what tests are affected by this change please? This ensures that FPUs on x86 systems are definitely in double precision mode. The default situation on Windows with MSVC is that the FPU is already in double precision setting as far as I've tested it - however there are situations (PHP embedded into a webserver for example) where this may not be true. Could you please provide some details for significant performance impact? Regards, Christian PS: My package arrived a while ago and I remember I didn't say thanks yet - so here it goes: Thanks a lot! -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] bug 48541 - critical for PHP 5.3
Hi, [quote from off-list:] I am in way over my head and would not do a good job fixing these (or have the time), care to take a crack at it? Ok, I'll do that tomorrow morning. I wonder if the problem is that spl_autoload_register should not just be checking to see if zcallable is IS_OBJECT, but also if it is an instance of Closure. That should clear up the examples below (spl_autoload_register($blah) where $blah is not a closure should not work). No, actually it should definitely work, since $blah () works when the class defined __invoke and objects of this class are thus callable. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Reflection
Hi Kalle, 3) Closures, theres alot of closure differences in HEAD and 5.3, for example HEAD has ReflectionMethod::getClosure() and ReflectionFunction::getClosureThis(), but 5.3 does not, which makes it looks like a change in 5.3 that never occured to HEAD, unless that is the logic is fixed in HEAD. We should really fix this, so 5.3 have these if needed. The reason for this is that I removed $this and OOP support from closures on Johannes's and Lukas's request in 5.3 since there was no consensus on how this should actually work (see [1]). I didn't revert in HEAD because there was at least consensus on the fact that someday in the future Closures should support OOP/$this after we decide how. The discussion on that topic has not progressed, however - nobody actually reacted to my explanation on this topic (see [2]) - even after the removal in 5.3 (which was done in order to allow the discussion to progress independently of 5.3 so that a solution is not forced due to time constraints). My rationale for not removing it from HEAD was the simple fact that I thought the discussion on this topic would progress and after 5.3 we would reach a consensus on how to implement that. In that case, the code in HEAD would have only needed to be changed, but not re-added. I did not anticipate that the discussion would die down completely for so long and that no progress would be made in that case. Anyway, we have the following two options for HEAD: 1. Sync HEAD with 5.3 and remove that stuff also. Which could lead to additional, unecessary work after we decide which way we want to go. 2. Leave it as is until we have decided. Anyway, if anybody wants to renew the discussion on this topic, look at [2] where all the details and problems are explained. [1] http://wiki.php.net/rfc/closures/removal-of-this [2] http://wiki.php.net/rfc/closures/object-extension (please ignore timeline in the second link, it's outdated) Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] double to long conversion change
Hi Daniel, But, for what you're testing, that's the behavior I'd expect -- once you've reached the precision of a double, you'll only get the closest representation possible (and of course a 64-bit long is more precise than a double since there's no floating point to represent). Also, I assume what can be represented by a double is the same across platforms, if it's IEEE 754. Yes. But I was expecting that since long on 64-bit machines holds 64 bits in PHP (et al), that PHP would use C's long double type for floats on 64-bit platforms rather than plain old doubles. It seems like the user-friendly, PHP way to handle the situation, particularly as 64-bit computers are commonplace these days. If you talk about 64 bit platforms, the 64 bit refers only to integers - not floats. There is no difference between the supported floating point types and operations of the most recent 32 bit x86 processors and the newer 64 bit x86_64 counterparts. Both have an i387 compatible FPU and both implement the SSE standard for vectorized floating point operations. SSE only supports single precision (23 + 8 + 1 = 32 bit) and double precision (52 + 11 + 1 = 64 bit) data types for vectorized operaitons. The i387 FPU supports single precision, double precision and a proprietary Intel double extended precision data type which uses 80 bits (actually only 79 are really necessary). Other processor architectures may only support single and double precision data types, yet others support some kind of double double precision which is a combination of two double precision values (total 128 bits) to support higher mantissa (but not higher exponent) and yet others support a real quad precision data type which has a larger mantissa and exponent. Some compilers allow the use of any of the above data types (Intel double extended, double-double, quad) via the »long double« C data type. Others (ESPECIALLY ALL the Microsoft compilers!) do not support »long double« but rather make »long double« be a normal double value. And for double-double data types the calculations are nearly always done with software emulation. So basically the situation is the following: You have 4 different possible data types for long double in C. Each with different mantissa and different exponent and 3 different sizes (64, 80 and 128 bit). To me, this sounds like hell. One of the changes I made in PHP 5.3 was actually to make sure *all* platforms use the normal IEEE double data type also for calculations. (there is something like internal precision in x87 compatible FPUs which makes life very complicated) This was done to ensure portability of the code. Because with floating point operations, even simple numbers (such as 0.01 or 1.0/3.0) cannot be exactly represented by a computer and thus every bit of the mantissa is required to approximate the number. If you then have different precisions on different platforms or even with different compilers, life gets complicated. For example, if you check that $a + $b = $c on one system, this need not be true on another system if the precisions don't match - there are examples in both ways! So please, please, please: Don't complicate life and introduce long double in PHP. At least as long as there is no standardized 64 bit floating point data type that works across platforms and compilers. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Memory Leak in ob_get_clean() / ob_get_flush ()
Hi Lukas, Did this get addressed yet? No, it didn't. But since no one complained about it I'll commit it later this evening. (commit freeze is over, right?) Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Memory Leak in ob_get_clean() / ob_get_flush ()
Hi, When running 'make test' on my system I discovered that tests/output/ob_start_basic_unerasable_003.phpt and tests/output/ob_start_basic_unerasable_004.phpt produced memory leaks - due to the fact that they first fetch the buffer into return_value but then do RETURN_FALSE if they detect an error and thus leak the copied buffer. I attached a patch that fixes that to this mail. Any objections to me applying this for 5.3 and HEAD? (after 5.3 RC1 when commits are allowed again of course) Any side-effects I didn't think about? Regards, Christian Index: main/output.c === RCS file: /repository/php-src/main/output.c,v retrieving revision 1.167.2.3.2.4.2.12 diff -u -p -r1.167.2.3.2.4.2.12 output.c --- main/output.c 13 Feb 2009 11:48:17 - 1.167.2.3.2.4.2.12 +++ main/output.c 18 Mar 2009 02:09:13 - @@ -867,10 +867,12 @@ PHP_FUNCTION(ob_get_flush) /* error checks */ if (!OG(ob_nesting_level)) { php_error_docref(ref.outcontrol TSRMLS_CC, E_NOTICE, failed to delete and flush buffer. No buffer to delete or flush.); + zval_dtor(return_value); RETURN_FALSE; } if (OG(ob_nesting_level) !OG(active_ob_buffer).status !OG(active_ob_buffer).erase) { php_error_docref(ref.outcontrol TSRMLS_CC, E_NOTICE, failed to delete buffer %s., OG(active_ob_buffer).handler_name); + zval_dtor(return_value); RETURN_FALSE; } /* flush */ @@ -892,10 +894,12 @@ PHP_FUNCTION(ob_get_clean) /* error checks */ if (!OG(ob_nesting_level)) { php_error_docref(ref.outcontrol TSRMLS_CC, E_NOTICE, failed to delete buffer. No buffer to delete.); + zval_dtor(return_value); RETURN_FALSE; } if (OG(ob_nesting_level) !OG(active_ob_buffer).status !OG(active_ob_buffer).erase) { php_error_docref(ref.outcontrol TSRMLS_CC, E_NOTICE, failed to delete buffer %s., OG(active_ob_buffer).handler_name); + zval_dtor(return_value); RETURN_FALSE; } /* delete buffer */ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Questions on closures and visibility
Hi Nate, Any thoughts or feedback would be very much appreciated. :-) The scoping problem is very deeply related to the $this issue: Should one be able to switch the change of a closure by re-binding it to another object or not? It was also an open point in my RFC on object extension in general - which describes the different approaches to $this and scoping and the reason why we have do decide BEFORE adding support for that which way we want it since otherwise we don't be able to change the behaviour later on if we decide against it. So, it may be a small WTF that the scope is not inherited at all. But if we re-add support for that, useful as it may be, there will be no possibility of changing that in the future - and since the scope is deeply related to $this, it could be that the then chosen solution for $this is also incompatible with the scoping version we choose now. When I reverted OOP support for beta 1, I carefully made sure that the current implementation in PHP 5.3 is done in such a way that OOP support may be added later on without dictating which type of OOP support it will be. Any change to that would limit future choices. And since nobody here actually continued the OOP - Closure discussion for post-5.3 after my revert in beta 1, I don't think that discussion will have progressed far enough for RC 1 that re-adding scoping behaviour would be a wise idea. Regards, Christian Just FYI: My stance on the issue hasn't changed, I'm still in favor of the original behaviour Dmitry and I designed + addition of bindTo () for dynamic object extension, see: http://wiki.php.net/rfc/closures/object-extension -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] ZEND_USER_OPCODE_CONTINUE
Hi Johannes, while implementing a small toy extension (see [1]) I found out that ZEND_USER_OPCODE_CONTINUE seems to misbehave as it doesn't go to the next opcode so I ended up in an endless loop executing the same opcode again and again. Isn't that expected behaviour? If a normal opcode handler does not increase the opline, simply returning will also cause re-execution of the same handler again and again. Why should it be different for user opcode handlers? Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] RFC for new INI's
Hi, variables_order: They should be the same on dev and prod. request_order: Seems like it should be the same. Caution! I've read several times in this thread that request_order should be set to something that also contains C. This is DANGEROUS. request_order was specifically introduced to determine the order of variable merging that leads to $_REQUEST, while variables_order defines the variables that are assigned *at all* (and without register_globals and with request_order, the _order is actually misleading). So: request_order should *ONLY* be set to GP in order NOT to have cookies popping up in $_REQUEST - else everybody who uses $_REQUEST is vulnerable to CSRF. Also, a recommendation for request_order only makes sense as GP (on both production *and* developement machine) and setting variables order to GPCS. Furthermore, the comment in the ini file that request_order is in there for performance reasons is just PLAIN WRONG and gives the impression that setting it to GPCS or empty will just cost a little performance - where it clearly allows for CSRF if people use $_REQUEST. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Closures: $this support removed for 5.3 beta 1 (but NOT in HEAD!)
Hi, as Lukas and Johannes have requested, I have removed $this support from Closures in PHP 5.3. Technical details on the changes can be found here: http://wiki.php.net/rfc/closures/removal-of-this (I didn't have time to write new tests for everything, I just removed those tests which didn't make sense at all and changed the rest in order to accommodate the current semantics.) I'd like to add that the Reflection part was really quite a mess. Marcus's property-calling-patch was not correctly revoked from the Reflection part (there was still quite a bit of code there in php_reflection.c). Also there were (independent of that) some changes to my original proposal that - as far as I know - were never discussed on-list. Which in this case caused two problems: Inconsistencies (see ReflectionParameter part in the above page) and there were some memory management issues (which don't occur if you keep your objects always around at least as long as the reflection objects themselves - and thus not during simple tests - but nevertheless issues). I don't want to assign blame here to anyone (the errors are errors I could have *easily* made myself), I just want to point out that if the changes had actually been discussed beforehand, this could have been avoided. I recognize that not everybody will be 100% happy with the changes I made to reflection even in light of the $this removal. However, in the few days it took until the beta1 freeze (today) I think I have found the sanest consistent solution for the problems. I do however acknowledge that we may need to tweak some minor things wrt. reflection here and there after beta 1. Also: I have NOT changed HEAD. This is only in the PHP_5_3 branch. I have also NOT ported the MM fixes to HEAD. It is far less trouble to simply wait until we have figured out $this in closures. snip - In light of everything that went wrong with Closures and OOP due to people wanting to improve something, I'd like to propose the following: Whenever *any* change is made to either the language core itself or to Reflection - that is not just a simple bugfix but rather a change in certain semantics - there should be an RFC page in the wiki for this purpose with LOTS of userland example code to demonstrate the semantics and to be able to think it all through more thoroughly. And only after an on-list discussion the changes should be made. And a page in the wiki should really be required instead of a simple mail (and the wiki page should actually be updated) because when it really comes down to it, you don't find the mail again. With Closures and OOP we had the situation that changes were committed both to Reflection and core where only later other people actually saw what they meant and this caused a discussion after the code was in a half-baked state. Also, there was not enough userland example code available from all sides for quite a while so that the discussion remained on the same spot for too long. I really would like to avoid that in the future and thus I think the above idea would be really helpful. To sum it up again: For every non-bugfix change to core or Reflection [*] that affects semantics: proposal in the wiki, lots of userland example code to demonstrate the change, discussion on-list. [*] Possibly more extensions. Reflection is especially tricky since it has to reflect the PHP core quite well on the one hand and on the other hand has to be very self-consistent which is quite hard due to all the inter-dependencies. It took me the last few days to figure the above changes all out and to make them consistent. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Object extension, $this binding of closures, chat results
Hi Dmitry, The only difference is in binding/creation. You suggest $obj-method2 = Closure::bind ($obj, function () { ... }); and I would prefer something like create_prototype_method($obj, method2, function () { ... }); I prefer a static method of the Closure class because it does not pollute the global function namespace - and also it is far mor OOP-ish and we are doing this for closure OOP support. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object extension, $this binding of closures, chat results
Hi again, ok, I just verified that the current PHP 5.3 CVS has the same behaviour as PHP 5.3 alpha 3 (which is the original design). So basically, I'd suggest the following: * Feature freeze as Lukas and Johannes had planned tomorrow with *no* more changes wrt. closures for beta1, then release beta1. * For post-beta1: Discuss which of the following options we want to take: a) My Closure::bind() compromise b) Leave as is in order to add Closure::bind() later (5.3.1, 5.4, 6.0) when we've discussed all the details. c) Drop $this/OOP support from closures for beta2 in order to be able to discuss this properly for a later version. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] prototyping
Hi, maybe an IRC meeting is the easiest way to come to an agreement. How about tomorrow evening 21:00 CEST in #php.closures on freenode? Just for clarification: I assume you mean Wednesday, January 21st, 19:00 UTC (CEST == UTC+2) and thus 20:00 CET? Would be fine with me. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] prototyping
Hi Marcus, And I could say that what the two of you designed ofr PHP is not a design but a very confusing incoherent implementation that is based on the lack of being able to get support for something else in the underlying c implementation. Huh? The current implementation is by design, not because of any limitations on C level. If you are refering to the following sentence in your answer to Stanislav: If this really was to be bound to a $this at the creation, then we'd put it into the scope binding list and not have it be handled implicitly. But then we cannot do that with the current c level api. The fact is: We could very easily. We decided against it (it's somewhere in the internals archives from over half a year ago, I'm too lazy to look for it). So, if I get you, you say: If $this is only bound at closure creation time, then you should add $this to use(), i.e.: $f = function () use ($this) { ... }; Well, if you rather want that syntax instead of static/non-static, you could simple edit zend_compile.c and do the following: - change zend_do_begin_lambda_function_declaration: set fn_flags to ZEND_ACC_STATIC by default (and remove the is_static parameter from that function) - change zend_do_fetch_lexical_variable: In the case $this was specified as a lexical variable, remove the ZEND_ACC_STATIC flag from the function flags and return immediately (and remove the current errormessage) - remove the T_STATIC from the grammar definition in zend_lang*.y Unified diff of that should be max. 100 lines long. So, this is NOT a limitation on the C level, this is a deliberate design decision. And, btw., it is not a design decision I care about very much - if there's a majority to change it, I've no problem with that. You took somethign very special and made it behave somewhat different making it even more specific and different from anyone's expectations, excluding people that understand the underlying c level issues. No, we did not do any such thing. People who do a lot of Javascript expect the behaviour you describe (i.e. re-binding), but that does not necessarily mean everybody does. And if you read postings made by beginners with Javascript in beginners' web forums or beginners' mailing lists, the re-binding of the this variable is a VERY confusing thing for beginners and not natural at all. You may have grown accustomed to this behaviour, but please do not make the mistake of thinking that your expectations are the expectations of the rest of the world. I do acknowledge that some people like you will expect this behaviour due to their background in Javscript. I also posted in HUGE DETAIL why I find your approach to be *very* problematic. I also went into detail on why you find your approach more natural and why Javascript is different than PHP in this aspect so you can't simply copy the concept from one to another. I also added a proposal which contains an alternative that may not be ideal but may satisfy all the needs. But instead of really replying to that, you simply took the first sentence, used my admittedly not very ideal wording as polemic and repeated your statement from before. I don't think this will get us anywhere. So, I would be very happy if you could 1) acknowledge that what you would expect is not necessarily that what everybody else would expect (some others certainly, but not everybody), 2) realize that many (albeit not all) things in the current implementation are NOT there because of C limitations but are deliberate (see above for an example) and 3) have another look at my previous posting, think about it for some time and THEN reply to it. Because the intention behind my posting was NOT to say you're wrong, you ... but rather to say I get where you are coming from but have a look at this example, your proposal will cause additional problems. And your expectations do not necessarily match mine. But I don't want to dismiss your idea completely, so here's an alternative idea that came to my mind, please think about it. Regards, Christian PS: By the way, Python does not rebind closures that enclose the current object handle either: class Foo: def __init__ (self, v): self.v = v def getClosure (self): def closure (w): return self.v + w return closure o1 = Foo (2) o2 = Foo (3) # both are o1.getClosure not o2! o1.xyz = o1.getClosure() o2.xyz = o1.getClosure() print o1.xyz (2) # 4 print o2.xyz (2) # 4 (Yes, that's due to the different concept of the 'self' variable in Python compared to the way PHP and Javascript use $this, but it just demonstrates that one has to look at the other concepts that are implemented in the language in order to get something consistent. That's precisely my argument for PHP, too.) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit:
Re: [PHP-DEV] [RFC] prototyping
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
Re: [PHP-DEV] [RFC] build IDs proposal
Hi Pierre, Would any of the PHP Windows guys like to comment on this? I did, and the reason behind this proposal was about solving the VC* problem for the end users (we have to add another condition to check, which CRT is used) :) I'm sorry for asking again, maybe I'm just plain stupid ;-) but I'm not quite sure how to interpret your response (I've no idea what VC* problem you mean anyway, I never had problems with PHP under Windows at all): So, basically, my situation would still work, even with the build ID? To clarify what I'm doing currently: 1. Build PHP with additional extension in ext/$name with VC8 (VS 2005) on machine A (developement machine). 2. Install both VC6 and VC8 runtimes on machine B (production machine) 3. Install official PHP build or Zend Core (compiled with VC6) on machine B 4. Copy the extension DLL (compiled with VC8) to machine B and use it as a PHP extension. (This *does* currently work with PHP 5.2.x - at least for me.) Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] build IDs proposal
Hi, I think that it would be better if we adopted more clean and scalable solution for that. I propose having one string build ID, which would look something like: API20071006,NTS,Debug,VC8 and would be rquired to match exactly in the engine and the module. This should be relatively easy to generate when compiling. I'm generally in favor of that change, however, I'm not quite sure on the impact of including the Visual Studio version specifier. I don't have that much experience with PHP on Windows but I have successfully built PHP extensions with VC8 (i.e. VS 2005 Express) for PHP 5.2.x. I didn't have any problems whatsoever in using these extensions together with the official PHP builds (VC6) and Zend Core (IIRC also VC6) - as long as both VC runtimes were installed on the corresponding system. Since VC6 is not available for free download I wouldn't be able to build Windows extensions for VC6 builds of PHP anymore if the version string check would inhibit the loading of the extension. Would any of the PHP Windows guys like to comment on this? Viele Grüße, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] build IDs proposal
Hi Pierre, A VC6 extension is not compatible with a VC9 build (or the other way round). It crashes randomly depending on what the extension does (xdebug for one is really good for this test :). You wouldn't know about how VC8 fits in here? Anyway, thanks for the pointer to xdebug, I'll try that with my VC6/VC8 mix and see if that crashes. By the way, VS2008 Express is free and 100% compatible with a normal VS2008. You may use it so you can use our VC9 builds directly or provide extensions for it. Yes, I know, but the problem is that it doesn't run on Win2k. ;-) Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Removing basic types from our JSON parser
Hi, Basic types were added to our JSON decoder to PHP 5.2.1, this allows one to use json_encode / json_decode on any of our scalar types. Omar correctly identified #38680 as not a bug but it appears that Ilia added support for this anyway violating the RFC [1]. Maybe there was a reason for this but I'm not sure why? PHP is typeless language, IMO its conductive to this design to allow encoding of basic types via json_encode, it reduces the code when communicating to/form JavaScript to avoid type detection. While it does violate the RFC IMO the convenience of the feature is definitely worth it. I agree that it's convenient for exporting variables to Javascript. On the other hand, one should always be lenient when accepting input but strict when generating output. Having json_encode encode non-arrays and non-objects by default will probably allow some people to shoot themselves in the foot when interfacing with other JSON applications. Thus I'd propose the following: json_encode already posesses an $options parameter that allows one to specify certain options for encoding as a bitmask. My proposal would be to add another option PHP_JSON_ENCODE_BASIC to allow the encoding basic types. When set, json_encode(hi) and json_encode(42) will work without any problem. When not set, I propose the following behaviour when passing a string or integer etc.: * PHP 5.3: An E_DEPRECATED-Warning which mentions the new option but still return the result. * PHP 6: Return false or whatever it currently returns when encoding wasn't possible and throw an E_WARNING. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: cvs: php-src /ext/posix/tests posix_getcwd.phpt posix_getgrnam.phpt posix_getpwnam.phpt posix_getrlimit.phpt posix_initgroups.phpt posix_isatty.phpt posix_mknod.phpt
Hi Felipe, --SKIP-- ?php if (!posix_mknod('posix_getcwd')) die('skip posix_getcwd() not found'); ? ?php if (!posix_mknod('posix_isatty')) die('skip posix_isatty() not found'); ? ?php if (!posix_mknod('posix_mknod')) die('skip posix_mknod() not found'); ? Shouldn't those be function_exists instead of posix_mknod? Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] apha3
Hi, I have given you ZE karma. Please commit this yourself no later than Tuesday evening! Thanks done. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: cvs: ZendEngine2(PHP_5_3) / Zend.m4 acinclude.m4 zend_float.h zend_operators.c zend_strtod.c /tests float_prec_001.phpt
Hi, Your macros are not compatible with autoconf 2.13: ***BUG in Autoconf--please report*** AC_LINK_IFELSE Grmpf. Problems like this are why I absolutely loathe autoconf. I'll install 2.13 right away and fix this. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: cvs: ZendEngine2(PHP_5_3) / Zend.m4 acinclude.m4 zend_float.h zend_operators.c zend_strtod.c /tests float_prec_001.phpt
Hi, Your macros are not compatible with autoconf 2.13: ***BUG in Autoconf--please report*** AC_LINK_IFELSE Grmpf. Problems like this are why I absolutely loathe autoconf. I'll install 2.13 right away and fix this. Done, tested with autoconf 2.13 and 2.61: http://news.php.net/php.zend-engine.cvs/7244 http://news.php.net/php.zend-engine.cvs/7245 Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] apha3
Hi Lukas, First of all, @all: http://wiki.php.net/rfc/rounding - I didn't have time to update it yet but the basically nothing has changed except for implementation details wrt. FPU precision. For FPU precision explanations see (*). Anyways, a few items from the top of my head that I know people were talking about: - rounding patch I've split the patch in two parts: (1) Make the Zend Engine always use double FPU precision on x87 platforms (creates zend_float.h, defines certain macros, uses those macros in zend_strtod.c and zend_operators.c), include the correct configure checks that detect the switching method. zend_float.h is basically a copy of my xpfpa.h from (*) where I put in the research on the behaviour of different platforms and compiler versions. (2) Modify round() behaviour as described in the RFC (that also uses the x87 FPU precision switch). The second part will only compile if the first part was applied. It is basically the same patch as I have already posted to this list - just with a few cleanups. Here are the basic changes to the previous patch versions: a) Added Zend/tests/fpu_prec_001.phpt which tests for double precision. With single, double-extended or quad precision, it will fail. b) @Macrus: I've thought about your points wrt. cross-compilation and I am now certain that if the configure test scripts compile link, the code will also work - so there is no need to execute it. I've thus changed AC_TRY_RUN to AC_LINK_IFELSE. c) Added ext/standard/tests/math/round_{prerounding,large_exp,modes}.phpt which check for the new behaviour of round(). d) Some minor cleanups to the FPU macros and configure checks. Anyway, here are the patches, please remember that 1+3 test files are added with the patches: PHP 5.3, ZE2 FPU precision: http://www.christian-seiler.de/temp/php/2008-11-28-fpu+rounding/php53-fpu.patch PHP 5.3, round() behaviour: http://www.christian-seiler.de/temp/php/2008-11-28-fpu+rounding/php53-round.patch PHP 6, ZE2 FPU precision: http://www.christian-seiler.de/temp/php/2008-11-28-fpu+rounding/php6-fpu.patch PHP 6, round() behaviour: http://www.christian-seiler.de/temp/php/2008-11-28-fpu+rounding/php6-round.patch (*) http://www.christian-seiler.de/projekte/fpmath/ Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: cvs: php-src(PHP_5_3) /ext/standard math.c
Hi, Modified files: (Branch: PHP_5_3) /php-src/ext/standard math.c Log: Fixed bug #42294 (Unified solution for round() based on C99 round) [DOC] New implementation of round() to work-around inconsistencies for win32 and 64 bit platforms. This solution is very roughly based on BSD's implmentation of round(), which itself is an implementation of C99 standard. We take the absolute value of number we want to round time the 10 to the power of the number of decimal spaces we are rounding to. The resulting value is rounded up and the pre-rounded value is subtracted from it. If the difference is greater then 0.51 we round up, otherwise we round down. Apparently, nobody reads internals anymore. :-( I made my initial comment on the bug report over a year ago. Since then I've dug quite a bit into floating point arithmetics and the actual problems behind round(). This lead to: http://wiki.php.net/rfc/rounding Which I posted quite a while ago and nearly nobody was interested: http://news.php.net/php.internals/40070 [Now please don't simply apply the patch there, I've done some additional research since.] The solution I proposed over a year ago is actually wrong and it does not solve all the problems of round()'s g, see my RFC for that. The general problems with round are actually: 1) Internal FPU precision on x86. 2) Specification problem (which rounding mode should actually be used?) 3) Dividing/multiplying by = 10^23 is not exact. 4) round (1.255, 2) should give 1.26 but gives 1.25. The FUZZ stuff tries to resolve this issue (but not the other three) but I've come to the conclusion that the FUZZ is actually the wrong solution for the problem. Since I didn't get any reaction on the RFC on internals, I pinged Lukas and Johannes (as they are RMs for PHP 5.3) in private in order to ask whether it was possible to include my proposal in 5.3 (I don't have ZE2 Karma and my patch also needs to change zend_strtod). Lukas and Johannes were concerned about the interopability of my solution of problem (1). So I did some tests on different platforms and operating systems and Lukas and Johannes asked around for other people to do tests, too. I've summarized results of these tests and proposed solution for correct cross-platform floating point arithmetics here: http://www.christian-seiler.de/projekte/fpmath/ I've mailed patches for PHP 5.3 and HEAD to Johannes and Lukas for ZendEngine2 that only address the above issue (1) and do the following: 1) Define some macros for math-related functions that will ensure the function itself always uses double precision. Add configure checks for these macros. 2) Modified zend_strtod and the add/sub/div/mul functions in zend_operators.c to make use of those macros. This ensures that PHP will always use double precision for calculations and math is thus portable. 3) Added a test that determines if the calculations precision is actually correct. The patches for 5.3 and HEAD are found here: http://www.christian-seiler.de/temp/php/2008-10-28-fpu/php-float-precision-5.3.patch http://www.christian-seiler.de/temp/php/2008-10-28-fpu/php-float-precision-6.patch My next step (as discussed with Johannes and Lukas) would have been to apply the Non-ZE2-part of my patch to ext/standard/math.c in 5.3 and HEAD (I don't have separate patches for that yet but they are quite trivial to adapt to the new macros). Now the question is: Where do we go from here? Your commit does not solve all the problems of PHP's round but is at least better than the previous implementation since at least the platform-dependency on whether or not to use the FUZZ is removed. The other problems, however, remain. Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: My rounding proposal: Ok for PHP 5.3?
Hi Lukas, Hi Scott, Scott said he could apply the patch for you. And he is sitting right in front of me .. Actually, since I've stirred up the list a bit just now, I'd like to wait until we have consensus on this issue. But thanks for the offer. :-) Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: cvs: php-src(PHP_5_3) /ext/standard math.c
Hi Stefan, 1) Define some macros for math-related functions that will ensure the function itself always uses double precision. Add configure checks for these macros. Do you know what the rationale behind the standard compiler behaviour is? Because trying to outsmart the compiler is usually not a good idea. The only time when I try to outsmart the compiler is the volatile return variable trick - and volatile guarantees according to the C standard the intended behaviour. The main use of these macros is to correctly set a processor flag to a certain value (either via other macros defined in system C headers or by functions provided by the runtime of the OS - and as a fallback inline assembly [1] if nothing of the above works). I don't actually want to outsmart the compiler (the volatile trick is only necessary because some GCC versions optimize at the wrong time) but simply make sure the FPU environment is in a specifically defined state. Compilers typically don't care about the FPU environment itself. Anyway, I've spent quite a lot of time documenting different compiler behaviours (with and without optimization) and I am extremely sure that there are no more surprises on x86 platforms. (on other platforms, the macros expand to /* NOP */ anyway) If you want to take a look at my documentation, see: http://www.christian-seiler.de/projekte/fpmath/ I believe I have been thorough enough in my research. [1] Which is simple enough and glibc on Linux basically does the same (just with a more generic approach). Also, I tested the inline assembly with GCC and Sun's CC on OpenSolaris and GCC and Intel's CC on Linux and those worked as expected. Also, PHP already uses inline assembly for certain tasks - just look at the ZEND_SIGNED_MULTIPLY_LONG definition in zend_multiply.h. Finally, my configure checks test if inline assembly actually works (not only if it compiles but actually does the expected). Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: cvs: php-src(PHP_5_3) /ext/standard math.c
Hi, [For conclusive proposal, see below.] 1) Internal FPU precision on x86. Do you have any test cases show the error? Sure. Consider the following C code: #include stdio.h int main (int argc, char **argv) { volatile double v = 100.0; printf (%.35f\n%.35f\n, 0.002877, 2877.0 / v); return 0; } According to the C standard, both values should give 0.0028769..something (the closest double representation of 0.002877). On most x86 platforms without SSE, the above code will give: 0.00287678320119886632256 0.00287721688206786052433 The first is the closest double representation of 0.002877, the second is the closest double-extended representation of 0.002877 trucated back to double. Current PHP (5.2.6) gives me (on my Linux platform): php -r 'printf (%.35f\n%.35f\n, 0.002877, 2877.0 / 100.0);' 0.00287721688206786052433 0.00287721688206786052433 This is due to the fact that PHP has an own implementation of strtod() - which uses double-extended instead of double for the entire internal calculation and just returns a double. PHP 5.2.6 on Windows on the other hand gives me the correct double representation (because MSVC/Windows always sets the FPU into double precision mode anyway). PHP on x86_64 or PHP on x86 Macs (where the compiler defaults to SSE) or PHP on PPC or Sparc etc. also have the correct double representation. Thus, FP semantics of PHP are highly platform and (!) compiler dependent. Worse, since PHP only exposes the double data type, the internal precision of x87 FPUs can never actually be profited from in PHP itself - consider $a * $b / $c. This will first calculate $a * $b (with double-extended precision), truncate that to double and store it in a temp var. Then it will calculate temp var / $c (double-extended) and truncate that. So the extra precision is never actually used within PHP and programmers can never actually take advantage of it (unlike in C where the variables remain in FPU registers by default [2]). On the other hand, having the above situation where the division of two exactly representable (!) FP numbers does not yield the closest FP representation of the actual result is problematic in my eyes. [1] http://www.wrcad.com/linux_numerics.txt [2] And even that is problematic in C, since for example before function calls, FPU registers are stored back to memory and loaded again after the function call returns, so one can't even be sure when extended precision is used and when not. 4) round (1.255, 2) should give 1.26 but gives 1.25. The FUZZ stuff tries to resolve this issue (but not the other three) but I've come to the conclusion that the FUZZ is actually the wrong solution for the problem. The current implementation does return 1.26 Errr, yes, of course. Wrong grammar. I meant »gave 1.25« on certain platforms. Your solution works for the cases discussed here and is certainly much better than the previous code. But it doesn't always work: Take 32769.255 for example. The FUZZ version gives 32769.25 where 32769.26 would be expected behaviour (previous PHP versions were of course never better). The proposed solution seems fairly complex and wide-reaching to me, I am also concerned with the overheads it introduces as that was a problem with many in-C implementation libc folks have tried and rejected. The libc folks didn't have that many problems. C's round() function doesn't accept a precision parameter, thus *always* rounds to integer. This means, if you have a look at my previous points: 1) Internal FPU precision 2) Spec problem (rounding mode) 3) Dividing/multiplying by = 10^23 inexact 4) round(1.255, 2) 1, 3 and 4 immediately disappear because they ONLY applie if you actually have a precision parameter (FPU precision is irrelevant for rounding to integer - if the original number with integer and fraction part could be represented, the integer part alone can also be represented - and no division / multiplication is necessary for integer rounding, so 3 is irrelevant. Since there's no precision parameter, rounding 1.255 to two places with C's round is irrelevant anyway). 2 also disappears since the C 99 standard defines round() to behave like arithmetic rounding (round-to-nearest, round-half-up). On the other hand, PHP has this precision parameter. We can't just drop it. The best thing would probably have been to never add it but we are about 8 years or so late for that. So we have to live with it. My proposal adresses all the four problems. 1 is addressed by the FPU precision macros (my two patches for ZE2 alone), 3 and 4 are also addressed (and my implementation correctly handles the above example I provided) - for implementation details see my RFC. 2 is addressed by the mere fact that I add a third optional parameter to round() that specifies the rounding mode. Thus the user of the round function can decide if he/she wants arithmetic or banker's rounding (and for completeness
Re: [PHP-DEV] namespaces - my RFC
Hi, My proposals are here: http://wiki.php.net/rfc/namespaceref Just for the record: I like both of them, the second one would probably be better to accomodate those who want functions in namespaces. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Could somebody with ZE2 karma please commit this? (was: [RFC] [PATCH] Rounding in PHP)
Hi, @internals: Anyway, since nobody of the core devs seems to be interested in this topic, is there any objection to me committing this patch to HEAD? Since I received no answer, I assumed that nobody would object and tried to commit my patch to HEAD. Problem is that I don't have ZE2 karma and since my patch touches zend_strtod.[ch] too, my commit failed. I isolated the ZE2 parts of the patch so they can be applied separately so that somebody with ZE2 karma can commit it: http://www.christian-seiler.de/temp/php/2008-08-23-rounding/ze2.patch For an explanation for everyone that hasn't read my rounding RFC, the ZE2 part of the patch does the following: - Define some macros in zend_strtod.h that set the FPU control registers to always use double precision (and not double extended precision) when performing calculations. - Add necessary configure checks for Unix based systems to see if the required mechanism is present. - Define the correct constant in the Windows build system (MSVC always defines _controlfp_s) - Modify zend_strtod() that it uses these macros. The macros are defined in zend_strtod.h because I also need them in ext/standard/math.c. The reason for the need to fix zend_strtod() is that it currently behaves like strtod() on most systems but like (double)strtold() on x86 UNIX with GCC (see my RFC for details why that's not the same). For the record, the complete patch I tried to commit (I'll commit the rest myself): http://www.christian-seiler.de/temp/php/2008-08-23-rounding/rounding-head.patch I'd really appreciate it if someone could commit the ZE2 parts. Thanks! Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [PATCH] Rounding in PHP
Hi, For the record, I was very excited to see your proposal and the work you've done. Thanks! :) I'm not a PHP dev so I can't comment on the patch, maybe there might be a performance concern? The new algorithm is slightly slower in most of the cases and quite a bit slower if strings have to be used in order to ensure correctness. The following table shows the performance comparison of the algorithm when compiling 5.3 with gcc 4.1.2 on Gentoo Linux with and without optimization activated: +++---+---+ ||| without patch | with patch| | st | pr | -O0 -ggdb | -O2 | -O0 -ggdb | -O2 | +++---+---+---+---+ | - | X | 6.427930 | 3.151313 | 7.107897 | 3.581429 | | X | X | 6.485305 | 3.197008 | 26.423409 | 15.571503 | | X | - | 6.318853 | 3.124304 | 11.926427 | 6.910457 | | - | - | 6.100793 | 2.850991 | 6.636300 | 2.926919 | | precov. | 5.432411 | 2.382959 | 5.524717 | 2.382117 | +++---+---+---+---+ Times are in seconds, script was basically the following: $start = microtime(true); for ($i = 0; $i 500; $i++) $ret = round (/*number*/, /*prec*/); $end = microtime(true); printf(%.6f\n, $end - $start); Explanations for the left columns: st: Uses to-string-and-back conversion to ensure correctness (the case when |places| 22) pr: Uses prerounding to precision to ensure correctness (see RFC for details) precov: Precision overflow, i.e. rounding 12345.123456789012345 to more than 10 places precision (which is the max. that doubles can represent) The most commen case by far would be the first one (things such as round(12.245, 2)), the cases where string-conversion is used are probably extremely rare (remember, |second parameter of round| must be 22). I used the following numbers to test the combinations: -st +pr: round (0.116, 2) +st +pr: round (0.00116, 32) +st -pr: round (0.5, 30) -st -pr: round (0.005, 0) precov.: round (1, 30) As you can see, the cases which don't to string-and-back conversion are not problematic: There's basically no difference for the precision overflow case, the case with prerounding is slightly slower and the case without prerounding is actually slightly faster when compiler optimizations are enabled (this is due to the fact that the 10^places was optimized in my patch). So the most common cases basically don't matter much at all. The only potential problems are the cases where the second parameter of the round() function is larger than 22 or smaller than -22. In that case, the new algorithm is quite a bit slower than the old one (nearly factor 5 for all numbers that won't become 0 after rounding). But these are edge cases which won't happen that often and if people actually have these numbers they are probably far more interested in correctness than in performance [see for example var_dump(2e-23 - round(2e-23,23));]. snip -- @internals: Anyway, since nobody of the core devs seems to be interested in this topic, is there any objection to me committing this patch to HEAD? @Johannes, @Lukas: Should there be a decision for an alpha3 release of PHP 5.3 due to the namespace discussion, is there any objection to me committing this patch to PHP_5_3? Should there be directly a beta1 release, any objections against this patch in 5.3.1? Or, even though it is a rewrite of the round() function, may this even qualify as a bugfix, since the previous behaviour was erroneous (see for example http://bugs.php.net/bug.php?id=42294 and my RFC)? Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC] [PATCH] Rounding in PHP
Hi, A few weeks ago I wrote quite a long posting to internals@ that tried to clarify the situation on the round() function in PHP. I was asked to write it up as an RFC in the wiki, which I have done: http://wiki.php.net/rfc/rounding In the mean time, I had some time to think about that problem again and am quite sure that I have a feasible solution for the problem. The RFC includes a patch that addresses all problems and is in my eyes the best solution. The patch is actually much simpler than the RFC itself. ;-) I also discovered that zend_strtod() which is responsible for string to double conversions does not yield the same results as the traditional strtod() due to the same extended precision truncated to double precision problem on 32 bit x86 systems with GNU compiler. The patch also addresses that. Anyway, feel free to comment. Regards, Christian PS: While testing the Windows versions, I discovered, that someone removed http://www.php.net/extras/win32build.zip - is that intentional? I got the previous version out of CVS in order to be able to build PHP. If it is intentional, where do I get updated build instruction when compared to http://www.php.net/manual/en/install.windows.building.php? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Closures and reflection improvements
Hi, all submitted now. Thanks everyone for their help and especially Christian for getting this started. Thanks for making my patch work after the modifications in zend_closures.c. I'm sorry I couldn't do it myself but I was kind of busy the last few days and the simple fix created memory leaks and I didn't want to post a knowingly broken patch. Anyway, there are still segfaults in the current version: $foo = function ($a) {}; $o = new ReflectionMethod ($foo); $p = $o-getParameters (); unset ($o); $m = $p[0]-getDeclaringFunction (); Reflection::export ($m); $foo = function ($a) {}; $p = new ReflectionParameter ($foo, 'a'); $m = $p-getDeclaringFunction (); unset ($p); Reflection::export ($m); This is due to the fact that getParameters() / getDeclaringFunction() don't copy the fptr and fptr-common.function_name pointers if it's a closure. Here are patches for PHP 5.3 and HEAD that fix the problem: http://www.christian-seiler.de/temp/php/2008-08-11-reflection/segfaults-5.3.patch http://www.christian-seiler.de/temp/php/2008-08-11-reflection/segfaults-6.patch Also, the following test that I already posted should be added to PHP_5_2, PHP_5_3 and HEAD: http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflectionParameter_invalidMethodInConstructor.phpt The test already works for PHP_5_3 (since the fix for that was included in my original patch) but a segfault still occurs in PHP_5_2 and HEAD, my patch that I already posted fixes that problem for 5.2: http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-segfault-fix-5.2.patch My original patch also fixed the problem for HEAD, I readded that part to the above segfault patch for HEAD. Also, it appears you do not have a cvs account, or did I overlook something? No, I don't. Dmitry also mentioned that - should I request one? Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] CVS Account Request: cseiler
Maintaining closures, see http://news.php.net/php.internals/39835 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Closures and reflection improvements
Hi Marcus, Account approved and access granted. Thanks! Please provide the function copying in a function next to _free_function() I just committed that (and included some tests). I hope I didn't do anything wrong in the process, if so, feel free to do so. and care for parameter info as well I assume with parameter info you mean arg_info? That isn't necessary and actually harmful since the arg_info is not copied but simply referenced via a C pointer in closures (zend_get_closure_invoke_method copies the entire function structure, including the pointer to arg_info and only changes some flags the function name). There is no need to copy or destroy arg_info in reflection. Also for your next patches (cvs di -N) allows to ship the new files in the patch and (cvs di -Np) shows the context, which makes reading the patches easier. Ah, thanks, I added that to my .cvsrc. Regards, Christan -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Closures and reflection improvements
Hi Dmitry, Hi Marcus, I have no objections against ZE part of the patch. If you like ext/reflection part please commit the whole patch. Due to your cleanup wrt. handlers the reflection part will currently (probably) segfault. I'll post an updated patch for this tomorrow. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Closures and reflection improvements
Hi, http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-5.3.patch http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-6.patch The last CVS commit for apply_func_t and TSRMLS_CC conflicted with the patches, I merged the conflicting line manually and reuploaded the patches to the same location. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] Closures and reflection improvements
Hi Marcus, patch looks fine and should go in. Thanks. do you think ReflectionMethod::__construct implementation could be done using parameterparsing 'f' rather than the spcial case 'o'? The Problem with 'f' is that it will accept every callback, even normal functions, so that would kind of break the idea of having ReflectionMethod different than ReflectionFunction. So in my eyes, using 'o' for now is the sanest approach. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] question about backward-compatibility break/bug in php 5.2.6
Hi, Last week I submitted a bug report on the issue described below. The response (also below) was that this is not a bug. I fail to see how it could *not* be a bug given that strtotime is parsing an invalid date into a seemingly-arbitrary and definitely-meaningless number, strtotime() has always accepted month and day numbers 0 in order to express last month of the previos year and last day of the previous month. Take: var_dump (date ('Y-m-d H:i:s', strtotime ('2008-00-01 12:00:00'))); This gives: string(19) 2007-12-01 12:00:00 Here, the 00 is interpreted as month before January and thus December of the previous year. Then, take: var_dump (date ('Y-m-d H:i:s', strtotime ('2008-02-00 12:00:00'))); This gives: string(19) 2008-01-31 12:00:00 Here, the 00 is interpreted as day before the first day of that month and thus the 31st of January. Take both together, you get: var_dump (date ('Y-m-d H:i:s', strtotime ('2008-00-00 12:00:00'))); string(19) 2007-11-30 12:00:00 Now, take the date '-01-01'. This is a valid date according to ISO 8601 and simply means the 1st of January in 1 BC (but according to a proleptic gregorian calendar, because ISO 8601 defines it as that). So '-00-00' is actually the 30th of November of 2 BC (proleptic gregorian calendar). Now, the return value of strtotime() is actually defined as the number of seconds since the 1st of January 1970, 00:00:00 UTC. If you add -62169966000 seconds to that date, you get the 30th of November 2 BC (proleptic gregorian calendar) at 00:00:00 of your time zone. Also, it is not a regression. On my 32bit CPU, strtotime() still returns false for your date since a 32bit integer cannot represent such a large number and thus strtotime() can't return a valid timestamp. But on a 64bit CPU, integers are large enough to hold that number and strtotime() can return a valid timestamp. If you compile a very old PHP version on a 64bit system, it will yield the same results. As somebody already commented in the bug reports: It's not a bug. You can write yourself a wrapper if you want to catch that specific date: function my_strtotime($str, $ts = null) { if ($ts === null) $ts = time (); return $str == '-00-00 00:00:00' ? false : strtotime ($str, $ts); } Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] closures questions
Hi! so do we even want the toString() method? IMHO we should drop toString from Closure. There is one case which *may* cause problems: function doSomething ($callback) { if (!is_callable ($callback)) { throw new Exception (...); } // special treatment if ($callback == 'default') { ... } else { ... } } Here, the comparison of $callback with 'default' will yield a notice that $callback couldn't be converted. The comparison will fail correctly, of course, but it could cause some confusion. The correct way of comparing would obviously be using ===, that won't cause these hickups. Personally, I don't care whether a object-to-string cast is present or not. I don't think Closure can be meaningfully exported. Can we prohibit it? Unfortunately, currently not, see the var_export code. I think we can make working clone there - just copy all data, etc. Perhaps. But I'd recommend this should be postponed, as there are quite a few subtleties regarding semantics of bound variables involved. Should there seem to be a strong need for cloning closures by the community, this can always be implemented in a future PHP version. IMHO. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Php 5.3 Snap - Lambda functions and $this scope
Hi, Wouldn't it be better (and maybe safer) to allow the use of $this as a closure instead of passing it to the new lambda function? We had that in a previous patch. We had quite a few discussions on what the best syntax would be, actually. In the end, Dmitry and I chose to implement it in this way because it seemed more consistent with current PHP behaviour and because more people on internals seemed to like it than dislike it. As for the consistency with current PHP behaviour: $this is always present in any class method which is non-static. Every other variable (except superglobals) used in a method or function must explicitly be defined - either as a parameter, or in the function or via 'global $foo'. So $this is already special. So using the presence or absence of the 'static' keyword in front of the 'function' keyword to determine whether $this is available inside the closure seems much more natural. Of course, there will be people who disagree with this. But we've already discussed closure syntax quite a bit while discussion my original patch and I believe the compromise we've reached is quite acceptable for everybody. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] Closures and reflection improvements
Hi, I'd like to add the following patch to reflection before the feature freeze, because it does not change any current reflection behaviour but adds quite a bit of good stuff for closures. With the current closure implementation, __invoke always has the same signature as the original lambda function, this makes passing parameters per reference possible. But this does not show in current reflection: $o = new ReflectionObject (function ($a) {}); $m = $o-getMethod ('__invoke'); var_dump ($m-getNumberOfParameters()); Currently, this does a lookup in the class function table. But the signature of the method found in the function table has no parameters. For the normal engine to have a different signature for a specific closure, the get_method handler is used. My patch does the following: * If a reflection method named '__invoke' is requested for an object of the 'Closure' class (in any way possible: via constructor, via ReflectionObject-getMethod, via ReflectionParameter constructor, ...), the correct invoke signature is used by reflection. * If the same method for the class 'Closure' (without an object) is requested, the standard signautre (no parameters) will be used. * If you pass a callable object (closure or otherwise) as the only parameter to the ReflectionMethod constructor, __invoke will be used as default method. Example: $m = new ReflectionMethod ($closure); is identical to $m = new ReflectionMethod ($closure, '__invoke'); For Post-5.3 we could even think of adding a new object handler for retrieving methods for reflection generally (get_method will not work correctly since the std_get_method handler does scope checking and prints error messages) and thus allowing e.g. COM, Java or SOAP objects to provide useful dynamic information for reflection. But to keep the changes in 5.3 as non-invasive as possible, it is probably best to have this single exception for closures. You can find the patch here: http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-5.3.patch http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-6.patch The patch is fairly straight-forward and should be easily reviewed. Here are some tests for this patch: http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-tests-5.3.tar.gz http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-tests-6.tar.gz I also found a segfault in *all* PHP versions in the ReflectionParameter constructor. The above patches already fix that, but here's a patch that fixes this for PHP 5.2: http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-segfault-fix-5.2.patch For the segfault I also created following test case (works with 5.2, 5.3 and 6.0), NOT included in the above .tar.gzs. http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflectionParameter_invalidMethodInConstructor.phpt Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] question about backward-compatibility break/bug in php 5.2.6
Hi! This 64-bit machine is running 5.2.5 and returns false on the all-zero datetime string. It was upgraded last week to 5.2.6 and started returning the large negative integer. Ah, yes, the change responsible is the following: http://cvs.php.net/viewvc.cgi/php-src/ext/date/lib/timelib.h?r1=1.10.2.11.2.4r2=1.10.2.11.2.5pathrev=PHP_5_2 It fixes bug #44209 http://bugs.php.net/bug.php?id=44209. timelib.h didn't include limits.h and thus always defined LONG_MAX / LONG_MIN to the 32bit values for longs and thus the check in timelib.c for LONG_MIN failed in 5.2.5 for your timestamp. But as 5.2.6 fixed that (LONG_MIN from limits.h used when present), your timestamp became valid and strtotime() returned it. Or to put it that way: The current behaviour of PHP 5.2.6 is the expected behaviour (at least what reading the source tells about the author's intentions) and it was broken before. As to whether it's a good idea that strtotime() accepts invalid dates, I'll stay out of that discussion. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] closures questions
Hi Lukas, 1) Closures on class properties just don't work, the only way to do it is to do something like: $c = $a-b; $c(); Calling: $a-b(); will result in method A::B() does not exists. Yes, that's expected behaviour (we had a few comments on this on the list). Compare this to, for example: function hello () { echo Hello World!\n; } $a-b = 'hello'; $c = $a-b; $a-b (); // undefined method $c (); // works But, as closures implement the __invoke method, it's possible to do this: $a-b-__invoke (); or, of course call_user_func ($a-b); (which works with all types of callbacks, even non-closures) so do we even want the toString() method? Personally, I don't care. 4) var_export() and Closures is useless What would you suggest? Having var_export return the function body for the closures? But what about bound variables? Well, perhaps in the future you could even export all the bound variables. But for know to keep it simple I'd say it's best that closures can't be exported. 5) Its impossible to clone a closure using the cloning keyword: $a = function(){}; $b = clone $a; That's intended behaviour. What would be the clone of a closure? Especially with bound variables? This would allow for all sorts of weird behaviour. Take, for example, the following: class Foo { private $someProperty; function getPrinter ($outputDevice) { return function ($text) use ($outputDevice) { $outputDevice-print ($this-someProperty, $text); }; } } How would you clone that? There are two bound variables in this closure: $this and $outputDevice. Do you clone them both? Do you clone only $this? Do you clone only $outputDevice? If you only clone the closure object itself, it won't change the behaviour from simply creating another reference to it... If one *really* needs a clonable *and* callable object, write a class that implements __clone and __invoke. Then the programmer can control the exact semantics. Everything else would simply be extremely confusing. And, if I take another language such as Python for example, there you can't clone closures either, y = copy.deepcopy (closure) returns the same object. This makes it hard to make a copy of the closure because of objects always are passed by reference. I guess this is a draw back from the OO approach (rather than the original resource approach), but solvable? No, the original resource approach was just the same. A resource is basically an integer value which is then used to look up some specific data. You can't clone a resource either. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Rounding in PHP and floating point numbers in general
Hi! Note too that the value actually stored in $f differs from that we may expect simply reading the source: the difference is very small, but it exists. Float values can always be converted back in decimal base with exact precision, so for example in our case the $f variable now contains exactly $f == 0.155511151231257827021181583404541015625(dec) No surprise on that: the error was introduced by the PHP parser itself when the statement $f = 0.1; was encountered the first time. Yes, sure. But IEEE 754 guarantees that the first 15 significant decimal digits are correct (for double precision anyway), and in your example, the first 17 significant decimal digits are in fact correct. So, if you always round to the first 15 significant decimal digits when outputting the floats, you always get correct results within that decimal precision. All these dec-to-bin conversions and float roundings sometimes bring to unexpected results as already pointed out in the 42294 bug: we (humans) continue to think at numbers like 0.285 as it they were decimals, but these numbers are actually handled in a completely different base inside the computer. Look it from this perspective: If you do echo 0.285 in current PHP, it will output 0.285. This is due to the 'precision' ini setting (default is 14) that implicitly rounds the result. And within those 14 decimal digits, the results ARE guaranteed to be exact by IEEE 754. The only real bug is the round() function itself, and number_format() or printf() should be used instead, because these function first convert the binary float to decimal, and then perform the rounding over the decimal number. Not entirely true. ;-) number_format() uses round internally (math.c, PHP 5.3 branch, line 944). And, sprintf (%.2f, 0.285) == 0.28 btw. - because even if you convert the float back to a full decimal number, you get 0.28499...something, so converting to string and rounding then doesn't work either (at least not as a user would expect). This is the reason why C for example doesn't bother to provide a round() function for arbitrary precision, it can only round to integer which always can be represented in an exact way. If we really want a round() function, this function should operate just like number_format() and printf() already do, i.e. returning a string, not a double, The problem is not returning a double. For example, literal 0.285 in the source and the result of the expression round (0.2846, 3) are the same floats. They are not exactly 0.285 but they are represented in the same way (assuming the FPU mode thing gets added, see my first mail). The actual problem is that the double you put in to round() is not always what people think it is (as you already said). So letting round() return a string() will not solve the underlying issues. A final note: programmers concerned with rounding problems and looking for exact results while dealing with floating point numbers are usually erroneusly using float values to store monetary values as if PHP were a desk calculator, rather than use BCMath or other specialized high-precision library suitable for business applications. The problem is that bcmath is not available everywhere - neither are other such libraries. Also, using bcmath is quite cumberstone to read. With floats you can do $a = $b + $c * $d + $e / $f - $g * $h / $j which is very clear to read while with bcmath you have lots of nested function calls which is really bad to read. And even if you have monetary calculations: Normally, you are WELL within 14 decimal digits precision. You also have to consider the following: If this were a proposal to introduce a precision parameter into the round function, I would completely agree that adding it will cause unecessary problems and should not be done. But the fact is that it is already there in PHP and you can't remove it with some serious BC breaks. And people are using it. So may proposal is to make sure people get the best results possible. (and since my proposal leaves some key issues open, what the best results possible are is open to discussion, cf. fuzz vs. no-fuzz etc.) Anyway, I'll put my proposal into the wiki tomorrow with some additional thoughts, perhaps my intentions are clearer then. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Rounding in PHP and floating point numbers in general
Hi, With this posting I'd like to make another try in resolving the rounding issues in PHP. This is basically in response to http://bugs.php.net/bug.php?id=42294 (bug #42294) where I added a comment last year. Since then I have read quite a bit on this subject so I feel that I should post an update to my previous comment (which isn't completely correct, as are some other things I reference in this mail, please keep that in mind while reading them). And since the bug seems to have been forgotten, I think I should try this on the mailing list. A) General explanations === Rounding methods: - Round to negative infinity: Should be obvious, some examples: -0.5 - -1 0.5 - 0 2.4 - 2 -3.6 - -4 4.8 - 4 This is what the floor() function does. - Round to positive infinity: Obvious, some examples: -0.5 - 0 [actually, -0] 0.5 - 1 2.4 - 3 -3.6 - -3 4.8 - 5 This is what the ceil() function does. - Round to zero: Some examples: -0.5 - 0 [actually, -0] 0.5 - 0 2.4 - 2 -3.6 - -3 4.8 - 4 - Round away from zero: Some examples: -0.5 - -1 0.5 - 1 2.4 - 3 -3.6 - -4 4.8 - 5 - Round no nearest: Rounds to nearest integer. Examples not involving .5: 2.4 - 2 -3.2 - -3 -3.6 - -4 4.8 - 5 2.501 - 3 -2.501 - -3 [Note the last two examples: they are not exactly .5 so they do not have the equal distance from both integers so the rounding direction in this case is clear.] There are four main variants of this rounding method regarding the treatment of .5 (which is exactly in the middle of two integers): * Away from zero: .5 will always be rounded to the next integer away from zero: -1.5 - -2 1.5 - 2 -2.5 - -3 2.5 - 3 This is the traditional arithmetic rounding people learn in school. * Towards zero: .5 will always be rounded to the next integer towards zero: -1.5 - -1 1.5 - 1 -2.5 - -2 2.5 - 2 * Towards even: .5 will always be rounded to the next even integer: -1.5 - -2 1.5 - 2 -2.5 - -2 2.5 - 2 This is the so-called banker's rouding. * Towards odd: .5 will always be rounded to the next odd integer: -1.5 - -1 1.5 - 1 -2.5 - -3 2.5 - 3 In practice, only round to negative/positive infinity (floor/ceil), arithmetic rounding (round to nearest, away from zero) and banker's rouding (round to nearest, towards odd) are widely used. B) Rounding occurs in = There are various places in PHP where rounding (explicit or implicit) occurs: Explicitely: * round() * number_format() * (sf)printf() with %3f etc. Implicitely: * float to string conversion (the 'precision' ini setting) * (sf)printf() with %g The problem is that different methods are used for this rounding. C) History of the round() function in PHP = First version in CVS (math.c): - No precision argument, always rounds to integer - Use rint() for rounding ISO C says: rint() uses the current rounding direction which on every machine known to me is round to even (so-called banker's rounding). But: Rounding direction may be changed by fesetround()! - On systems where rint() is not specified, rint() is emulated. But the emulation is done via an algorithm that does arithmetic rounding. Version 1.22 (May 17, 2000): - Allow arbitrary precision rounding, now implement an algorithm that does arithmetic rounding. Version 1.104 (Aug 8, 2003): - Add fuzz due to incorrect results (see below) Version 1.106 (Aug 9, 2003): - Disable fuzz on Win32, make (useless) configure check on UNIX July 23, 2004: (Slightly flawed) analysis of the FP and rounding situation in PHP and proposal to fix that on the internals mailinglist: http://marc.info/?l=php-internalsm=109057070829170w=2 The linked C files are still available through the wayback machine via http://web.archive.org/ to get an impression what George actually meant. D) Problem analysis === In this section I want to provide an analysis of the problems that occur with PHPs rounding functionality. First of all, rounding to integer is never a problem as long as the number of digits before the decimal point is smaller than the floating point precision. But does anyone really want to round something to 15 significant (!) digits precision and still use floats? I believe not. So, if you want to do
Re: [PHP-DEV] [RFC] Closures: updated proposal and patch
Hi, 1) The RFC page says that closures pass by value by default. Although it is not stated, am I correct in saying that due to the way resources and objects (and presumably therefore lambdas) are handled they will still have the effect of passing by reference anyway, just as with a function parameter? Yes, of course. :) 2) It is unclear from the RFC if static class closures (I am not fond of that syntax, I grant) are the only ones that won't import $this. Is the suggested detection optimization not included, and therefore it's up to the programmer to avoid dangling references that keep stray classes around? It's up to the programmer for now, static = no $this, no static = $this. 3) Can __invoke() accept parameters, Yes. 4) The ability to reflect and then call a function or method as a closure sounds highly nifty. However, I am unclear on why it is necessary to pass in the $this to use. In that case, wouldn't it make more sense to use ReflectionObject() in the first place so that the $this is implicitly known? Probably. But if we start picking on every detail of the patch until it is absolutely perfect, it will never get committed, not even to HEAD. So I propose that it should be committed to HEAD (as long as there are no major objections) and then worry about tiny details afterwards. 5) There's a couple spelling and grammar errors. :-) Feel free to correct them if you have access to the wiki. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC] Closures: updated proposal and patch
Hi, After some discussion with Dmitry, he and I have continued to improve the current closure patch. You can find the current proposal with patches for 5_3 and HEAD here: http://wiki.php.net/rfc/closures (Please read it again, I've changed quite a lot.) Basically, it's the syntax with use ($lexical) in the function declaration, optional references, optional static keyword for functions that don't need $this and support for __invoke. I know that there was some disagreement on the syntax (including by me) but I think this is the best compromise we can achieve while still staying consistent with PHPs current semantics and not breaking BC at all. I've spoken to Dmitry and he said the patch will be committed to HEAD soon. Since both Dmitry and I still want to have it in 5_3 too, we'd want to ask for opinions on this again - especially since after quite a lot of thorough review and discussion on this list basically all the side-effects have been addressed and there are now quite a few tests that ensure the correct behaviour of closures. Also, the patch is now built in a way that the main functionality remains inside zend_closures.c, so any possible not yet encountered bug can be fixed without breaking binary compability. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Closures: updated proposal and patch
Hi Stanislav, Or did you just mean $example-setSearch() and I'm worried about nothing? :) Yes, absolutely, Sorry for the confusion caused. ;-) I fixed that in the Wiki. In this case, I'd just propose to have getThis() anyway. I don't see a need, but I'm not against it. Should be extremely trivial to implement. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi Dmitry, I'm fine if you'll improve my patch (It's mainly yours :) I updated my closures RFC: http://wiki.php.net/rfc/closures I have based my new version of the patch on yours (Dmitry), but I made some changes to that: * Objects instead of resources are used, two new files zend_closures.[ch] are added where the new Closure class is defined. Currently, it contains a dummy __toString method that in future may be extended to provide enhanced debugging info, also further additional cool stuff could be added to such a class later on. But I prefer to only add the basic closure functionality at first - you can always extend it once it's there. * I have *not* added any __invoke() magic to normal objects. This is mainly due to the simple reason that adding that would not help a closure implementation at all. Closures need some engine internal magic (use a dynamically created op_array instead of looking one up, setting the correct class scope and setting the correct EG(this). And as I said: I want to stick with the closure basics for now. That said, I do like the possibility of invoking objects directly, so I suggest someone created an additional proposal for that? * I've added a patch for PHP HEAD (PHP 6.0). This is due to the fact that Dmitry's variant of my patch has far less intersections with the unicode functionality than my original patch, so it was quite straight-forward to do so. * Lexical vars are now copied instead of referenced by default. Using in front of the var, the behaviour may be changed. I added that in order to demonstrate that both was possible and that a simply change of grammar suffices. In my eyes this is the main issue where a discussion has to take place (i.e. copy or reference by default? possibility to change default via syntax? which lexical syntax?) before the proposal can be accepted. * I provided patches for both lexical $var and use ($var) syntaxes. * I provided a patch variant that only stores $this if $this is explicitely used inside a closure (or a nested closure of that closure). This works since it is possible to detect whether $this is used at compile time. For this, I have added a this_used flag to the op_array structure. * I added tests (Zend/tests/closures_*.phpt) that ensure the correct behaviour of closures. [Note that I created my own local SVN repos for developing these patches because I was fed up with CVS's inability to local diffs and locally mark files as added to include them in the diffs. Just to explain the format of the patch.] Anyway, feel free to discuss. In my eyes, the following questions should be answered: * Do you want closures in PHP? I have not seen a single negative reaction to my proposal, so I assume the answer to that is yes. ;-) * Which syntax should be used for lexical variables? Should references or copies be created by default? This is far trickier. First of all: There must *always* be the _possiblity_ to create references, else you can't call it closures. Second: I prefer the 'lexical' keyword, but I could live with the use solution [function () use ($...)]. Third: There are several arguments against default referencing: * (use syntax) As they look similar to parameters and normal parameters aren't passed by ref, this could be quite odd. * Loop index WTFs (see proposal) * Speed? (You always read that refs are slower in PHP than normal variable copies. Is that actually true?) * While it is possible to simply add an in front of the variable name to switch to refs in no refs default mode, there is no obvious syntax to use copies in refs default mode other than unsetting the variable in the parent scope immediately after closure creation. Fourth: There are several arguments for default referencing: * (lexical syntax) global also creates a reference, why shouldn't lexical? * Other languages *appear* to exhibit a very similar behaviour to PHP if PHP created references. This is due to the fact that other languages have a different concept of scope as PHP does. Although the list of against arguments appears to be longer, I do prefer using references by default nevertheless. But that's just my personal opinion. * Are you OK with the change that $this is only stored when needed? I don't see a problem. Dmitry seems to be very touchy (;-)) about changing op_arrays but in this case it's only a flag so I don't see a problem for opcode caches (in contrast to a HashTable where the opcode cache must actually add code to duplicate that table). * Do you want closures in PHP 5.3? Since the majority of core developers appear to be against it, I presume the answer is no. I will provide a revised patch that incorporates the results of
Re: [PHP-DEV] Using Network functions.
Hi! 2. What does streams has to do with this networking? Can it be done using streams? Yes, it can. Take the following simple, incomplete example without much error handling as a starting point: // in the header #include php_streams.h // PHP function: PHP_FUNCTION(simplehttp_fetch) { php_stream *stream; struct timeval tv; int err; char *errstr; char buf[8192]; tv.tv_sec = 30; tv.tv_usec = 0; stream = php_stream_xport_create(tcp://www.google.com:80, sizeof(tcp://www.google.com:80)-1, ENFORCE_SAFE_MODE | REPORT_ERRORS, TREAM_XPORT_CLIENT | STREAM_XPORT_CONNECT, NULL, tv, NULL, errstr, err); if (stream == NULL) { php_error(E_WARNING TSRMLS_CC, unable to connect to tcp://www.google.com:80: %s, errstr == NULL ? unknown error : errstr); RETURN_NULL(); } php_stream_write(stream, GET / HTTP/1.0\r\nHost: www.google.com\r\n\r\n, sizeof(GET / HTTP/1.0\r\nHost: www.google.com\r\n\r\n)-1); memset(buf, 0, 8192); php_stream_read(stream, buf, 8191); php_stream_close(stream); RETURN_STRING(buf, 1); } Basically, you want to look at main/php_streams.h and main/streams/php_stream_transport.h in the PHP source code. Userland fsockopen() essentially maps to php_stream_xport_create() and userland fread, fwrite, close, etc. essentially map to php_stream_read(), php_stream_write, php_stream_close, etc. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi Marcus, I like the new ability to reference if wanted. But then I don't like references at all. As I said: Without reference support, you can't call it closures. Closures must per definition have the possibility to change the values of used variables in the parent scope - and the only sensible PHP way to do that is references. If people here say: We want copy as default and references optional, that's fine with me since real closures can still be achieved and - let's face it - since PHP wasn't designed with closures in mind, the syntax will always be not 100% perfect, regardsless of how we'll do it. But I'm stronly against removing reference support entirely. * Are you OK with the change that $this is only stored when needed? I don't see a problem. Dmitry seems to be very touchy (;-)) about changing op_arrays but in this case it's only a flag so I don't see a problem for opcode caches (in contrast to a HashTable where the opcode cache must actually add code to duplicate that table). I see it dangerous eval comes to mind. Certain things don't work anyway as of now, see bug http://bugs.php.net/bug.php?id=43163 for example... And that's intended behaviour. Ok, eval() inside a closure still is a possible problem but we still could say ok, as soon as eval() appears, just assume $this is used. That will benefit those people who use neither $this nor eval() inside a closure with an optimization ($this will be GCed and not copied). And as far as I can see, eval() is the only thing you can do inside a normal class method that will allow you to access $this without the compiler knowing about it. But thanks for pointing eval out, I hadn't thought of that. And also, why create something special when the normal way of doing things that is done everywhere else would work too. What do you mean by that? Comments on the first patch version: - col, I am the listed author: Zend/zend_closures.c/h Oh, yeah, I just copied some header I found elsewhere. Should I put my name there or what's the policy for contributions? - you shouldn't be having a __destruct. Can you prevent that? But if I don't have a destructor, how do I garbage collect the op_array? - please drop __toString, with the new behavior only stuff that has something to say in a string context should have a __toString Somebody on the internals list suggested adding it to print out useful debugging info. But I'm fine with dropping it until there is actually a useful implementation. - a tiny optimization: +ZEND_API zend_closure *zend_get_closure(zval *obj TSRMLS_DC) /* {{{ */ +{ + zend_class_entry *ce = Z_OBJCE_P(obj); + if (instanceof_function(ce, zend_ce_closure TSRMLS_CC)) { + zend_closure *closure = (zend_closure *)zend_object_store_get_object(obj); + if (closure-initialized) return closure; + } + return NULL; +} Yes, good idea. - a faster way would be to: a) add a new type (probably not so good though) b) add a new ce_flag +#define ZEND_ACC_CLOSURE ... I'm quite indifferent to that change. On the one hand, it's a performance optimization, on the other hand, it digs deeper into current Zend code. - maybe even inline this function? Yes, good idea. As stated before, I'll keep all changes in mind and post and updated patch as soon as the discussion is done. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi! I see exactly one problem with the patch, which is that the above script shouldn't work without use ($i). I find it counterintuitive that the creation of the lambda creates a copy of $i, but all invocations of $lambda use a reference to the same $i. For n calls to $lambda, there are only 2 copies of $i (one global, one static in $lambda) where I would expect n+1 copies. Yes, you're right. My solution for this would be: -- zend_compile.h - -void zend_do_fetch_static_variable(znode *varname, znode *static_assignment, int fetch_type TSRMLS_DC); +void zend_do_fetch_static_variable(znode *varname, znode *static_assignment, int fetch_type, int as_ref TSRMLS_DC); -- zend_compile.h - -- zend_compile.c - # in zend_do_fetch_lexical_variable: -zend_do_fetch_static_variable(varname, value, ZEND_FETCH_STATIC TSRMLS_CC); +zend_do_fetch_static_variable(varname, value, ZEND_FETCH_STATIC, is_ref TSRMLS_CC); ... -void zend_do_fetch_static_variable(znode *varname, znode *static_assignment, int fetch_type TSRMLS_DC) +void zend_do_fetch_static_variable(znode *varname, znode *static_assignment, int fetch_type, int as_ref TSRMLS_DC) ... - zend_do_assign_ref(NULL, lval, result TSRMLS_CC); + if (as_ref) { + zend_do_assign_ref(NULL, lval, result TSRMLS_CC); + } else { + zend_do_assign(NULL, lval, result TSRMLS_CC); + } # and make sure zend_do_assign can live with NULL for first param -- zend_compile.c - -- zend_language_parser.y - static_var_list: - static_var_list ',' T_VARIABLE { zend_do_fetch_static_variable($3, NULL, ZEND_FETCH_STATIC TSRMLS_CC); } - | static_var_list ',' T_VARIABLE '=' static_scalar { zend_do_fetch_static_variable($3, $5, ZEND_FETCH_STATIC TSRMLS_CC); } - | T_VARIABLE { zend_do_fetch_static_variable($1, NULL, ZEND_FETCH_STATIC TSRMLS_CC); } - | T_VARIABLE '=' static_scalar { zend_do_fetch_static_variable($1, $3, ZEND_FETCH_STATIC TSRMLS_CC); } + static_var_list ',' T_VARIABLE { zend_do_fetch_static_variable($3, NULL, ZEND_FETCH_STATIC, 1 TSRMLS_CC); } + | static_var_list ',' T_VARIABLE '=' static_scalar { zend_do_fetch_static_variable($3, $5, ZEND_FETCH_STATIC, 1 TSRMLS_CC); } + | T_VARIABLE { zend_do_fetch_static_variable($1, NULL, ZEND_FETCH_STATIC, 1 TSRMLS_CC); } + | T_VARIABLE '=' static_scalar { zend_do_fetch_static_variable($1, $3, ZEND_FETCH_STATIC, 1 TSRMLS_CC); } ; -- zend_language_parser.y - Any objections (in case copies are wanted at all)? Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi! Since some raised issues with the word lexical, what do people think to just re-use the (afaik deprecated) var keyword, so we won't need a new keyword in the chain. That would be quite confusing IMHO, since JavaScript uses 'var' for the exact opposite - to declare variables that are local and thus *not* taken from the parent scope. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi Dmitry, First of all: Your patch does really simplify things internally quite a bit - I like it. I have a few issues though: The patch shouldn't affect opcode caches and other extensions as it doesn't change any structures. I don't see a problem in changing structures for either extensions nor opcode caches - as long as only entries are added. Binary compability with PHP 5.2 is not provided anyway (by neither 5.3 nor 6) and source compability is not affected if the old members are not touched or their semantics change. It uses the op_array-static_variables for lexical variables. That's a point I don't like. Although you use IS_CONSTANT to cleverly mask lexical variables, I really think a separate hash table would be a far better idea, especially for code maintainability. The patch also fixes several small issues and adds some missing functionality which didn't allow preg_replace_callback() (and may be others) to work with lambda functions. Oh yes, I somehow missed that, thanks! Please review. I (personally) have some smaller issues with the patch and one big issue: Smaller issues: * A separate hash table for the lexical variables would be much cleaner in my eyes. * The segfault that occurs with my patch still occurs with yours (see below for an example) But the one big issue is the syntax: ($foo | $bar) is just extremely painful in my eyes. I wouldn't want to use it - and it would be quite confusing (which side are the normal parameters, which side are the lexical vars?). I do see your point that the 'lexical' keyword inside the function body to actually have an effect on the function semantics is not optimal and that the list of lexical variables is probably better placed in the function definition. I therefore propose the following syntax: function (parameters) { } // no closure, simply lambda function (parameters) KEYWORD (lexical) { } // closure with lexical vars KEYWORD could be for example 'use'. That probably describes best what the function does: Use/import those variables from the current scope. Example: return function ($x) use ($s) { static $n = 0; $n++; $s = $n.':'.$s; $this-foo($x[0].':'.$s); }; As for simply omitting the keyword, e.g. function () () - as already suggested: I don't like that syntax either. Although I'm not a fan of too much language verbosity (that's why I don't like Fortran, Basic and Pascal), I think in this case, a little more verbosity wouldn't hurt - and typing 'use' is just 3 additional characters. Now for the examples for the smaller issues: Segfault: ?php $a = function () { $GLOBALS['a'] = NULL; echo destroyed closure\n; }; var_dump ($a); $a (); ? This crashes - due to the fact that the currently used op_array is destroyed upon destruction of the variable. This could get even more interesting if the closure called itself recursively. My proposal is to create a copy (but not a reference, just do a normal copy, for resources or objects that will just do the trick) of the variable internally in zend_call_function and zend_do_fcall_common_helper into a dummy zval and destroy that zval after the function call ended. That way, the GC won't kick in until after the execution of the closure. In zend_call_function that's easy - in zend_do_fcall_common helper we have the problem that the variable containing the closure is no longer available. An idea could be that the INIT_FCALL functions always additionally push the lambda zval to the argument stack (inside the function it will be ignored) and the fcall_common_helper will remove that zval from the stack prior to returning (and free it). If a non-closure is called, NULL (or an empty zval or whatever) could be pushed to the stack instead. Hmm, perhap's I'll have a better idea tomorrow. Anyway, since Andi suggested to use objects instead of resources, I'd like to use your patch as a starting point, if there are no objections. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi! - I am a little confused about the OOP interaction. How does a function become a public method of the class? To clarify: the public method ist just the internal representation of the lambda function and has *nothing* to do with the semantics of calling the lambda itself. The method only means that the lambda function defined inside another method can access the class members and public only means that the lambda function can still be called from outside the class. If one knew how to access it, which it seems is not possible/feasible for user-space code. No, that's not what I meant. The engine uses the following internal trick: a) Upon copmilation, my patch simply adds the lambdas as normal functions to the function table with an automatically generated unique (!) name. If it happens to be defined within a class method, the function will be added as a public final method to that class. b) That added function is not directly callable due to checks of a flag in the internal structure of that function. c) At the place of the function definition the compiler leaves an opcode grab function $generatedname and make a closure out of it. This opcode then looks up the generated lambda function, copies the function structure, saves the bound variables in that structure and returns the copied structure as a resource. d) Normally, when a function is called, the name is looked up in the function table. The function structure that is retrieved from there is then used to execute the function. Since a lambda resource is already a function structure, there is no necessity to look up anything in the function table but the function structure can be directly passed on to the executor. Please note step d): The closure functionality only changes the *lookup* of the function - so instead of getting the function structure from a hash table lookup I get the function structure by retrieving it from the resource. But *after* the lookup of a class method there are checks for the access protection of that method. So these access protection checks also apply to closures that were called. If a lambda function was not declared public, it could not be used outside of the class it was defined in. Perhaps this makes it clearer? I see. It would be great if you could update the RFC with this information so that it's clearer. Done: http://wiki.php.net/rfc/closures Two other questions that just occurred to me: 1) What is the interaction with namespaces, if any? Are lambdas as implemented here ignorant of namespace, or do they take the namespace where they are lexically defined? My patch itself is namespace-ignorant, but the net result is not: a) The generated internal function names do not contain the current namespace name, but since namespace names in function names are only used for lookup if you want to call the function. And calling lambdas by name (!) directly doesn't work anyway (is not supposed to work) so this poses no problem. b) The code *inside* the closure is namespace-aware because the information of which namespace is used is added at compile time. Either the name lookup is done entirely at compile time or the current compiler namespace is automatically added to all runtime lookup calls (this is already the case with current code). So the information which namespace a function resides in is currently *irrelevant* at runtime when calling other functions. For (b) let me make two examples: Suppose you have the following code: namespace Foo; function baz () { return Hello World!\n; } function bar () { return function () { echo baz (); }; } and in another file: $lambda = Foo::bar (); $lambda (); This will - as expected - print Hello World!\n. The reason is that the compiler upon arriving at the baz() function call inside the closure already looks up the function in the function table directly (it knows the current namespace) - and simply creates a series of opcodes that will call the function with the name Foo::baz (the lookup is already done at compile time). Consider this other code: foo-bar.php: namespace Foo; function bar () { return function () { echo baz (); }; } foo-baz.php: namespace Foo; function baz () { return Hello World!\n; } baz.php: function baz () { return PHP!\n; } test1.php: require 'foo-bar.php'; require 'foo-baz.php'; $lambda = Foo::bar (); $lambda (); test2.php: require 'foo-bar.php'; require 'baz.php'; $lambda = Foo::bar (); $lambda (); Running test1.php yields Hello World! whereas running test2.php yields PHP!. Why is this? Because when the compiler reaches the baz () function call in the closure, it cannot find the function so it cannot determine whether it's a function in global or in namespace scope. So it will simply add a series of opcodes that say try Foo::bar and if that does not exist try bar. Here again, Foo:: is added by
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi! 1) I am not sure that the current semantics of the lexical keyword is great in all cases. Is the reason why you don't allow by-value binding so that we don't have to manage more than one lambda instance per declaration? First of all: global and static are also used to create references to other variables (OK, with static they are not visible to the outside, but nevertheless...) and second because other languages do the same. As someone corrected me a while ago, even JS uses references, test the following JavaScript code: function foo () { var x = 5; var f = function (n) { x += n; }; alert (x); f (2); alert (x); } foo (); That will yield first 5 and then 7. 2) [minor curiosity - do we want to consider reusing parent instead of lexical? I guess that could be confusing but it's not the first time we reuse a keyword when it's clear that the usage is in two different places (this is minor and I don't mind much either way although lexical doesn't mean too much to me).] Consider this code: class A { public function printSomething ($var) { echo $var\n; } } class B extends A { public function printSomething ($var) { $printer = function () { parent $var; parent::printSomething ('I print: ' . $var); }; $printer (); } } Yeah, of course, my example is extremely stupid since it could be done entirely without closures but I really dread the perspective of having to explain someone the difference between those two lines... 3) I am concerned about binding to classes. First of all we need to look into more detail what the implications are for bytecode caches when changing class entries at run-time. Well, that's the thing: My patch does NOT change classes at runtime, so that is totally a non-issue. :-) When creating a lambda function inside a class method, it adds a new class method for the lambda function at compile time (!). This compile-time added method has a dynamic name (__compiled_lambda_F_N where F is the filename and N is a per-file counteŕ). To an opcode cache processing this class this added method will appear no different than a normal class method - it can be cached just the same. Now, upon execution of the code containing the closure, the new opcode just copies the zend_function structure into a copy, registers that copy as a resource and returns that resource. As soon as the resource is garbage collected (or explicitly unset), the op_array copy is destroyed. No modification of the actual class is done at all - the cache remains happy. Just for clarity I have posted a sample output of PHP with my Patch and VLD active (153 is the new ZEND_DECLARE_LAMBDA_FUNC opcode that VLD does not yet know about): http://www.christian-seiler.de/temp/php-closure-opcodes.txt Perhaps this helps to understand better how my patch works? We may want to also consider an option where the lambda binds to the object and only has public access although I realize that may be considered by some as too limiting. We'll review these two things in the coming days. What do you mean with binds to the object? But if you only want to grant access to public object members: If I declare a closure inside a class method, from a programmers point of view I am still within that class - why shouldn't I be able to access all class properties there? I would find such a limitation quite odd (and technically unecessary). Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi, [I'm going to collect here a bit:] Stanislav Malyshev wrote: lexical in the proposal binds to creator's scope, not caller's scope, as I understood. Anyway, binding to caller's immediate scope doesn't seem that useful since you could just pass it as a parameter when calling. Correct and I completely agree. Chris Stockton wrote: I am curious if is_callable will be able to detect these? Yes, as is call_user_func able to call them. (But as I was verifying that I saw that there was a tiny bug in the code that makes sure the internal names are not used directly, I will fix that.) Richard Quadling wrote: [JS Example] I'm not sure I would say that there is a reference used there. I'm no expert, but x, f() and n are all working like normal variables with x having to look outside of f(). Unless I'm getting confused with pass by reference. Yes, shure, ok, it's not a reference in the classical sense BUT the effect is the same: A change INSIDE the closure changes the variable outside. The only useful way of doing that in PHP without rewriting the complete engine is using references - and such things are *already* done via references - namely global variables: If you import a variable via $global, in reality you are creating a reference to the actual global variable - global $foo is actually more or less the same as $foo = $_GLOBALS['foo']. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi Andi, Hi Stanislav, - You mention global and static as examples of how we do things today. They are actually not good examples because the binding by reference which they do has been a real pain over the years. This is why we introduced the $GLOBALS[] array so that you could also assign by reference ($GLOBALS[foo] = $var). Now that I think of this example I'd actually prefer to see $LEXICALS[] or something similar to access variables then go with the broken global/static behavior. This will bite us and people will complain... In general, I always recommend to people to keep away from global and go with $GLOBALS[]. The problem here might be that if we do something like $LEX[$foo] in runtime, we don't know which parts of parent's scope we need to preserve. While I like the syntax, it may not work this way. Yes, that's the point. 'lexical $foo' does two things (instead of global simply doing one thing): a) At compile time (!) remember the name of the variable specified in an internal list assigned to the function. Example: function () { lexical $data, $i; } This will cause op_array-lexical_names to be the list data, i. b) At run time, the lexical keyword creates a reference to the lexical variables that are stored in op_array-lexical_variables (just as global does with global scope) The op_array-lexical_variables itself is filled in the new opcode which is executed upon *assignment* (read: creation) of the closure. It's essentially a for loop that goes through op_array-lexical_names and adds a reference from the current symbol table to the op_array-lexical_variables table. So, to make an example (with line numbers for reference): 1 $data = foo; 2 $i = 4; 3 $func = function () { 4 lexical $data, $i; 5 return array ($data, $i); 6 }; 7 8 $func (); Step 1 (Line 4 at compile time): op_array-lexical_names is set to data, i. Step 2 (Line 3 at run time): The ZEND_DECLARE_LAMBDA_FUNC opcode is executed, it creates a copy of the op_array to store in the return value, in the copy it initializes the hash table op_array-lexical_variables and then creates two new variables in op_array-lexical_variables which are references to the current scope varialbes $data and $i: +---+ +-+ | lex_variables | | EG(active_symbol_table) | +---+ ref +-+ | data--|--|- data | | i --|--|- i | | | | func | | | | ... | +---+ +-+ Step 3 (Line 8 at run time): The closure is executed. Step 4 (Line 4 at run time): The lexical keyword retrieves the $data and $i variables from op_array-lexical_variables and adds a reference to them: +-+ +---+ +-+ | EG(active_symbol_table) | | lex_variables | | parent s.t. | +-+ +---+ +-+ | data|--|- data ---|--|- data | | i |--|- i---|--|- i | | | | | | func | | | | | | ... | +-+ +---+ +-+ Btw: The grammar for lexical_variable contains only T_VARIABLE (and not ${...} etc.) on purpose - to be sure the name can be extracted at compile time. (Just as a clarification how the patch internally works.) Frankly, I don't really see a problem with using references. It fits into what's already there in PHP and it assures that closures have the necessary properties to make them useful. Which brings me to another point - how bad would it be if closure's lifetime would be limited to parent's lifetime? Of course, this would limit some tricks, but would allow some other - like this direct access to parent's scope. That trick would actually completely destroy the concept of closures: The idea behind closures is that the lexical scope during *creation* of the closure is saved. If you say I want direct access via $LEXICALS then the lexical scope during the *execution* of the closure will be used (yeah sure, the scope will be the scope during the creation of the closure but the *state* of that scope will be the scope during execution and not creation - unbinding variables after defining the closure (and therefore for example loops) will not be possible at that point). Furthermore, the idea that the closure lives longer than the scope in which it was declared is one of the other most basic ideas behind closures. Also, consider this code: function foo () { $lambda = function () { echo Hello World!\n; }; $lambda
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi, - I am a little confused about the OOP interaction. How does a function become a public method of the class? To clarify: the public method ist just the internal representation of the lambda function and has *nothing* to do with the semantics of calling the lambda itself. The method only means that the lambda function defined inside another method can access the class members and public only means that the lambda function can still be called from outside the class. class Example { private $a = 2; function myMethod($b) { $lambda = function() { lexical $b; return $this-a * $b; // This part I get }; return $lambda; } } $e = new Example(); $lambda = $e-myMethod(); $e-$lambda(5); No, that's not what my patch does. My patch does: class Example { private $a = 2; public function myMethod ($b) { return function () { lexical $b; return $this-a * $b; }; } } $e = new Example (); $lambda = $e-myMethod (4); var_dump ($lambda ()); // int(8) $lambda2 = $e-myMethod (6); var_dump ($lambda2 ()); // int(12) So esentially, it does not matter whether you define a lambda function inside a method or a function (or in global scope, for that matter), you always use it the same way. The in-class-method lambda function only has the additional advantage of being able to access the private and protected class members since *internally* it is treated like a public class method. - Related to that, would it then be possible to add methods to a class at runtime using lambda functions as the added methods? No. If not, is that something that could reasonably be added here without hosing performance (or at least doing so less than stacking __call() and call_user_func_array() does)? If you want to add methods dynamically to classes, why not use the runkit extension? I really don't see a point in making lambda functions and closures something they are not. Regards, Christiaan -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi! class Dynamic { private $someVar = 5; /// adding a function to instances from within the class public function addMethodAtRuntime() { $this-dynamicFunc1 = function() { return $this-someVar; // expected to work } } } /// invoking dynamically added methods /// (anticipated behavior given above definitions) $dynamic-addMethodAtRuntime(); echo $dynamic-dynamicFunc1(); // 5 This will not work - for the same reason as this does not work: class SpecialChars { public $process = 'htmlspecialchars'; } $sc = new SpecialChars (); var_dump ($sc-process ('')); // call to undefined ... On the other hand, the following will work: $sc = new SpecialChars (); $processor = $sc-process; var_dump ($processor ('')); // string(8) lt;gt; The same with closures: echo $dynamic-dynamicFunc1(); // call to undefined ... $func = $dynamic-dynamicFunc1; echo $func (); // 5 echo call_user_func ($dynamic-dynamicFunc1); // 5 As I sead in my other mail: I don't see closures as a method for somehow making it possible to extend classes dynamically - if you want that, use runkit. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi Marcus, very nice work. Thanks! The only thing I don't like is the function naming (“\0__compiled_lambda_FILENAME_N”). Can we drop the \0? I used \0 because it is already used in two other places: 1) create_function (run-time lambda functions) uses \0__lambda_N 2) build_runtime_defined_function_key uses \0 to start function names. I can drop it if you like; personally, I don't care for either solution - it's an internal name that *may* leak to userspace in some circumstances but is never really useful for userspace anyway.. A minor side-note here: I oriented myself at build_runtime_defined_function_key at the time of writing but I have noticed a slight discrepancy between function names generated by build_runtime_defined_function_key and the normal function names: When stored in the corresponding function_table hash, for runtime defined function keys it is opline-op1.u.constant.value.str.len, whereas for normal function names it is »Z_STRLEN_P(...) + 1« and thus including the *trailing* (not preceding!) \0 in the hash key for normal function names but not including it for runtime defined function keys. Any idea why that is the case? [For the record: I'm refering to the code that is already used in PHP, not to my patch!] Or did you use the \0 prefix to prevent direct invocations? No, direct invocations are prevented by the is_lambda == 1 is_closure == 0 check. I think the best option would be to force lambda functions being public always. They are. If you look at my modified version of zend_do_begin_function_declaration, you will see that: if (is_lambda CG(active_class_entry)) { is_method = 1; fn_flags = ZEND_ACC_PUBLIC; if (CG(active_op_array)-fn_flags ZEND_ACC_STATIC) { fn_flags |= ZEND_ACC_STATIC; } } else if (is_lambda) { fn_flags = 0; } The only attribute that is inherited from the parent function is whether that function is static or not. The last question is about changing visibility and overriding functions, do we want that, or should we mark lamdas as final? Internal representations of lambda fuctions should never be overridden, so yes, ZEND_ACC_FINAL would probably be a good idea. Overriding them won't work anyway since the new opcode that instantiates a closure will always use the class in which the closure was defined to look it up. I'll add that. Actually how does it integrate with reflection? Good question, I will investigate that and come back to you. Comments about the implementation: - ZendEngine2/zend_compile.c: Why provide forward declaration for free_filename(), simply putting the new function above the first user avoids it and does the same with increased maintainability. s/static void add_lexical_var (/static void add_lexical_var(/ Ok, I will fix that. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi Marcus, I now have revised my patch to include your suggestions: http://www.christian-seiler.de/temp/closures-php-5.3-2008-06-17-2.diff The changes to the previous version: - \0 at the start of the compiled lambda function name is dropped. - lambdas which are class members are now marked as final - the generated name of the lambda is now also stored within the op_array (before op_array-function_name was simply lambda) - your suggestions for code cleanups in zend_compile.c Actually how does it integrate with reflection? Consider the following class: class Example { private $x = 0; public function getIncer () { return function () { $this-x++; }; } public function show () { $this-reallyShow (); } protected function reallyShow () { echo {$this-x}\n; } } Running Reflection::export(new ReflectionClass('Example')); will yield (among other things): - Methods [4] { Method [ user public method getIncer ] { @@ /home/christian/dev/php5.3/c-tests/reflection.php 6 - 10 } Method [ user final public method __compiled_lambda_/home/christian/dev/php5.3/c-tests/reflection.php_0 ] { @@ /home/christian/dev/php5.3/c-tests/reflection.php 7 - 9 } Method [ user public method show ] { @@ /home/christian/dev/php5.3/c-tests/reflection.php 12 - 14 } Method [ user protected method reallyShow ] { @@ /home/christian/dev/php5.3/c-tests/reflection.php 16 - 18 } } So lambda functions appear simply as additional methods in the class, with their generated name. Of course, the ReflectionMethod / ReflectionFunction classes could be extended so that it provides and additional method isLambda() in order to determine whether a function actually is a lambda function. But I'd rather do that in a separate step and a separate patch. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi! I'm not Johannes and I didn't review the proposal in detail yet, but I think we have enough for 5.3 right now. I'd think we better concentrate on tying the loose ends and rolling beta out and then moving towards the release than adding more and more features and never releasing it. 5.3 is not the final release until the end of times, there will be 5.4 etc. and 6, so there will be ample opportunity to add stuff. And 5.3 has enough stuff to be released, there's no rush to add more new things, especially radically new ones. My opinion is that we better take some time with it and not tie it to 5.3. I would like to see 5.3 released first before we add really cool features like this. I'm really +1 for closures but I please for 5.4 and 6. If I may add my own personal (and biased ;-)) opinion (which may not count much but I'd like to present the arguments): I'd like to see it in PHP 5.3. Mainly because of two reasons: First: My patch is quite non-intrusive, it only adds things in a few places (new opcode, a few checks). If you only look at the non-generated files (i.e. excluding files generated by re2c or zend_vm_gen.php), the patch is actually not even that long: http://www.christian-seiler.de/temp/closures-php-5.3-2008-06-17-3-redux.diff Except for the introduction of a 'lexical' keyword I carefully designed the patch not to have *any* impact *at all* on PHPs other behaviour. I'd be genuinely surprised if any code breaks with my patch. I also don't see how this would delay 5.3 - of course things have to be tested but at least as far as I can tell the major showstoppers currently are class inheritance rules and namespaces which still cause quite a few headaches of their own. Second: If closures are not supported in PHP 5.3, even with the release of PHP 6 backwards compability will be a hindrance in using them. Since PHP 6 will have Unicode support and thus quite a few semantic changes, this will of course not matter much for the actual PHP applications since these will have to change anyway. But think of class libraries: There are many things that can be implemented in class libraries where unicode support doesn't matter at all - such as for example advanced date and time calculations (beyond timelib), financial calculations etc. Such libraries will probably want to maintain compability with PHP 5.3 as long as possible. But these libraries may profit from closures. If you still decide to include closures only from post PHP 5.3, I suggest to at least declare 'lexical' a reserved keyword in PHP 5.3. Just my 2 cents... As to the patch for HEAD: I thought it best to wait for unicode.semantics to go away along with the if (UG(unicode)) checks before implementing it (everything else would be a waste of time - since if I'm not mistaken someone is actually currently removing those). If I really am mistaken in my interpretation of the discussions here on this topic and they are not going away (at least not in the short term), I can of course provide one now (meaning the next few days). Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi, As a followup to the discussion in January, I'd like post a revised patch to this list that implements closures and anonymous functions in PHP. INTRODUCTION Closures and lambda functions can make programming much easier in several ways: 1. Lambda functions allow the quick definition of throw-away functions that are not used elsewhere. Imaging for example a piece of code that needs to call preg_replace_callback(). Currently, there are three possibilities to acchieve this: a. Define the callback function elsewhere. This distributes code that belongs together throughout the file and decreases readability. b. Define the callback function in-place (but with a name). In that case one has to use function_exists() to make sure the function is only defined once. Example code: ?php function replace_spaces ($text) { if (!function_exists ('replace_spaces_helper')) { function replace_spaces_helper ($matches) { return str_replace ($matches[1], ' ', 'nbsp;').' '; } } return preg_replace_callback ('/( +) /', 'replace_spaces_helper', $text); } ? Here, the additional if() around the function definition makes the source code difficult to read. c. Use the present create_function() in order to create a function at runtime. This approach has several disadvantages: First of all, syntax highlighting does not work because a string is passed to the function. It also compiles the function at run time and not at compile time so opcode caches can't cache the function. 2. Closures provide a very useful tool in order to make lambda functions even more useful. Just imagine you want to replace 'hello' through 'goodbye' in all elements of an array. PHP provides the array_map() function which accepts a callback. If you don't wan't to hard-code 'hello' and 'goodbye' into your sourcecode, you have only four choices: a. Use create_function(). But then you may only pass literal values (strings, integers, floats) into the function, objects at best as clones (if var_export() allows for it) and resources not at all. And you have to worry about escaping everything correctly. Especially when handling user input this can lead to all sorts of security issues. b. Write a function that uses global variables. This is ugly, non-reentrant and bad style. c. Create an entire class, instantiate it and pass the member function as a callback. This is perhaps the cleanest solution for this problem with current PHP but just think about it: Creating an entire class for this extremely simple purpose and nothing else seems overkill. d. Don't use array_map() but simply do it manually (foreach). In this simple case it may not be that much of an issue (because one simply wants to iterate over an array) but there are cases where doing something manually that a function with a callback as parameter does for you is quite tedious. [Yes, I know that str_replace also accepts arrays as a third parameter so this example may be a bit useless. But imagine you want to do a more complex operation than simple search and replace.] PROPOSED PATCH -- I now propose a patch that implements compile-time lambda functions and closures for PHP while keeping the patch as simple as possible. The patch is based on a previous patch on mine which was based on ideas discussed here end of December / start of January. Userland perspective 1. The patch adds the following syntax as a valid expression: function (parameters) { body } (The is optional and indicates - just as with normal functions - that the anonymous function returns a reference instead of a value) Example usage: $lambda = function () { echo Hello World!\n; }; The variable $lambda then contains a callable resource that may be called through different means: $lambda (); call_user_func ($lambda); call_user_func_array ($lambda, array ()); This allows for simple lambda functions, for example: function replace_spaces ($text) { $replacement = function ($matches) { return str_replace ($matches[1], ' ', 'nbsp;').' '; }; return preg_replace_callback ('/( +) /', $replacement, $text); } 2. The patch implements closures by defining an additional keyword 'lexical' that allows an lambda function (and *only* an lambda function) to import a variable from the parent scope to the lambda function scope. Example: function replace_in_array ($search, $replacement, $array) { $map = function ($text) { lexical $search, $replacement; if (strpos ($text, $search) 50) { return str_replace ($search, $replacement,
Re: [PHP-DEV] [PATCH] [RFC] Closures and lambda functions in PHP
Hi, Lukas Kahwe Smith asked me to put my proposal into the PHP wiki, which I have done: http://wiki.php.net/rfc/closures I also have incorporated the last comment by troels knak-nielsen about JS behaving the same way as my patch and thus not being that much of a WTF at all (I somehow had a different behaviour for JS in mind, I probably got confused with yet another language). Anyway, feel free to comment. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] PATCH: Implementing closures in PHP
Hi! Just a minor note; The semi-colon after the closing brace, seems superfluous. Is there any reason for it? Unfortunately, yes. The problem is that the closure must be an expression so it can a) be assigned to a variable and b) returned directly. And since the expression is only a part of a statement, I can take no influence at that point in the grammar as to whether a semicolon should follow or not. I don't see any way of removing the semicolon without a) either making the language inconsistent and/or b) adding a lot of bloat to the grammar. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] PATCH: Implementing closures in PHP
Hi David! One question about the names you generate for the function table in combination with opcode caches. [...] I now updated the patch so that this problem is addressed. You will find it here: http://www.christian-seiler.de/temp/closures-php-5-3-v2.patch The compiled functions are now named __compiled_lambda_$hash_$counter, where $counter is a per-file lambda counter and $hash is a hash made from the file name (see hash_compiled_filename in zend_compile.c, I wasn't sure how good Zend's hash function is with duplicates so I hashed the file name AND its basename - feel free to change that function if you have a better idea or know it's safe to only hash the filename itself because duplicates are too rare). Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] PATCH: Implementing closures in PHP (was: anonymous functions in PHP)
Hi, I was following this thread and came upon Jeff's posting on how closures could be implemented in PHP. Since I would find the feature to be EXTREMELY useful, I decided to actually implement it more or less the way Jeff proposed. So, here's the patch (against PHP_5_3, I can write one against HEAD if you whish): http://www.christian-seiler.de/temp/closures-php-5-3.patch I started with Wez's patch for adding anonymous functions that aren't closures. I changed it to make sure no shift/reduce or reduce/reduce error occur in the grammar. Then I started implementing the actual closure stuff. It was fun because I learned quite a lot about how PHP actually works. I had the following main goals while developing the patch: 1. Don't reinvent the wheel. 2. Don't break anything unless absolutely necessary. 3. Keep it simple. Jeff proposed a new type of zval that holds additional information about the function that is to be called. Adding a new type of zval would need changes throughout the ENTIRE PHP source and probably also throughout quite a few scripts. But fortunately, PHP already posesses a zval that supports the storage of arbitrary data while being very lightweight: Resources. So I simply added a new resource type that stores zend functions. The $var = function () {}; will now make $var a resource (of the type anonymous function. Anonymous functions are ALWAYS defined at compile time, no matter where they occur. They are simply named __compiled_lamda_1234 and added to the global function table. But instead of simply returning the string '__compiled_lambda_1234', I introduced a new opcode that will create references to the correct local variables that are referenced inside the function. For example, if you have: $func = function () { echo Hello World\n; }; This will result in an anonymous function called '__compiled_lambda_0' that is added to the function table at compile time. The opcode for the assignment to $func will be something like: 1 ZEND_DECLARE_ANON_FUNC ~0 '__compiled_lambda_0' 2 ASSIGN !0, ~0 The ZEND_DECLARE_ANON_FUNC opcode handler does the following: It creates a new zend_function, copies the contents of the entire structure of the function table entry corresponding to '__compiled_lamda_0' into that new structure, increments the refcount, registeres it as a resource and returns that resource so it can be assigned to the variable. Now, have a look at a real closure: $string = Hello World!\n; $func = function () { lexical $string; echo $string; }; This will result in the same opcode as above. But here, three additional things happen: 1. The compiler sees the keyword 'lexical' and stores the information, that a variable called 'string' should be used inside the closure. 2. The opcode handler sees that a variable named 'string' is marked as lexical in the function definition. Therefore it creates a reference to it in a HashTable of the COPIED zend_function (that will be stored in the resource). 3. The 'lexical $string;' translates into a FETCH opcode that will work in exactly the same way as 'static' or 'global' - only fetching it from the additional HashTable in the zend_function structure. The resource destructor makes sure that the HashTable containing the references to the lexical veriables is correctly destroyed upon destruction of the resource. It does NOT destroy other parts of the function structure because they will be freed when the function is removed from the global function table. With these changes, closures work in PHP. Some caveats / bugs / todo: * Calling anonymous functions by name directly is problematic if there are lexical variables that need to be assigned. I added checks to make sure this case does not happen. * In the opcode handler, error handling needs to be added. * If somebody removes the function from the global function table, (e.g. with runkit), the new opcode will return NULL instead of a resource (error handling is missing). Since I do increment refcount of the zend_function, it SHOULD not cause segfaults or memory leaks, but I haven't tested it. * $this is kind of a problem, because all the fetch handlers in PHP make sure $this is a special kind of variable. For the first version of the patch I chose not to care about this because what still works is e.g. the following: $object = $this; $func = function () { lexical $object; // do something }; Also, inside the closures, the class context is not preserved, so accessing private / protected members is not possible. I'm not sure this actually represents a problem because you can always use normal local variables to pass values between closure and calling method and make the calling method change the properties itself. * I've had some problems with eval(), have a look at the following code: $func = eval ('return function ()
Re: [PHP-DEV] PATCH: Implementing closures in PHP
Hi! typo alert: Oh, thanks (don't know how it got in there ;-)), I fixed that, same address: http://www.christian-seiler.de/temp/closures-php-5-3.patch Very impressive patch, I'll be interested to try it out when I get a chance. Thanks! Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] PATCH: Implementing closures in PHP
Hi! I'm going to answer to everybody at once, if that's OK. David Zülke wrote: One question about the names you generate for the function table in combination with opcode caches. [...] While this is a constructed example, it could easily occur with conditional includes with environments that use opcode caches. Oh, yes, that's true, I didn't think of that, thanks a lot for pointing that out! Tomorrow I'll post an updated patch which will make sure this doesn't happen (see below). troels knak-nielsen wrote: I have another observation about names. Instead of using an arbitrary name, as the name of the function, wouldn't it be possible to let the name be derived from the function-body. Eg., if you took the function-body's tokens and created a hash from them. This would have two implications: 1) Multiple definitions of the same function would be optimised into one. And more importantly 2) , it would be possible to serialize/unserialize a closure. Of course, this won't work if an anonymous function is a resource, since resources can't be serialized. This would work for Wez' original patch though. Thanks for the suggestions, and although I don't completely agree with you, you pointed me into the direction I'm leaning towards now, so thanks. :-) First of all: I don't quite understand what you mean when you want to serialize a function (closure or not)? The opcodes? Ok, sure, with the current PHP implementation you can serialize the variable used to CALL the function (e.g. with $func = 'str_replace'; $func is only a string and can be serialized). But where would you need that? (Ok, for normal functions that are named this could actually be useful, but for anonymous functions?) Using the function body itself as a hashed value for the function name would require some _major_ changes to the parser. Currently, the function name has to be known before the function body starts. Also, the tokens inside the function would have to be tracked and stored in an array somehow. This would be quite a performance penalty during compile time. But: The idea I hat thanks to you is to use the file name and a per-file counter for the function name. So the name would be something like (symbollically speaking) '__compiled_lambda_' + hashfunction(__FILE__) + '_' + per_file_counter. That would solve the problem with opcode caches while not causing any real performance penalties (the hash of the loaded file would only have to be calculated once). As I also wrote above, I'll post an updated patch tomorrow that will address this problem. Martin Alterisio wrote: That's very kind of you but, if I was explained right, you don't have copyright on a patch. [...] I don't agree at all (except for the part where a patch is a derived work) but since I don't want to talk about copyright laws here (no offense, but I'd rather spend my time writing code), I think we both can live with the following statement: If you're right, the PHP team can use the patch anyway, so it isn't a problem. If I'm right, I've given the PHP team the necessary permission to use it, so it isn't a problem either. Regards, Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php