[PHP-DEV] RE: Improved zend_string API
Hi, OK, I didn’t understand Dmitry’s comment on ‘aggressively’ using inline functions but, thanks to your comment, I understand now. It has nothing to do with performance. Using a function for -val is useless, I agree, as it is read-only be nature. But using 2 functions for len provides read-only/write-only methods, which is not possible with a macro. As this is a new API, I thought it was the occasion to provide a high encapsulation level from the start. This will always be less work for the future. Dmitry suggested we rename everything using a ‘ZSTR_’ terminology, promote these names, and declare old names as compatibility macros. I’m currently working on this and will probably deliver it tonight. It will be a separate optional commit. You’ll see if time allows to include it. Anyway, this mustn’t be blocking inclusion in 7.0. Regards François De : Anatol Belski [mailto:anatol@belski.net] Envoyé : vendredi 26 juin 2015 21:16 À : 'Dmitry Stogov'; 'francois' Cc : 'Anatol Belski'; 'PHP Internals' Objet : RE: Improved zend_string API Hi, I’ve checked the patch and tested the build, both looks fine. Though I would go by the suggestion from Dmitry about the new inlined functions. At least VC produces same ASM, probably gcc would produce good results as well. However, to operate just on one struct member a function looks like too much. Also we test mostly gcc and vc, but macros would ensure it works properly with compilers we don’t test. Also probably it would be a bit simpler from the dev perspective. I wouldn’t insist on this, but imho macros would make more sense. But all in all, if Dmitry’s tests bring positive results, we’re likely to accept this patch for 7.0. Regards Anatol From: Dmitry Stogov [mailto:dmi...@zend.com] Sent: Friday, June 26, 2015 12:59 PM To: francois Cc: Anatol Belski; PHP Internals Subject: Re: Improved zend_string API Hi Francois, From the source code review, I don't see any problems. May be too aggressive usage of inline functions in zend_string implementation, but I hope, it shouldn't make any harm. I'll need to check, if C compilers are smart enough to produce good optimized code after inlining. In general ZSTR_xxx API is more consistent to me. I would even add ZSTR_...() twins for all zend_string_...(), and recommend to use ZTR_xxx names. Then we may decide to remove zend_string_..., STR_... and other name inconsistencies (in the future). In any case, I'll be able to test the PR only on Monday. Anatol, I would suggest to accept this, like an additional more consistent API (keeping the old names). But if it makes any troubles, delays or significant risks for 7.0 release process, I would say no. Thanks. Dmitry. On Fri, Jun 26, 2015 at 4:05 AM, François Laupretre franc...@php.net wrote: Hi Dmitry, I have created a separate PR (https://github.com/php/php-src/pull/1367) containing just the zend_string API enhancements we've been talking about. It integrates the new ZSTR_VAL/ZSTR_LEN/ZSTR_HASH, renaming of STR_xxx to ZSTR_, plus compatibility macros and a pair of utility functions. I have incorporated a commit which changes the macros names from STR_ to ZSTR_ everywhere they are used in the distrib. I hope you can incorporate this as part of phpng for 7.0. It would be important to provide an API so that people stop using -val/-len directly as soon as possible. I give up on the 'api-checks' and 'strict-api' configure flags. ATM, I don't have the energy to go through an RFC process which has every chances to fail. Maybe later... Regards François
RE: [PHP-DEV] Headsup: PHP7 feature freeze
Hi Yasuo, -Original Message- From: yohg...@gmail.com [mailto:yohg...@gmail.com] On Behalf Of Yasuo Ohgaki Sent: Friday, June 26, 2015 1:58 PM To: Hannes Magnusson Cc: Kalle Sommer Nielsen; Internals; Anatoliy Belsky; Dmitry Stogov; Nikita Popov; Ferenc Kovacs; Xinchen Hui Subject: Re: [PHP-DEV] Headsup: PHP7 feature freeze Hi all, On Fri, Jun 26, 2015 at 12:56 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi Hannes, On Fri, Jun 26, 2015 at 12:51 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Fri, Jun 26, 2015 at 10:48 AM, Hannes Magnusson hannes.magnus...@gmail.com wrote: Why do you think its undocumented? http://php.net/manual/en/sessionhandler.create-sid.php Rename discussion was there. And I explicitly discussed it's undocumented and it violates CODING_STANDARDS, but it was added recently (after the discussion I suppose). [yohgaki@dev session]$ svn log -r 334814 - --- r334814 | aharvey | 2014-09-09 04:49:26 +0900 (2014年09月09日 (火)) | 2 lines Add documentation for SessionHandler::create_sid(). - --- 334814aharvey classnameSessionHandler/classname is a special class that can be used 334814aharvey to expose the current internal PHP session save handler by inheritance. 334814aharvey There are seven methods which wrap the seven internal session save handler 334814aharvey callbacks (parameteropen/parameter, parameterclose/parameter, 334814aharvey parameterread/parameter, parameterwrite/parameter, 334814aharvey parameterdestroy/parameter, parametergc/parameter and 334814aharvey parametercreate_sid/parameter). By default, this class will wrap 334814aharvey whatever internal save handler is set as defined by the 334814aharvey link linkend=ini.session.save-handlersession.save_handler/link 334814aharvey configuration directive which is usually parameterfiles/parameter by 334814aharvey default. Other internal session save handlers are provided by PHP 334814aharvey extensions such as SQLite (as parametersqlite/parameter), Memcache (as 334814aharvey parametermemcache/parameter), and Memcached (as 334814aharvey parametermemcached/parameter). I think this should be reverted. Or it may stay there. It's just a matter of having a copy of create_sid(). I'll add documentation. I forgot that session_create_id() is needed createSid() method to be more useful. The code for session_create_id() is in the source, but it isn't enabled. I wouldn't like to have different naming session_create_id() and createSid(). So I would like to have - session_create_id() function - createId() function because there is - session_id() since PHP4. I don't think session internal names do not have to be changed. i.e. Macros, etc. Any comments? Changing internal or user space API is kind of too late, IMHO. Especially the user space APIs that are documented. But also the internals, as a lot of extensions are already ported. Also because sessions are a core functionality where changes should be supported but a good migration path. Please target later 7.x versions with this change. But probably would make sense to create an RFC and start the discussion like already... yesterday, so the topic is good discussed and accepted for the next. Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Improved zend_string API
Committed except of ZVAL_STR_INC/DEC_LEN() and some other unused macros. Thanks. Dmitry. On Mon, Jun 29, 2015 at 4:23 PM, François Laupretre franc...@php.net wrote: Hi, OK, I didn’t understand Dmitry’s comment on ‘aggressively’ using inline functions but, thanks to your comment, I understand now. It has nothing to do with performance. Using a function for -val is useless, I agree, as it is read-only be nature. But using 2 functions for len provides read-only/write-only methods, which is not possible with a macro. As this is a new API, I thought it was the occasion to provide a high encapsulation level from the start. This will always be less work for the future. Dmitry suggested we rename everything using a ‘ZSTR_’ terminology, promote these names, and declare old names as compatibility macros. I’m currently working on this and will probably deliver it tonight. It will be a separate optional commit. You’ll see if time allows to include it. Anyway, this mustn’t be blocking inclusion in 7.0. Regards François *De :* Anatol Belski [mailto:anatol@belski.net] *Envoyé :* vendredi 26 juin 2015 21:16 *À :* 'Dmitry Stogov'; 'francois' *Cc :* 'Anatol Belski'; 'PHP Internals' *Objet :* RE: Improved zend_string API Hi, I’ve checked the patch and tested the build, both looks fine. Though I would go by the suggestion from Dmitry about the new inlined functions. At least VC produces same ASM, probably gcc would produce good results as well. However, to operate just on one struct member a function looks like too much. Also we test mostly gcc and vc, but macros would ensure it works properly with compilers we don’t test. Also probably it would be a bit simpler from the dev perspective. I wouldn’t insist on this, but imho macros would make more sense. But all in all, if Dmitry’s tests bring positive results, we’re likely to accept this patch for 7.0. Regards Anatol *From:* Dmitry Stogov [mailto:dmi...@zend.com dmi...@zend.com] *Sent:* Friday, June 26, 2015 12:59 PM *To:* francois *Cc:* Anatol Belski; PHP Internals *Subject:* Re: Improved zend_string API Hi Francois, From the source code review, I don't see any problems. May be too aggressive usage of inline functions in zend_string implementation, but I hope, it shouldn't make any harm. I'll need to check, if C compilers are smart enough to produce good optimized code after inlining. In general ZSTR_xxx API is more consistent to me. I would even add ZSTR_...() twins for all zend_string_...(), and recommend to use ZTR_xxx names. Then we may decide to remove zend_string_..., STR_... and other name inconsistencies (in the future). In any case, I'll be able to test the PR only on Monday. Anatol, I would suggest to accept this, like an additional more consistent API (keeping the old names). But if it makes any troubles, delays or significant risks for 7.0 release process, I would say no. Thanks. Dmitry. On Fri, Jun 26, 2015 at 4:05 AM, François Laupretre franc...@php.net wrote: Hi Dmitry, I have created a separate PR (https://github.com/php/php-src/pull/1367) containing just the zend_string API enhancements we've been talking about. It integrates the new ZSTR_VAL/ZSTR_LEN/ZSTR_HASH, renaming of STR_xxx to ZSTR_, plus compatibility macros and a pair of utility functions. I have incorporated a commit which changes the macros names from STR_ to ZSTR_ everywhere they are used in the distrib. I hope you can incorporate this as part of phpng for 7.0. It would be important to provide an API so that people stop using -val/-len directly as soon as possible. I give up on the 'api-checks' and 'strict-api' configure flags. ATM, I don't have the energy to go through an RFC process which has every chances to fail. Maybe later... Regards François
[PHP-DEV] RE: [PHP-CVS] com php-src: Fix bug #69957 (Different ways of handling div/mod by zero): NEWS Zend/tests/bug69957.phpt Zend/zend_compile.c Zend/zend_operators.c Zend/zend_vm_def.h Zend/zend_v
Hi Bob, This commit and 79b1832dd5086cd9b4a2e778a62f1743b548726c was reverted, please check also the comments in the ticket. Bob, please do more discussion of such cases with Dmitry and Hui before. Especially the patches that affect very basic functionality should not be urged to be pushed right at the second. Discussing before will indeed spare our time, as possible issues were pre evaluated. That will also bring more structure in our work. Thanks Anatol -Original Message- From: Bob Weinand [mailto:bwo...@php.net] Sent: Sunday, June 28, 2015 6:23 PM To: php-...@lists.php.net Subject: [PHP-CVS] com php-src: Fix bug #69957 (Different ways of handling div/mod by zero): NEWS Zend/tests/bug69957.phpt Zend/zend_compile.c Zend/zend_operators.c Zend/zend_vm_def.h Zend/zend_vm_execute.h Commit:fb08798c9f0ea820d567668d0cea4833dc61dd8e Author:Bob Weinand bobw...@hotmail.com Sun, 28 Jun 2015 18:22:10 +0200 Parents: 64c371142cbdb82eb0879d247430797d73a8ac2d Branches: master Link: http://git.php.net/?p=php- src.git;a=commitdiff;h=fb08798c9f0ea820d567668d0cea4833dc61dd8e Log: Fix bug #69957 (Different ways of handling div/mod by zero) Bugs: https://bugs.php.net/69957 Changed paths: M NEWS A Zend/tests/bug69957.phpt M Zend/zend_compile.c M Zend/zend_operators.c M Zend/zend_vm_def.h M Zend/zend_vm_execute.h -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Bundled GD library
Pierre Joye wrote: Hi, On Jun 29, 2015 3:34 AM, Christoph Becker cmbecke...@gmx.de wrote: Hi! I've just learned from ext/gd/libgd/README that the bundled GD library is actually a fork of libgd, which was quite a surprise to me as I thought gd is handled similar to pcre and sqlite3. It is somehow similar as it was an actual fork before we took over the upstream version. Due to own BC reasons we cannot be fully synced with upstream at this stage but a large part of the library is. Ah, I see. Thanks for the explanation. :) Anyhow, I wonder how to deal with bug reports for PHP's gd extension, if they point to a common bug in libgd. Shall I make PRs against php-src *and* report the issue to libgd? Either way. If you can do both it will be fantastic. But as the same persons maintain both sides, we do our best to keep them synced :) Okay, so I'll do both. :) -- Christoph M. Becker -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Fix division by zero to throw exception (round 2)
I would like to bring this topic back up, as there were users confused with it and it's absolutely not consistent what we have now. See also https://bugs.php.net/bug.php?id=69957 https://bugs.php.net/bug.php?id=69957 (As I thought it was non-intentional, I went ahead and fixed it, was reverted later, hence discussing that now here.) So, looks like there was some quick decisions and discussion I totally had missed. What we have now is: Am 03.04.2015 um 23:13 schrieb Dmitry Stogov dmi...@zend.com: So the summary: 1) division by zero produces a warning and +/-INF IS_DOUBLE. Compile-time evaluation is disabled. 3) Modulo by zero produces Exception.Compile-time evaluation is disabled. Why? Why do we change the one but not the other? Why does 0 % 0 throw an Exception, but 0 / 0 NAN? Why does 1 % 0 throw an Exception, but 1 / 0 INF? I'd like to either properly return 0, INF or NAN in both cases or in none. Having different rules for so similar operations is non-sense, I think. It just is inconsistent and causes confusion. Bob
Re: [PHP-DEV] Fix division by zero to throw exception (round 2)
Hi Bob, On 29 Jun 2015, at 16:54, Bob Weinand bobw...@hotmail.com wrote: I would like to bring this topic back up, as there were users confused with it and it's absolutely not consistent what we have now. See also https://bugs.php.net/bug.php?id=69957 (As I thought it was non-intentional, I went ahead and fixed it, was reverted later, hence discussing that now here.) So, looks like there was some quick decisions and discussion I totally had missed. What we have now is: Am 03.04.2015 um 23:13 schrieb Dmitry Stogov dmi...@zend.com: So the summary: 1) division by zero produces a warning and +/-INF IS_DOUBLE. Compile-time evaluation is disabled. 3) Modulo by zero produces Exception.Compile-time evaluation is disabled. Why? Why do we change the one but not the other? Why does 0 % 0 throw an Exception, but 0 / 0 NAN? Why does 1 % 0 throw an Exception, but 1 / 0 INF? I'd like to either properly return 0, INF or NAN in both cases or in none. Having different rules for so similar operations is non-sense, I think. It just is inconsistent and causes confusion. Those operations are not as similar as you think! In PHP, the complement of % isn’t /, it’s intdiv(). intdiv() takes an integer divided and divisor, and produces an integer quotient. % also takes an integer dividend and divisor, but produces an integer remainder. intdiv() and % work with integer-only inputs and always produce integer outputs. /, on the other hand, takes an integer or float dividend and divisor, and produces an integer or float fractional result. It might as well work only on floats and always produce floats, really, given that it behaves the same as if it did. So intdiv() and % work on integers, and / works on integers and floats. This informs how they handle certain error cases. For integer division and modulo (PHP’s intdiv() and %), the accepted way to handle division by zero is to produce an error. There’s no correct or useful result you can produce. In PHP, we used to produce FALSE here, which is a different type (boolean). I don’t think that was a good idea because of PHP’s weak typing. If you use FALSE in some other arithmetic operation, it’ll be coerced to zero, and produce weird results from other operations. Sure, there’s an E_WARNING produced, but your code keeps running and produces garbage. So, throwing an exception is safer and brings PHP into line with established practice in other programming languages. For floating-point division (PHP’s /), on the other hand, errors are usually handled differently. IEEE 754 defines special error values we can produce: +Infinity, -Infinity and NaN. These are still values of the float type, but they’re not normal numbers. They flow through floating-point operations in a well-defined manner. For example, if you do anything with a NaN and a NaN, you get a NaN, while if you divide by zero, you get ±Infinity, and if you divide by Infinity, you get ±0. Again, PHP used to produce FALSE here, which has the problems described earlier. ±Infinity and NaN, on the other hand, flow properly through later arithmetic operations. If the error affects your result, it will most likely be obvious, because it’ll be ±0, ±Infinity, or NaN. Yes, you result is garbage, but it’s at least obviously so. And, like with the integer behaviour, this brings PHP into line with established practice. tl;dr: intdiv() and % have matching behaviour because they work with integers and that’s what’s usually does for them, / has different behaviour because it works with floats and that’s what’s done for them. I hope that makes sense. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Fix division by zero to throw exception (round 2)
Hey Andrea, Am 29.06.2015 um 18:49 schrieb Andrea Faulds a...@ajf.me: Hi Bob, On 29 Jun 2015, at 16:54, Bob Weinand bobw...@hotmail.com wrote: I would like to bring this topic back up, as there were users confused with it and it's absolutely not consistent what we have now. See also https://bugs.php.net/bug.php?id=69957 (As I thought it was non-intentional, I went ahead and fixed it, was reverted later, hence discussing that now here.) So, looks like there was some quick decisions and discussion I totally had missed. What we have now is: Am 03.04.2015 um 23:13 schrieb Dmitry Stogov dmi...@zend.com: So the summary: 1) division by zero produces a warning and +/-INF IS_DOUBLE. Compile-time evaluation is disabled. 3) Modulo by zero produces Exception.Compile-time evaluation is disabled. Why? Why do we change the one but not the other? Why does 0 % 0 throw an Exception, but 0 / 0 NAN? Why does 1 % 0 throw an Exception, but 1 / 0 INF? I'd like to either properly return 0, INF or NAN in both cases or in none. Having different rules for so similar operations is non-sense, I think. It just is inconsistent and causes confusion. Those operations are not as similar as you think! In PHP, the complement of % isn’t /, it’s intdiv(). intdiv() takes an integer divided and divisor, and produces an integer quotient. % also takes an integer dividend and divisor, but produces an integer remainder. intdiv() and % work with integer-only inputs and always produce integer outputs. /, on the other hand, takes an integer or float dividend and divisor, and produces an integer or float fractional result. It might as well work only on floats and always produce floats, really, given that it behaves the same as if it did. So intdiv() and % work on integers, and / works on integers and floats. This informs how they handle certain error cases. For integer division and modulo (PHP’s intdiv() and %), the accepted way to handle division by zero is to produce an error. There’s no correct or useful result you can produce. In PHP, we used to produce FALSE here, which is a different type (boolean). I don’t think that was a good idea because of PHP’s weak typing. If you use FALSE in some other arithmetic operation, it’ll be coerced to zero, and produce weird results from other operations. Sure, there’s an E_WARNING produced, but your code keeps running and produces garbage. So, throwing an exception is safer and brings PHP into line with established practice in other programming languages. For floating-point division (PHP’s /), on the other hand, errors are usually handled differently. IEEE 754 defines special error values we can produce: +Infinity, -Infinity and NaN. These are still values of the float type, but they’re not normal numbers. They flow through floating-point operations in a well-defined manner. For example, if you do anything with a NaN and a NaN, you get a NaN, while if you divide by zero, you get ±Infinity, and if you divide by Infinity, you get ±0. Again, PHP used to produce FALSE here, which has the problems described earlier. ±Infinity and NaN, on the other hand, flow properly through later arithmetic operations. If the error affects your result, it will most likely be obvious, because it’ll be ±0, ±Infinity, or NaN. Yes, you result is garbage, but it’s at least obviously so. And, like with the integer behaviour, this brings PHP into line with established practice. tl;dr: intdiv() and % have matching behaviour because they work with integers and that’s what’s usually does for them, / has different behaviour because it works with floats and that’s what’s done for them. I hope that makes sense. -- Andrea Faulds http://ajf.me/ http://ajf.me/ Yes, it generally makes sense... Then I have other questions: - Why do we then still have a Warning? Either we have well-defined behavior, or we throw an exception. Well-defined behavior *plus* a warning is IMO non-sense. - Is it intentional for intdiv and % to throw an Exception instead of Error or some more specific DivisionByZeroError or similar? (yes, I know, Error is only very recent, but the question still needs to be asked). Bob.
Re: [PHP-DEV] Fix division by zero to throw exception (round 2)
On Mon, Jun 29, 2015 at 5:54 PM, Bob Weinand bobw...@hotmail.com wrote: I would like to bring this topic back up, as there were users confused with it and it's absolutely not consistent what we have now. See also https://bugs.php.net/bug.php?id=69957 (As I thought it was non-intentional, I went ahead and fixed it, was reverted later, hence discussing that now here.) So, looks like there was some quick decisions and discussion I totally had missed. What we have now is: Am 03.04.2015 um 23:13 schrieb Dmitry Stogov dmi...@zend.com: So the summary: 1) division by zero produces a warning and +/-INF IS_DOUBLE. Compile-time evaluation is disabled. 3) Modulo by zero produces Exception.Compile-time evaluation is disabled. Why? Why do we change the one but not the other? Why does 0 % 0 throw an Exception, but 0 / 0 NAN? Why does 1 % 0 throw an Exception, but 1 / 0 INF? I'd like to either properly return 0, INF or NAN in both cases or in none. Having different rules for so similar operations is non-sense, I think. It just is inconsistent and causes confusion. Bob I agree with Bob here: It would be nice if the behavior of division and modulus lined up. I'd like to add that the choice of returning Inf and additionally throwing a warning is particularly weird. While using Inf as the result of division by zero makes sense in some contexts (mainly scientific computing), in these contexts it's also a perfectly normal operation that would not generate a warning. If we want to use Inf we should remove the warning as well. However it should be noted that PHP is not well known for it's application in scientific computing and for our purposes the exception is likely more useful. Nikita
Re: [PHP-DEV] Improved zend_string API
Am 29.06.2015 um 15:46 schrieb Dmitry Stogov dmi...@zend.com: Committed except of ZVAL_STR_INC/DEC_LEN() and some other unused macros. Thanks. Dmitry. On Mon, Jun 29, 2015 at 4:23 PM, François Laupretre franc...@php.net wrote: Hi, OK, I didn’t understand Dmitry’s comment on ‘aggressively’ using inline functions but, thanks to your comment, I understand now. It has nothing to do with performance. Using a function for -val is useless, I agree, as it is read-only be nature. But using 2 functions for len provides read-only/write-only methods, which is not possible with a macro. As this is a new API, I thought it was the occasion to provide a high encapsulation level from the start. This will always be less work for the future. Dmitry suggested we rename everything using a ‘ZSTR_’ terminology, promote these names, and declare old names as compatibility macros. I’m currently working on this and will probably deliver it tonight. It will be a separate optional commit. You’ll see if time allows to include it. Anyway, this mustn’t be blocking inclusion in 7.0. Regards François *De :* Anatol Belski [mailto:anatol@belski.net] *Envoyé :* vendredi 26 juin 2015 21:16 *À :* 'Dmitry Stogov'; 'francois' *Cc :* 'Anatol Belski'; 'PHP Internals' *Objet :* RE: Improved zend_string API Hi, I’ve checked the patch and tested the build, both looks fine. Though I would go by the suggestion from Dmitry about the new inlined functions. At least VC produces same ASM, probably gcc would produce good results as well. However, to operate just on one struct member a function looks like too much. Also we test mostly gcc and vc, but macros would ensure it works properly with compilers we don’t test. Also probably it would be a bit simpler from the dev perspective. I wouldn’t insist on this, but imho macros would make more sense. But all in all, if Dmitry’s tests bring positive results, we’re likely to accept this patch for 7.0. Regards Anatol *From:* Dmitry Stogov [mailto:dmi...@zend.com dmi...@zend.com] *Sent:* Friday, June 26, 2015 12:59 PM *To:* francois *Cc:* Anatol Belski; PHP Internals *Subject:* Re: Improved zend_string API Hi Francois, From the source code review, I don't see any problems. May be too aggressive usage of inline functions in zend_string implementation, but I hope, it shouldn't make any harm. I'll need to check, if C compilers are smart enough to produce good optimized code after inlining. In general ZSTR_xxx API is more consistent to me. I would even add ZSTR_...() twins for all zend_string_...(), and recommend to use ZTR_xxx names. Then we may decide to remove zend_string_..., STR_... and other name inconsistencies (in the future). In any case, I'll be able to test the PR only on Monday. Anatol, I would suggest to accept this, like an additional more consistent API (keeping the old names). But if it makes any troubles, delays or significant risks for 7.0 release process, I would say no. Thanks. Dmitry. On Fri, Jun 26, 2015 at 4:05 AM, François Laupretre franc...@php.net wrote: Hi Dmitry, I have created a separate PR (https://github.com/php/php-src/pull/1367) containing just the zend_string API enhancements we've been talking about. It integrates the new ZSTR_VAL/ZSTR_LEN/ZSTR_HASH, renaming of STR_xxx to ZSTR_, plus compatibility macros and a pair of utility functions. I have incorporated a commit which changes the macros names from STR_ to ZSTR_ everywhere they are used in the distrib. I hope you can incorporate this as part of phpng for 7.0. It would be important to provide an API so that people stop using -val/-len directly as soon as possible. I give up on the 'api-checks' and 'strict-api' configure flags. ATM, I don't have the energy to go through an RFC process which has every chances to fail. Maybe later... Regards François Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str-val[str-len] = 0; zend_string_set_len(str, ZSTR_LEN(str) - 2); or str-len -= 2; Also, that particular function suggests to me that the string is expanded if lengths don't match. I much prefer direct accessors which indicate a) no side-effects happening here, b) less functions to remember (you just look at the struct and know everything about it) and c) end up being more readable. Yes, I'm totally aware that when we ever want to change the API (I don't see coming it in near future anyway), it could help having wrappers. But wrappers don't help against every API change… let's say for some reason we'd want to go back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad example, but just trying to show that, while it has maybe a
Re: [PHP-DEV] Fix division by zero to throw exception (round 2)
Hi again, On 29 Jun 2015, at 18:02, Bob Weinand bobw...@hotmail.com wrote: Yes, it generally makes sense... Then I have other questions: - Why do we then still have a Warning? Either we have well-defined behavior, or we throw an exception. Well-defined behavior *plus* a warning is IMO non-sense. That’s weird, yeah. We don’t throw warnings for the math functions when you give them odd inputs, e.g. sin(INF) is just NAN, no warning. I think removing it would make sense. - Is it intentional for intdiv and % to throw an Exception instead of Error or some more specific DivisionByZeroError or similar? (yes, I know, Error is only very recent, but the question still needs to be asked). Hmm. Using Error might make some sense given it used to raise E_WARNING. I think DivisionByZeroError sounds like a good idea. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] RE: Improved zend_string API
Hi Dmitry, I just commited some additional changes, following your suggestions : - Renamed every ‘zend_string_xxx()’ functions to ‘ZSTR_’-prefixed uppercase names, - Defined compatibility macros for old names - Changed the rest of the code to use new ZSTR_ names - A special case for zend_string_tolower(), which I found in ‘zend_operators.c’. I renamed it but didn’t move it to zend_string.c/.h. You probably know better than me if there’s a good reason to define it there or if it should be moved. I also made ZSTR_LEN() read-only (ZSTR_SET_LEN() must be used for write operations), making the zend_string API ‘clean’ in terms of separate getters/setters (Z_STRLEN() remains a read-write macro, no BC break here). I removed the functions you removed. Anyway, increment/decrement functions will need to be defined when we migrate the code to use the API only because the code often increments/decrements string length. As ZSTR_LEN() is read-only, we cannot use ‘++’ with it, and providing increment/decrement functions at the zend_string level is the only way to avoid writing ‘ZSTR_SET_LEN(s,ZSTR_LEN(s) + 1)’. I hope we (and especially you) still have enough time to integrate this, especially the new ‘ZSTR’ naming scheme. Anyway, I’m glad you could commit the most important additions for 7.0. Regards François -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Improved zend_string API
Hi Bob, -Original Message- From: Bob Weinand [mailto:bobw...@hotmail.com] Sent: Monday, June 29, 2015 6:44 PM To: Dmitry Stogov Cc: francois; Anatol Belski; Anatol Belski; PHP Internals Subject: Re: [PHP-DEV] Improved zend_string API Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str-val[str-len] = 0; zend_string_set_len(str, ZSTR_LEN(str) - 2); or str-len -= 2; Also, that particular function suggests to me that the string is expanded if lengths don't match. I much prefer direct accessors which indicate a) no side-effects happening here, b) less functions to remember (you just look at the struct and know everything about it) and c) end up being more readable. Yes, I'm totally aware that when we ever want to change the API (I don't see coming it in near future anyway), it could help having wrappers. But wrappers don't help against every API change… let's say for some reason we'd want to go back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad example, but just trying to show that, while it has maybe a theoretical gain, I don't see a real practical benefit.) zend_string is really a core structure and extremely simple. Please don't trade simplicity for a small other benefit. Also, we still leave zend_string struct exposed (presumably for allowing inlining). At the point where we are now, many extension authors probably already are also accessing zend_string directly. We're going to have a colorful mix of code using the APIs and not using it, which is the worst we can get now. To end up, I very much like unifying prefixes (ZSTR_*), but I strongly dislike hurting readability and consistency so much for such little benefit. I can only express my background on accepting this patch. In first place, the names ZSTR_* are really speaking, like (z)end_(str)ing and the rest mirros the name of the actual member. That is better than STR_*. Or like IS_INTERNED vs ZSTR_IS_INTERNED, talking about what happens. With the set len function - yep, probably a bit misleading. Thinking also about the new foreach macros for hash or even zend_hash_num_elements symbol which has survived from PHP5 among others. Same here, access to one member encapsulated. Though direct accesses -nNumOfElements is still here and there, where it's useful or no care taken. Just to name one analogue. A macro doesn't cost anything, while it does its job abstracting things. The PR is closed. It's not about forcing anyone to not to use -val and alike (maybe indeed it is), but about what one calls application programming interface. aka API. -val is not an API. For the outer codes it's probably the own responsibility on using the APIs vs direct access. Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Headsup: PHP7 feature freeze
2015-06-28 22:19 GMT+02:00 Anatol Belski anatol@belski.net: What is the benefit changing it? XHTML is a standard which is alive. That's on every person's opinion. XHTML is currently not a standard, XHTML is dead. But, there's for sure some code based on parsing the phpinfo() output, since not everything is exported as a constant. IMHO having HTML5 in phpinfo(), while being nice, doesn't justify itself breaking those codes. Okay, that could be an argument. So I will rather focus on replacing just these completely legacy things like a name or font etc. (plus inline styles to make the HTML code more readable and self-explainable). I would argue that software can be updated, but... okay, let it be. Best regards, Kubo2, Jakub Kubíček -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Headsup: PHP7 feature freeze
Hi Anatol, On Mon, Jun 29, 2015 at 10:19 PM, Anatol Belski anatol@belski.net wrote: -Original Message- From: yohg...@gmail.com [mailto:yohg...@gmail.com] On Behalf Of Yasuo Ohgaki Sent: Friday, June 26, 2015 1:58 PM To: Hannes Magnusson Cc: Kalle Sommer Nielsen; Internals; Anatoliy Belsky; Dmitry Stogov; Nikita Popov; Ferenc Kovacs; Xinchen Hui Subject: Re: [PHP-DEV] Headsup: PHP7 feature freeze Hi all, On Fri, Jun 26, 2015 at 12:56 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi Hannes, On Fri, Jun 26, 2015 at 12:51 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Fri, Jun 26, 2015 at 10:48 AM, Hannes Magnusson hannes.magnus...@gmail.com wrote: Why do you think its undocumented? http://php.net/manual/en/sessionhandler.create-sid.php Rename discussion was there. And I explicitly discussed it's undocumented and it violates CODING_STANDARDS, but it was added recently (after the discussion I suppose). [yohgaki@dev session]$ svn log -r 334814 - --- r334814 | aharvey | 2014-09-09 04:49:26 +0900 (2014年09月09日 (火)) | 2 lines Add documentation for SessionHandler::create_sid(). - --- 334814aharvey classnameSessionHandler/classname is a special class that can be used 334814aharvey to expose the current internal PHP session save handler by inheritance. 334814aharvey There are seven methods which wrap the seven internal session save handler 334814aharvey callbacks (parameteropen/parameter, parameterclose/parameter, 334814aharvey parameterread/parameter, parameterwrite/parameter, 334814aharvey parameterdestroy/parameter, parametergc/parameter and 334814aharvey parametercreate_sid/parameter). By default, this class will wrap 334814aharvey whatever internal save handler is set as defined by the 334814aharvey link linkend=ini.session.save-handlersession.save_handler/link 334814aharvey configuration directive which is usually parameterfiles/parameter by 334814aharvey default. Other internal session save handlers are provided by PHP 334814aharvey extensions such as SQLite (as parametersqlite/parameter), Memcache (as 334814aharvey parametermemcache/parameter), and Memcached (as 334814aharvey parametermemcached/parameter). I think this should be reverted. Or it may stay there. It's just a matter of having a copy of create_sid(). I'll add documentation. I forgot that session_create_id() is needed createSid() method to be more useful. The code for session_create_id() is in the source, but it isn't enabled. I wouldn't like to have different naming session_create_id() and createSid(). So I would like to have - session_create_id() function - createId() function because there is - session_id() since PHP4. I don't think session internal names do not have to be changed. i.e. Macros, etc. Any comments? Changing internal or user space API is kind of too late, IMHO. Especially the user space APIs that are documented. But also the internals, as a lot of extensions are already ported. Also because sessions are a core functionality where changes should be supported but a good migration path. Please target later 7.x versions with this change. But probably would make sense to create an RFC and start the discussion like already... yesterday, so the topic is good discussed and accepted for the next. No problem. I'll write a RFC for this. For the record, please don't document questionable APIs... I'll add comment to source if there are similar cases. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Move to Fast ZPP?
On Sat, Jun 27, 2015 at 12:36 AM, Matt Wilmas php_li...@realplain.com wrote: Hi Dmitry, all, - Original Message - From: Dmitry Stogov Sent: Thursday, June 25, 2015 On Wed, Jun 24, 2015 at 1:35 PM, Matt Wilmas php_li...@realplain.com wrote: Hi Dmitry, all, - Original Message - From: Dmitry Stogov Sent: Wednesday, June 24, 2015 We should NOT use it everywhere. It'll lead to code bloat. Thanks. Dmitry. [...] Right, FAST_ZPP makes code larger, inlining stuff in each function, etc. But I was wondering, is there any more that can be done to optimize the slowness of the traditional ZPP? I don't recall any changes being made to it in the last 12-18 months to speed it up at all... Is there a particular part that's making it slow? Is it the *string* of param types? Is it the va_arg's, that doesn't allow the call to be optimized as well and/or the arg fetching that's slow? I know there were the syntax issues as well, but would using something like a single param (array pointer) for param types/values help at all? Or is it just the internals of ZPP that are inherently slow...? :-/ Or that's fine but the mechanism of getting there is the issue? The traditional ZPP is actually a et another interpreter that reads specification string char by char and perform actions. And interpreters are usually slower. I think it may be improved, but I don't expect significant overall improvement because of that. Now, the cumulative overhead of traditional ZPP on wordpress is ~0.4%. Even if we make it 2 times faster, we may get just 0.2% overall speed up. Yeah, I knew how the traditional ZPP worked, just wondered about any certain problem area. :-) But it seems it's just the whole thing, so much it's doing, besides just the string format interpretation. First, only fractions of a % old ZPP is using on WordPress now? That doesn't make sense to me... On fast_zpp wiki page, you said last year it was taking ~6% of time on Wordpress (before FAST_ZPP, of course). And changing key/hot functions to FAST_ZPP saved ~2.5% time. So that should have left a few percent of time used by traditional ZPP. But everything else has gotten faster since then, so therefore, for an unchanged old ZPP, its percentage contribution should have gone up? Well, anyway... I went ahead and tried implementing my idea (had been awhile since I really looked at the FAST_ZPP stuff, and didn't realize it was as simple to work from). :-) It uses the same syntax as FAST_ZPP (if we/others like/prefer that) and a zend_FAST_parse_parameters() function. Code size should be about same as before, maybe a few more bytes depending on instructions needed (still thinking/adjusting). It seems to have pretty good performance increase! BTW, in quick testing, I don't see old ZPP using 90% of time even with empty/dummy function. Just about 50% (with or without ZTS)... I didn't know how close we could get to the inlined FAST_ZPP, but it seems the majority of the way there: ~70% in the simple case. (To be clear, 3x faster than old.) This was on Windows XP with ancient VC9. I don't have a patch ready for you to look at yet, since I didn't finish changing the macros, etc. It would be awesome if this could start being used throughout the codebase, and not just functions with preferential treatment. :-P Maybe you'd even switch back from the inlined version in some places, if smaller code would be better. Send you implementation as soon as it's ready. I'll test it. Thanks. Dmitry. Thanks. Dmitry. - Matt
RE: [PHP-DEV] Allow exceptions in __toString()
Hi Nikita,
-Original Message-
From: Nikita Popov [mailto:nikita@gmail.com]
Sent: Thursday, June 25, 2015 2:53 PM
To: PHP internals; Dmitry Stogov
Subject: [PHP-DEV] Allow exceptions in __toString()
Hi internals!
Currently it's not possible to throw an exception from __toString() - instead
you
will get a (real) fatal error. This is becoming more and more problematic as
time
goes on, e.g. with PHP 7 an exception can be triggered as a result of a VM
error
or even a parse error.
I'd like to lift this restriction. A patch can be found here:
https://github.com/php/php-src/pull/1364
Apart from allowing exceptions, the patch also converts two leftover
recoverable fatal errors relating to __toString() into Error exceptions.
Furthermore the patch makes sure that we correctly (i.e. without leaks and
without superfluous notices or other side-effects) handle __toString()
exceptions
in the engine. This includes concatenation and interpolation, but also things
like
writing $foo-$object or ${$object}.
zend_parse_parameters() and Fast ZPP also deal with exceptions correctly.
However what it doesn't do, and what I wouldn't consider feasible to do, is
ensure that every single string conversion in library functions is exception
safe.
Personally I don't think this is a blocking issue, as the worst that can
happen is
usually an additional superfluous warning to be thrown, or something similar.
If
cases like this turn up, we can specifically target them.
It should also be noted that whatever issues with exceptions-safety may
remain,
they already exist now (plus those the patch fixes), because the two
aforementioned recoverable fatal errors can be converted to exceptions (and
anyone doing blanket ErrorException conversions will do this).
So basically the question here is, is this partial solution acceptable for us?
Given the expressed concerns, despite the change itself is a good thing when
working properly, IMHO we shouldn't urge it into 7.0. If having it now can lead
to subtle improper behavior and even worse bugs, preferable were to have a
complete patch in 7.1.
Regards
Anatol
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Improved zend_string API
On Jun 29, 2015, at 7:37 PM, Bob Weinand bobw...@hotmail.com wrote: Hey Anatol, Am 29.06.2015 um 22:41 schrieb Anatol Belski anatol@belski.net: Hi Bob, -Original Message- From: Bob Weinand [mailto:bobw...@hotmail.com] Sent: Monday, June 29, 2015 6:44 PM To: Dmitry Stogov Cc: francois; Anatol Belski; Anatol Belski; PHP Internals Subject: Re: [PHP-DEV] Improved zend_string API Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str-val[str-len] = 0; zend_string_set_len(str, ZSTR_LEN(str) - 2); or str-len -= 2; Also, that particular function suggests to me that the string is expanded if lengths don't match. I much prefer direct accessors which indicate a) no side-effects happening here, b) less functions to remember (you just look at the struct and know everything about it) and c) end up being more readable. Yes, I'm totally aware that when we ever want to change the API (I don't see coming it in near future anyway), it could help having wrappers. But wrappers don't help against every API change… let's say for some reason we'd want to go back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad example, but just trying to show that, while it has maybe a theoretical gain, I don't see a real practical benefit.) zend_string is really a core structure and extremely simple. Please don't trade simplicity for a small other benefit. Also, we still leave zend_string struct exposed (presumably for allowing inlining). At the point where we are now, many extension authors probably already are also accessing zend_string directly. We're going to have a colorful mix of code using the APIs and not using it, which is the worst we can get now. To end up, I very much like unifying prefixes (ZSTR_*), but I strongly dislike hurting readability and consistency so much for such little benefit. I can only express my background on accepting this patch. In first place, the names ZSTR_* are really speaking, like (z)end_(str)ing and the rest mirros the name of the actual member. That is better than STR_*. Or like IS_INTERNED vs ZSTR_IS_INTERNED, talking about what happens. With the set len function - yep, probably a bit misleading. Thinking also about the new foreach macros for hash or even zend_hash_num_elements symbol which has survived from PHP5 among others. Same here, access to one member encapsulated. Though direct accesses -nNumOfElements is still here and there, where it's useful or no care taken. Just to name one analogue. A macro doesn't cost anything, while it does its job abstracting things. The PR is closed. It's not about forcing anyone to not to use -val and alike (maybe indeed it is), but about what one calls application programming interface. aka API. -val is not an API. For the outer codes it's probably the own responsibility on using the APIs vs direct access. Regards Anatol They're definitely speaking and as I said I very much like unifying prefixes. That's totally fine and not what I'm complaining about. Generally, I'm really just disliking ZSTR_LEN, ZSTR_VAL(), zend_string_set_len() and zend_string_revert_hash(). Sure, but who wants to type that? nNumberOfElements? I don't feel like you can remember that very well. oh… and wait. It's called nNumOfElements… See? That's why we needed a wrapper here. Point here is that it doesn't abstract anything, it just renames it. It costs the confusion it creates. Should I directly access it or not? There it was directly accessed, but there not. If we'd done that, we should have done that at the very beginning IMHO. Not now. And still, using a struct is an API. The members have a well defined meaning what they do. The struct itself is an API, it is an interface giving you the offset from base zend_string * address. Using my_len = *((size_t*)((char *)zend_string) + 16)) would be an example of not programming to the API. That's what a struct provides. A layer of abstraction over the offset. Hence it's already a level of abstraction. I'd like to urge you to not have two (used) APIs. And considering, that IMHO the direct accessors are much more readable than the macros and currently also much more widely used, I'd like to remove these 4 macros/inline functions. Thanks, Bob -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I’m very much in agreement with Bob on the confusion that these macros create. Most of the source still uses zend_string-val or zend_string-len, while now a few places now use ZSTR_VAL(zend_string) and ZSTR_LEN(zend_string). Which am I suppose to use? Do you intend to change every occurrence of direct access to the struct members to use
Re: [PHP-DEV] Improved zend_string API
Hey Dmitry, Am 29.06.2015 um 15:46 schrieb Dmitry Stogov dmi...@zend.com: Committed except of ZVAL_STR_INC/DEC_LEN() and some other unused macros. Thanks. Dmitry. I see you changed the ZSTR_* macros to be pure aliases of the equivalent structure members. May I now honestly ask where the benefit of those macros is? ZSTR_VAL(str) instead of str-val ZSTR_H(str) instead of str-h ZSTR_LEN(str) instead of str-len What is the point here? It's obvious why we have Z_* macros for the zval: because of the unions. (Like nobody wants to write zv-value.lval instead of just Z_LVAL_P(zv)) It's a good thing to hide that union here. (Though I'd prefer anon unions, but as I'm not sure what MSVC support here is, so no comment.) But there's no such reason for zend_string… Hence the question remains, what value does it bring for zend_string? Except that it obviously makes code harder to read? (It's longer, you can't be sure about potential side effects etc.) Thanks, Bob -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Improved zend_string API
De : Aaron Piotrowski [mailto:aa...@icicle.io] Envoyé : mardi 30 juin 2015 04:16 À : Bob Weinand; Anatol Belski Cc : internals@lists.php.net Objet : Re: [PHP-DEV] Improved zend_string API On Jun 29, 2015, at 7:37 PM, Bob Weinand bobw...@hotmail.com wrote: Hey Anatol, Am 29.06.2015 um 22:41 schrieb Anatol Belski anatol@belski.net: Hi Bob, -Original Message- From: Bob Weinand [mailto:bobw...@hotmail.com] Sent: Monday, June 29, 2015 6:44 PM To: Dmitry Stogov Cc: francois; Anatol Belski; Anatol Belski; PHP Internals Subject: Re: [PHP-DEV] Improved zend_string API Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str-val[str-len] = 0; zend_string_set_len(str, ZSTR_LEN(str) - 2); or str-len -= 2; Also, that particular function suggests to me that the string is expanded if lengths don't match. I much prefer direct accessors which indicate a) no side-effects happening here, b) less functions to remember (you just look at the struct and know everything about it) and c) end up being more readable. Yes, I'm totally aware that when we ever want to change the API (I don't see coming it in near future anyway), it could help having wrappers. But wrappers don't help against every API change… let's say for some reason we'd want to go back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad example, but just trying to show that, while it has maybe a theoretical gain, I don't see a real practical benefit.) zend_string is really a core structure and extremely simple. Please don't trade simplicity for a small other benefit. Also, we still leave zend_string struct exposed (presumably for allowing inlining). At the point where we are now, many extension authors probably already are also accessing zend_string directly. We're going to have a colorful mix of code using the APIs and not using it, which is the worst we can get now. To end up, I very much like unifying prefixes (ZSTR_*), but I strongly dislike hurting readability and consistency so much for such little benefit. I can only express my background on accepting this patch. In first place, the names ZSTR_* are really speaking, like (z)end_(str)ing and the rest mirros the name of the actual member. That is better than STR_*. Or like IS_INTERNED vs ZSTR_IS_INTERNED, talking about what happens. With the set len function - yep, probably a bit misleading. Thinking also about the new foreach macros for hash or even zend_hash_num_elements symbol which has survived from PHP5 among others. Same here, access to one member encapsulated. Though direct accesses -nNumOfElements is still here and there, where it's useful or no care taken. Just to name one analogue. A macro doesn't cost anything, while it does its job abstracting things. The PR is closed. It's not about forcing anyone to not to use -val and alike (maybe indeed it is), but about what one calls application programming interface. aka API. -val is not an API. For the outer codes it's probably the own responsibility on using the APIs vs direct access. Regards Anatol They're definitely speaking and as I said I very much like unifying prefixes. That's totally fine and not what I'm complaining about. Generally, I'm really just disliking ZSTR_LEN, ZSTR_VAL(), zend_string_set_len() and zend_string_revert_hash(). Sure, but who wants to type that? nNumberOfElements? I don't feel like you can remember that very well. oh… and wait. It's called nNumOfElements… See? That's why we needed a wrapper here. Point here is that it doesn't abstract anything, it just renames it. It costs the confusion it creates. Should I directly access it or not? There it was directly accessed, but there not. If we'd done that, we should have done that at the very beginning IMHO. Not now. And still, using a struct is an API. The members have a well defined meaning what they do. The struct itself is an API, it is an interface giving you the offset from base zend_string * address. Using my_len = *((size_t*)((char *)zend_string) + 16)) would be an example of not programming to the API. That's what a struct provides. A layer of abstraction over the offset. Hence it's already a level of abstraction. I'd like to urge you to not have two (used) APIs. And considering, that IMHO the direct accessors are much more readable than the macros and currently also much more widely used, I'd like to remove these 4 macros/inline functions. Thanks, Bob -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I’m very much in agreement with Bob on the confusion that these macros create. Most of the source still uses
Re: [PHP-DEV] Improved zend_string API
Hey Anatol, Am 29.06.2015 um 22:41 schrieb Anatol Belski anatol@belski.net: Hi Bob, -Original Message- From: Bob Weinand [mailto:bobw...@hotmail.com] Sent: Monday, June 29, 2015 6:44 PM To: Dmitry Stogov Cc: francois; Anatol Belski; Anatol Belski; PHP Internals Subject: Re: [PHP-DEV] Improved zend_string API Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str-val[str-len] = 0; zend_string_set_len(str, ZSTR_LEN(str) - 2); or str-len -= 2; Also, that particular function suggests to me that the string is expanded if lengths don't match. I much prefer direct accessors which indicate a) no side-effects happening here, b) less functions to remember (you just look at the struct and know everything about it) and c) end up being more readable. Yes, I'm totally aware that when we ever want to change the API (I don't see coming it in near future anyway), it could help having wrappers. But wrappers don't help against every API change… let's say for some reason we'd want to go back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad example, but just trying to show that, while it has maybe a theoretical gain, I don't see a real practical benefit.) zend_string is really a core structure and extremely simple. Please don't trade simplicity for a small other benefit. Also, we still leave zend_string struct exposed (presumably for allowing inlining). At the point where we are now, many extension authors probably already are also accessing zend_string directly. We're going to have a colorful mix of code using the APIs and not using it, which is the worst we can get now. To end up, I very much like unifying prefixes (ZSTR_*), but I strongly dislike hurting readability and consistency so much for such little benefit. I can only express my background on accepting this patch. In first place, the names ZSTR_* are really speaking, like (z)end_(str)ing and the rest mirros the name of the actual member. That is better than STR_*. Or like IS_INTERNED vs ZSTR_IS_INTERNED, talking about what happens. With the set len function - yep, probably a bit misleading. Thinking also about the new foreach macros for hash or even zend_hash_num_elements symbol which has survived from PHP5 among others. Same here, access to one member encapsulated. Though direct accesses -nNumOfElements is still here and there, where it's useful or no care taken. Just to name one analogue. A macro doesn't cost anything, while it does its job abstracting things. The PR is closed. It's not about forcing anyone to not to use -val and alike (maybe indeed it is), but about what one calls application programming interface. aka API. -val is not an API. For the outer codes it's probably the own responsibility on using the APIs vs direct access. Regards Anatol They're definitely speaking and as I said I very much like unifying prefixes. That's totally fine and not what I'm complaining about. Generally, I'm really just disliking ZSTR_LEN, ZSTR_VAL(), zend_string_set_len() and zend_string_revert_hash(). Sure, but who wants to type that? nNumberOfElements? I don't feel like you can remember that very well. oh… and wait. It's called nNumOfElements… See? That's why we needed a wrapper here. Point here is that it doesn't abstract anything, it just renames it. It costs the confusion it creates. Should I directly access it or not? There it was directly accessed, but there not. If we'd done that, we should have done that at the very beginning IMHO. Not now. And still, using a struct is an API. The members have a well defined meaning what they do. The struct itself is an API, it is an interface giving you the offset from base zend_string * address. Using my_len = *((size_t*)((char *)zend_string) + 16)) would be an example of not programming to the API. That's what a struct provides. A layer of abstraction over the offset. Hence it's already a level of abstraction. I'd like to urge you to not have two (used) APIs. And considering, that IMHO the direct accessors are much more readable than the macros and currently also much more widely used, I'd like to remove these 4 macros/inline functions. Thanks, Bob -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Improved zend_string API
Hi, -Message d'origine- De : Bob Weinand [mailto:bobw...@hotmail.com] Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? If you find it 'very nice', probably nothing. I suggest you stop using macros everywhere. After all, why did we define Z_STRVAL() instead of writing something 'very nice' like 'zv.value.str-val' ? Just one reason : if we had written 'zv.value.val' everywhere, zend_strings couldn't exist today. In a few words, we are improving encapsulation : publishing an API instead of a C structure. ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str-val[str-len] = 0; zend_string_set_len(str, ZSTR_LEN(str) - 2); or str-len -= 2; Also, that particular function suggests to me that the string is expanded if lengths don't match. I agree it would be quite natural and I'd like to implement it in a future version. We just need to store the allocated size in the struct, which also allows to over-allocate memory in advance. Anyway, performance is critical there, so the code must remain simple and fast. I much prefer direct accessors which indicate a) no side-effects happening here, b) less functions to remember (you just look at the struct and know everything about it) and c) end up being more readable. a) You deal with an API, not a structure. So, you call methods instead of accessing variables. The user does not have to care about side effects of the methods he's using. Don't think about C macros, think about methods. b) Getting documentation from the code is something we should forget. We still need to do it because PHP lacks a usable core API documentation, especially for PHP 7. So, getting API usage from the code is a workaround, not the appropriate way to publish an API. The same with some recently-published articles about new PHP 7 structures. These articles should *never* include the underlying structures. Then, we complain about people bypassing the 'official' APIs but, if they use such documentation, how can they know what they should access or not ? IMO, the rules are 'never publish structure details' and 'never allow direct access to structure elements'. c) Using ZSTR_VAL/ZSTR_LEN macros can also be considered as more readable. I personally consider it as more readable because other unrelated structures may also define 'len' and 'val' elements. Yes, I'm totally aware that when we ever want to change the API (I don't see coming it in near future anyway), it could help having wrappers. But wrappers don't help against every API change… let's say for some reason we'd want to go back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad example, but just trying to show that, while it has maybe a theoretical gain, I don't see a real practical benefit.) The fact that you don't imagine the API changing in the near future is not an argument. First, wrappers don't protect against API changes, they allow to maintain the same API when the backend implementation is modified. I see at least two reasons that could bring changes in the zend_string API, and it could be quite soon. The first one is unifying zend_string with smart_str, including in zend_string the smart_str allocation scheme. I don't know exactly the form it would take but reading and/or writing the string length could involve more than just writing a value in a structure element. The other subject that could require implementation changes is Unicode. It failed for a number of reasons in PHP6, but it doesn't mean we'll avoid the question forever. If we want to include Unicode at a low level, we may have to work at the zend_string level. Once again, in this case, reading and writing the string length will probably involve different internal operations. So, the question is not to blindly apply the theory and say 'tight coupling is evil'. There are objective reasons to encapsulate APIs. Zend_string is just the first one. It was choosen because no API existed to access the 'val' and 'len' elements, and because it was relatively easy to fix, allowing for an integration in 7.0. But the next one is HashTable, which is *much* more complex. If you look at the implementation and usage of HashTables in the code, you'll see how a combination of growing complexity and tight coupling can make code unmanageable. Actually, we are at a point where changing the HashTable internal implementation would be extremely heavy and complex. If we don't fix it now, it can only become worse over time. zend_string is really a core structure and extremely simple. Please don't trade simplicity for a small other benefit. Also, we still leave zend_string struct exposed (presumably for allowing inlining). At the point where we are now, many extension authors probably already are also accessing zend_string directly. We're going to have a colorful mix of code
RE: [PHP-DEV] Improved zend_string API
On Jun 30, 2015 9:38 AM, François Laupretre franc...@php.net wrote: Hi, -Message d'origine- De : Bob Weinand [mailto:bobw...@hotmail.com] Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? If you find it 'very nice', probably nothing. I suggest you stop using macros everywhere. After all, why did we define Z_STRVAL() instead of writing something 'very nice' like 'zv.value.str-val' ? Just one reason : if we had written 'zv.value.val' everywhere, zend_strings couldn't exist today. In a few words, we are improving encapsulation : publishing an API instead of a C structure. I agree here. It is good practice to do not expose structure directly. It prevents structure or behavior changes without users code modifications. Using an API only requires a compilation without any code change. I am not a fan of macros everywhere (we have way too much :) but this is not a reason to expose strict. Cheers, Pierre
RE: [PHP-DEV] RE: Improved zend_string API
Hi, De : François Laupretre [mailto:franc...@php.net] Envoyé : mardi 30 juin 2015 03:08 À : 'Dmitry Stogov' Cc : 'Anatol Belski'; 'PHP Internals' Objet : [PHP-DEV] RE: Improved zend_string API Hi Dmitry, I just commited some additional changes, following your suggestions : - Renamed every ‘zend_string_xxx()’ functions to ‘ZSTR_’-prefixed uppercase names, - Defined compatibility macros for old names - Changed the rest of the code to use new ZSTR_ names - A special case for zend_string_tolower(), which I found in ‘zend_operators.c’. I renamed it but didn’t move it to zend_string.c/.h. You probably know better than me if there’s a good reason to define it there or if it should be moved. I also made ZSTR_LEN() read-only (ZSTR_SET_LEN() must be used for write operations), making the zend_string API ‘clean’ in terms of separate getters/setters (Z_STRLEN() remains a read-write macro, no BC break here). I removed the functions you removed. Anyway, increment/decrement functions will need to be defined when we migrate the code to use the API only because the code often increments/decrements string length. As ZSTR_LEN() is read-only, we cannot use ‘++’ with it, and providing increment/decrement functions at the zend_string level is the only way to avoid writing ‘ZSTR_SET_LEN(s,ZSTR_LEN(s) + 1)’. I hope we (and especially you) still have enough time to integrate this, especially the new ‘ZSTR’ naming scheme. Anyway, I’m glad you could commit the most important additions for 7.0. I hadn't noticed that the PR had been closed. The changes described in my previous message are now at : https://github.com/php/php-src/pull/1381 Regards François -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Improved zend_string API
De : Bob Weinand [mailto:bobw...@hotmail.com] I see you changed the ZSTR_* macros to be pure aliases of the equivalent structure members. May I now honestly ask where the benefit of those macros is? ZSTR_VAL(str) instead of str-val ZSTR_H(str) instead of str-h ZSTR_LEN(str) instead of str-len What is the point here? It's obvious why we have Z_* macros for the zval: because of the unions. (Like nobody wants to write zv-value.lval instead of just Z_LVAL_P(zv)) It's a good thing to hide that union here. (Though I'd prefer anon unions, but as I'm not sure what MSVC support here is, so no comment.) But there's no such reason for zend_string… Hence the question remains, what value does it bring for zend_string? Except that it obviously makes code harder to read? (It's longer, you can't be sure about potential side effects etc.) This is the last reply I send because, if you don't understand that the purpose of an API is not just to provide shortcuts for lazy developers, I cannot do much more for you. Read books, go to school, there must be a way... Well, I try again : *Today*, writing 'ZSTR_VAL(zstr)' or 'zstr-val' produces the same code. *Tomorrow*, when you will compile your code with a new version of 'zend_string.h', the generated code may change for different reasons. It's called 'loose vs tight' coupling and I stop here because there are tons of books explaining it much better than I do. Oh, and the 'obvious' reason why we have zval macros is not hiding the union level, it is to provide an abstraction layer. Once again, if we didn't have defined this layer a long time ago, PHP 7 would be very different or, even, couldn't exist. Please, read my previous message. I tried to explain the benefits of encapsulation but it seems the case is too hard for me. Regards François -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Headsup: PHP7 feature freeze
On Mon, Jun 29, 2015 at 11:31 PM, Levi Morrison le...@php.net wrote: On Mon, Jun 29, 2015 at 3:47 PM, Jakub Kubíček kelerest...@gmail.com wrote: 2015-06-28 22:19 GMT+02:00 Anatol Belski anatol@belski.net: What is the benefit changing it? XHTML is a standard which is alive. That's on every person's opinion. XHTML is currently not a standard, XHTML is dead. But, there's for sure some code based on parsing the phpinfo() output, since not everything is exported as a constant. IMHO having HTML5 in phpinfo(), while being nice, doesn't justify itself breaking those codes. Okay, that could be an argument. So I will rather focus on replacing just these completely legacy things like a name or font etc. (plus inline styles to make the HTML code more readable and self-explainable). I would argue that software can be updated, but... okay, let it be. Best regards, Kubo2, Jakub Kubíček -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php (Sorry for previous blank email) XHTML is preferable to HTML for many people because tools for XML are stable and mature. Even HTML 5 is permitted to be serialized as XHTML (meaning you close tags that do not technically need to be closed, you self-close certain tags, etc). If we move to HTML 5 there is definite value to making sure it is XHTML compatible. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] VCS Account Request: trowski
Quickly applying bug fixes. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Improved zend_string API
Am 30.06.2015 um 04:37 schrieb François Laupretre franc...@php.net: Hi, -Message d'origine- De : Bob Weinand [mailto:bobw...@hotmail.com] Hey, looks like I'm a bit late to the party, but I'd like to object against the accessor macros on zend_string. What does it help to replace very nice -val, -len etc. with macros? If you find it 'very nice', probably nothing. I suggest you stop using macros everywhere. After all, why did we define Z_STRVAL() instead of writing something 'very nice' like 'zv.value.str-val' ? Just one reason : if we had written 'zv.value.val' everywhere, zend_strings couldn't exist today. In a few words, we are improving encapsulation : publishing an API instead of a C structure. That's what I said in the other mail (I sent about 4 mins after you sent yours)… It makes sense for hiding an union, but that's rather out of necessity because it's the best of two bads. (Anon union would be preferable) ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str-val[str-len] = 0; zend_string_set_len(str, ZSTR_LEN(str) - 2); or str-len -= 2; Also, that particular function suggests to me that the string is expanded if lengths don't match. I agree it would be quite natural and I'd like to implement it in a future version. We just need to store the allocated size in the struct, which also allows to over-allocate memory in advance. Anyway, performance is critical there, so the code must remain simple and fast. The point rather is that, while it's natural, you probably don't want that in most cases. I much prefer direct accessors which indicate a) no side-effects happening here, b) less functions to remember (you just look at the struct and know everything about it) and c) end up being more readable. a) You deal with an API, not a structure. So, you call methods instead of accessing variables. The user does not have to care about side effects of the methods he's using. Don't think about C macros, think about methods. b) Getting documentation from the code is something we should forget. We still need to do it because PHP lacks a usable core API documentation, especially for PHP 7. So, getting API usage from the code is a workaround, not the appropriate way to publish an API. The same with some recently-published articles about new PHP 7 structures. These articles should *never* include the underlying structures. Then, we complain about people bypassing the 'official' APIs but, if they use such documentation, how can they know what they should access or not ? IMO, the rules are 'never publish structure details' and 'never allow direct access to structure elements'. c) Using ZSTR_VAL/ZSTR_LEN macros can also be considered as more readable. I personally consider it as more readable because other unrelated structures may also define 'len' and 'val' elements. a) the structure is the API. Assigning or reading from/to a structure is like fetching the location to/from where to write. b) You'll never reach that goal. IMHO the goal is rather simplifying everything, yes, even direct structure usage, if it's making things *easy to use with a small API*. There are cases where the structure needs to be opaque, because all you ever want to do on that API is higher level. E.g. HashTable. Every manipulation on a HashTable needs many other things (adapting size, managing buckets etc.) changed. It's really *simplifying* our lives here. The ZSTR_VAL/LEN/H() macros? no idea where that simplifies anything. c) Sure, but the variable name will usually tell you. like zend_string *function_name;. Does it there make it more readable to have ZSTR_VAL(function_name) than function_name-val Also, note, when debugging, you ultimately end up always using the structures directly; like when lldb sees a str[1], I just see one char (when printing a zend_string *) … So having to access it via p str-val as char[1] decays to char * upon direct access. You end up needing knowledge of the structures. Using an API which is only a direct access alias doesn't make things easier here and is potentially only confusing. Yes, I'm totally aware that when we ever want to change the API (I don't see coming it in near future anyway), it could help having wrappers. But wrappers don't help against every API change… let's say for some reason we'd want to go back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad example, but just trying to show that, while it has maybe a theoretical gain, I don't see a real practical benefit.) The fact that you don't imagine the API changing in the near future is not an argument. First, wrappers don't protect against API changes, they allow to maintain the same API when the backend implementation is modified. I see at least two reasons that could bring changes in the zend_string API, and it could be quite soon. The first one is unifying zend_string with smart_str, including in zend_string the
Re: [PHP-DEV] Improved zend_string API
Am 30.06.2015 um 05:44 schrieb François Laupretre franc...@php.net: De : Bob Weinand [mailto:bobw...@hotmail.com] I see you changed the ZSTR_* macros to be pure aliases of the equivalent structure members. May I now honestly ask where the benefit of those macros is? ZSTR_VAL(str) instead of str-val ZSTR_H(str) instead of str-h ZSTR_LEN(str) instead of str-len What is the point here? It's obvious why we have Z_* macros for the zval: because of the unions. (Like nobody wants to write zv-value.lval instead of just Z_LVAL_P(zv)) It's a good thing to hide that union here. (Though I'd prefer anon unions, but as I'm not sure what MSVC support here is, so no comment.) But there's no such reason for zend_string… Hence the question remains, what value does it bring for zend_string? Except that it obviously makes code harder to read? (It's longer, you can't be sure about potential side effects etc.) This is the last reply I send because, if you don't understand that the purpose of an API is not just to provide shortcuts for lazy developers, I cannot do much more for you. Read books, go to school, there must be a way... APIs are there to abstract complexity away. Is there complexity? No. … And *additionally* it doesn't even add readability. Well, I try again : *Today*, writing 'ZSTR_VAL(zstr)' or 'zstr-val' produces the same code. *Tomorrow*, when you will compile your code with a new version of 'zend_string.h', the generated code may change for different reasons. It's called 'loose vs tight' coupling and I stop here because there are tons of books explaining it much better than I do. Precisely. Instead of tight coupling to the structure, we now couple to the macro… and it's not like the macro will be left untouched once we change the underlying structure. Did you see what changed with the arrays? We changed our underlying structures a lot… but had to adjust our APIs to make them fit to the underlying structures. Oh, and the 'obvious' reason why we have zval macros is not hiding the union level, it is to provide an abstraction layer. Once again, if we didn't have defined this layer a long time ago, PHP 7 would be very different or, even, couldn't exist. Oh, please, tell me why it would have been an issue? We'd just have changed our direct type value access to a Z_TYPE() access with PHP 7 (as there is now complexity which is a good thing to hide with PHP 7). Like … PHP 7 changed the semantics of Z_TYPE() macro… you can no longer assign directly to it. We anyway have to change our code. It's rare that such underlying changes don't leak through the API. Please, read my previous message. I tried to explain the benefits of encapsulation but it seems the case is too hard for me. Yes, I did; but I hadn't received your message yet before I sent that mail ;-) I generally have the feeling that you strongly overvalue encapsulation. Encapsulation makes things easier, some implementation details are changeable then, but e.g. zend_string * is already very low-level. Nearly every change on zend_string * will leak through the API encapsulating it. It's not the holy grail of forward compatibility. Use it where appropriate; don't _abuse_ it. Regards François Thanks, Bob -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Headsup: PHP7 feature freeze
On Mon, Jun 29, 2015 at 3:47 PM, Jakub Kubíček kelerest...@gmail.com wrote: 2015-06-28 22:19 GMT+02:00 Anatol Belski anatol@belski.net: What is the benefit changing it? XHTML is a standard which is alive. That's on every person's opinion. XHTML is currently not a standard, XHTML is dead. But, there's for sure some code based on parsing the phpinfo() output, since not everything is exported as a constant. IMHO having HTML5 in phpinfo(), while being nice, doesn't justify itself breaking those codes. Okay, that could be an argument. So I will rather focus on replacing just these completely legacy things like a name or font etc. (plus inline styles to make the HTML code more readable and self-explainable). I would argue that software can be updated, but... okay, let it be. Best regards, Kubo2, Jakub Kubíček -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
