# [RFC] Better type names for int64 RFC - Thread: https://externals.io/message/76797 - Original archive: https://news-web.php.net/php.internals/76797 - Started by Nikita Popov on 2014-08-22T11:16:27+00:00 - Messages: 35 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/76797.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #76797 — Nikita Popov — 2014-08-22T11:16:27+00:00 URL: https://externals.io/message/76797#76797 Hi internals! Today the int64 RFC has been merged, despite objections regarding the naming changes it introduces. As we were not given a chance to resolve this issue before the merge, a short proposal has been created, which aims to revert all unnecessary naming changes and instead use type names that are consistent with the existing code base and expectations: https://wiki.php.net/rfc/better_type_names_for_int64 Due to the unexpected merge on short notice, with no chance for discussion, this issue is blocking a number of other patches. As such I ask if we can move through this RFC with a shortened cycle. I would not appreciate having to wait three weeks to have a workable codebase again. Nikita --- ## #76798 — Joe Watkins — 2014-08-22T11:27:52+00:00 URL: https://externals.io/message/76797#76798 · In reply to: #76797 > Hi internals! > > Today the int64 RFC has been merged, despite objections regarding the > naming changes it introduces. > > As we were not given a chance to resolve this issue before the merge, a > short proposal has been created, which aims to revert all unnecessary > naming changes and instead use type names that are consistent with the > existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > Due to the unexpected merge on short notice, with no chance for discussion, > this issue is blocking a number of other patches. As such I ask if we can > move through this RFC with a shortened cycle. I would not appreciate having > to wait three weeks to have a workable codebase again. > > Nikita Morning internals, I'd very much like the same as Nikita; that's expected names, and quickly. Already it is going to be extremely difficult to maintain extensions for : > Hi internals! > > Today the int64 RFC has been merged, despite objections regarding the > naming changes it introduces. > > As we were not given a chance to resolve this issue before the merge, a > short proposal has been created, which aims to revert all unnecessary > naming changes and instead use type names that are consistent with the > existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > Due to the unexpected merge on short notice, with no chance for discussion, > this issue is blocking a number of other patches. As such I ask if we can > move through this RFC with a shortened cycle. I would not appreciate having > to wait three weeks to have a workable codebase again. > > Nikita Yes, please. We're all in favor of the 64 bit implementation, but definitely not on the naming. I think it was a big mistake to not separate the naming and implementation votes initially. I also would support a shorter RFC cycle, due to its blocking nature. [Vote in 2-3 days, vote lasting one week as usual] Bob --- ## #76803 — Derick Rethans — 2014-08-22T11:57:50+00:00 URL: https://externals.io/message/76797#76803 · In reply to: #76797 > Hi internals! > > Today the int64 RFC has been merged, despite objections regarding the > naming changes it introduces. > > As we were not given a chance to resolve this issue before the merge, > a short proposal has been created, which aims to revert all > unnecessary naming changes and instead use type names that are > consistent with the existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > Due to the unexpected merge on short notice, with no chance for > discussion, this issue is blocking a number of other patches. As such > I ask if we can move through this RFC with a shortened cycle. I would > not appreciate having to wait three weeks to have a workable codebase > again. Yes please. cheers, Derick --- ## #76804 — Anatol Belski — 2014-08-22T12:09:25+00:00 URL: https://externals.io/message/76797#76804 · In reply to: #76797 Hi Nikita, > Hi internals! > > > Today the int64 RFC has been merged, despite objections regarding the > naming changes it introduces. > > As we were not given a chance to resolve this issue before the merge, a > short proposal has been created, which aims to revert all unnecessary > naming changes and instead use type names that are consistent with the > existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > > Due to the unexpected merge on short notice, with no chance for > discussion, this issue is blocking a number of other patches. As such I > ask if we can move through this RFC with a shortened cycle. I would not > appreciate having to wait three weeks to have a workable codebase again. > > Nikita > to be complete, the RFC probably should take in account several new identifiers which came in (zend_off_t, zend_stat_t and co.). As their naming scheme is probably somewhat different. I've just documented those http://git.php.net/?p=php-src.git;a=blob;f=UPGRADING.INTERNALS and continue. Regards Anatol --- ## #76807 — Pierre Joye — 2014-08-22T12:10:55+00:00 URL: https://externals.io/message/76797#76807 · In reply to: #76797 > Hi internals! > > Today the int64 RFC has been merged, despite objections regarding the > naming changes it introduces. > > As we were not given a chance to resolve this issue before the merge, a > short proposal has been created, which aims to revert all unnecessary > naming changes and instead use type names that are consistent with the > existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > Due to the unexpected merge on short notice, with no chance for discussion, > this issue is blocking a number of other patches. As such I ask if we can > move through this RFC with a shortened cycle. I would not appreciate having > to wait three weeks to have a workable codebase again. I am against what you propose while being open to have better naming. Keeping what we have in 5.x is a huge mistake and will cause many issues, just like what NG does by using the same APIs or macros names but with different argument types. One example is the STR macro, a major pain. And the names of a macro should match what they actually do and the actually types they deal with, not some random totally misleading names. Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76808 — Pierre Joye — 2014-08-22T12:11:32+00:00 URL: https://externals.io/message/76797#76808 · In reply to: #76799 > Yes, please. We're all in favor of the 64 bit implementation, but definitely not on > the naming. I think it was a big mistake to not separate the naming and > implementation votes initially. > > I also would support a shorter RFC cycle, due to its blocking nature. > [Vote in 2-3 days, vote lasting one week as usual] No. -- Pierre @pierrejoye | http://www.libgd.org --- ## #76806 — Pierre Joye — 2014-08-22T12:17:04+00:00 URL: https://externals.io/message/76797#76806 · In reply to: #76797 > Hi internals! > > Today the int64 RFC has been merged, despite objections regarding the > naming changes it introduces. > > As we were not given a chance to resolve this issue before the merge, a > short proposal has been created, which aims to revert all unnecessary > naming changes and instead use type names that are consistent with the > existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > Due to the unexpected merge on short notice, with no chance for discussion, > this issue is blocking a number of other patches. As such I ask if we can > move through this RFC with a shortened cycle. I would not appreciate having > to wait three weeks to have a workable codebase again. I also request to let me add options for actual good naming (see my previous reply). I would also strongly suggest to group these renaming actions in one RFC and get rid of it in the next couple of weeks (following the usual periods for a RFC). STR macros being on top of my list. And I'd to remember that we suffer from double standards while you guys do whatever you want with ng, and certainly with ast as well. I wonder what you would say if we begin to be as picky and keep asking to change things for months :/ Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76810 — Jan Ehrhardt — 2014-08-22T12:39:20+00:00 URL: https://externals.io/message/76797#76810 · In reply to: #76806 Pierre Joye in php.internals (Fri, 22 Aug 2014 14:17:04 +0200): >And I'd to remember that we suffer from double standards while you >guys do whatever you want with ng, and certainly with ast as well. Just a side note: talking in "you guys" and "we" is not constructive. I had the idea you had come a lot closer while working on the 64bit patch. Do not spoil that. Jan --- ## #76813 — christopher jones — 2014-08-22T15:06:42+00:00 URL: https://externals.io/message/76797#76813 · In reply to: #76808 > > > > […] > > No. > Since this is obviously contentious it should follow the normal RFC cycle. Chris -- christopher.jones@oracle.com http://twitter.com/ghrd Free PHP & Oracle book: http://www.oracle.com/technetwork/topics/php/underground-php-oracle-manual-098250.html --- ## #76815 — Andrea Faulds — 2014-08-22T15:32:51+00:00 URL: https://externals.io/message/76797#76815 · In reply to: #76797 > As we were not given a chance to resolve this issue before the merge, a > short proposal has been created, which aims to revert all unnecessary > naming changes and instead use type names that are consistent with the > existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > Due to the unexpected merge on short notice, with no chance for discussion, > this issue is blocking a number of other patches. As such I ask if we can > move through this RFC with a shortened cycle. I would not appreciate having > to wait three weeks to have a workable codebase again. I’m very much in favour of this RFC. It is not necessary to change the type names after all; if people turn on compiler warnings, they’ll find out what’s broken anyway. -- Andrea Faulds http://ajf.me/ --- ## #76820 — Ferenc Kovacs — 2014-08-22T16:59:01+00:00 URL: https://externals.io/message/76797#76820 2014.08.22. 17:56 ezt írta ("Jan Ehrhardt" ): > > Andrea Faulds in php.internals (Fri, 22 Aug 2014 16:32:51 +0100): > > […] > > 'people'? Which people? > > Will a normal user ever notice anything of the change from long to int? > Isn't this something that only developers of extensions will notice? My > guess is that this chnage is only a minor point when converting > extensions to ng. > People means ext devs here. The concern raised here that if we delay the namechange then early adopters(ext developers adding support for php7) have to update their code twice. I don't think this will major pita (compared to the phpng changes or other upcoming stuff) but would have been nice resolving this before the merge --- ## #76821 — Jan Ehrhardt — 2014-08-22T17:11:15+00:00 URL: https://externals.io/message/76797#76821 · In reply to: #76820 Ferenc Kovacs in php.internals (Fri, 22 Aug 2014 18:59:01 +0200): >People means ext devs here. OK. >The concern raised here that if we delay the namechange then early >adopters(ext developers adding support for php7) have to update their code >twice. Provided the name change (back!) is agreed upon. Now that is has been changed once, there is a chance to look at it with an open mind and decide what is best for the coming years. Jan --- ## #76822 — Pierre Joye — 2014-08-22T18:38:43+00:00 URL: https://externals.io/message/76797#76822 · In reply to: #76815 > > > > > > […] discussion, > > […] can > > […] having > > […] > > I’m very much in favour of this RFC. It is not necessary to change the type names after all; if people turn on compiler warnings, they’ll find out what’s broken anyway. That's one of my points. They won't for all cases. And why good names reflecting the underlying type will help. It is what the 1st version if the int64 rfc did. --- ## #76825 — Rasmus Lerdorf — 2014-08-23T02:05:33+00:00 URL: https://externals.io/message/76797#76825 · In reply to: #76822 > > > […] > discussion, > > […] > can > > […] > having > > […] > type names after all; if people turn on compiler warnings, they’ll find out > what’s broken anyway. > > That's one of my points. They won't for all cases. And why good names > reflecting the underlying type will help. It is what the 1st version if the > int64 rfc did. The end result here is likely that in order to write a portable extension, extension authors will simply do: #ifndef IS_LONG # define IS_LONG IS_INT # define Z_STRLEN Z_STRSIZE # define RETURN_LONG RETURN_INT # define RETVAL_LONG RETVAL_INT # define Z_LVAL Z_IVAL ... #endif and not change a single macro anywhere in the actual code. That's certainly what I would do. And if the current naming survives we should provide a macro_compat.h file with the complete set. Overall I don't think trying to make the macro names match the underlying types exactly is worth the hassle here. Our audience for this is extension authors. In most cases the author doesn't necessarily care about the subtle differences between calling it IS_LONG and IS_INT and I don't think it actually makes it any more clear to a C developer. "int" is not a well-defined cross-platform type anymore than a "long" is. So we are swapping one set of vague names for another set of vague names. I'd much rather see a nice clear README.TYPES or perhaps it is simply part of the existing README.PARAMETER_PARSING_API which defines exactly what these various macros mean. -Rasmus --- ## #76826 — Andrea Faulds — 2014-08-23T02:10:07+00:00 URL: https://externals.io/message/76797#76826 · In reply to: #76825 > The end result here is likely that in order to write a portable > extension, extension authors will simply do: > > #ifndef IS_LONG > # define IS_LONG IS_INT > # define Z_STRLEN Z_STRSIZE > # define RETURN_LONG RETURN_INT > # define RETVAL_LONG RETVAL_INT > # define Z_LVAL Z_IVAL > ... > #endif > > and not change a single macro anywhere in the actual code. That’s a very good point. It’s easy to forget that many people write cross-version compatible extensions. > That's certainly what I would do. And if the current naming survives we > should provide a macro_compat.h file with the complete set. Perhaps we could backport the new types, too, and make them just aliases on older PHP versions? For example, zend_int would be a long in 5.5 and 5.6, but a 64-bit-on-64-bit type on 7. > Overall I don't think trying to make the macro names match the > underlying types exactly is worth the hassle here. Our audience for this > is extension authors. In most cases the author doesn't necessarily care > about the subtle differences between calling it IS_LONG and IS_INT and I > don't think it actually makes it any more clear to a C developer. "int" > is not a well-defined cross-platform type anymore than a "long" is. So > we are swapping one set of vague names for another set of vague names. > I'd much rather see a nice clear README.TYPES or perhaps it is simply > part of the existing README.PARAMETER_PARSING_API which defines exactly > what these various macros mean. +1 to that. -- Andrea Faulds http://ajf.me/ --- ## #76828 — Pierre Joye — 2014-08-23T05:46:03+00:00 URL: https://externals.io/message/76797#76828 · In reply to: #76825 hi Rasmus, > > > […] > > The end result here is likely that in order to write a portable > extension, extension authors will simply do: > > #ifndef IS_LONG > # define IS_LONG IS_INT > # define Z_STRLEN Z_STRSIZE > # define RETURN_LONG RETURN_INT > # define RETVAL_LONG RETVAL_INT > # define Z_LVAL Z_IVAL > ... > #endif > > and not change a single macro anywhere in the actual code. If the int64 RFC was accepted for 5.x, this solution would be perfect (except from a confusing usage and type point of view). But you would also need #ifdef for the declarations when used with zpp, or storing values in variables, for the zval integer and strings values: /* for zval string */ #if `PHP_MAJOR_VERSION` > 6 size_t str_size; char *str; #else int str_size; char *str; #endif Same for zpp argument_ #if `PHP_MAJOR_VERSION` > 6 zend_string path; #else int str_size; char *path; int path_len; #endif Besides that many changes can't be aliased, same APIs or macros use different arguments or argument content, f.e. the hash api: f.e.: - warning at build time: ex 1: - php_basename(path_cleaned, path_cleaned_len, NULL, 0, &file_basename, (size_t *)&file_basename_len TSRMLS_CC); + file_basename = php_basename(path_cleaned, path_cleaned_len, NULL, 0 TSRMLS_CC); ex 2: - add_next_index_string(return_value, fullpath, 1); + add_next_index_string(return_value, fullpath); - or even more dangerous if forgotten (no compile time detection (size and return value): ex 1 (hash functions): - if (zend_hash_find(HASH_OF(options), "add_path", sizeof("add_path"), (void **)&option) == SUCCESS) { + if ((option = zend_hash_str_find(HASH_OF(options), "add_path", sizeof("add_path") - 1)) != NULL) { - zend_hash_add(prop_handler, name, strlen(name)+1, &hnd, sizeof(zip_prop_handler), NULL); + zend_hash_str_add_mem(prop_handler, name, strlen(name), &hnd, sizeof(zip_prop_handler)); ex 2: use of string will need different free calls: - efree(file_basename); + STR_RELEASE(file_basename); > That's certainly what I would do. And if the current naming survives we > should provide a macro_compat.h file with the complete set. The more exts I work on the less I think our current flow for git will work out of the box. I am slowly considering creating separate branches for 7 and 5.x, with different packages for the releases (avoiding to have to upload/download two code bases when only one is used). But this is almost another topic, I will post a mail about it and see how Pickle could help here. > Overall I don't think trying to make the macro names match the > underlying types exactly is worth the hassle here. Our audience for this > is extension authors. In most cases the author doesn't necessarily care > about the subtle differences between calling it IS_LONG and IS_INT and I > don't think it actually makes it any more clear to a C developer. "int" > is not a well-defined cross-platform type anymore than a "long" is. Well, in the context of of the integer case, zend_uint vs zend_int_t, it is. Or it is for the case used in the engine. Nikita and I had a long discussions last night. We came to two conclusions: - zend_uint should go away. - uint32_t should be used instead - zend_int_t could remain . it matches the respective, architecture specific int32_t and int64_t being used behind it. _t standing for _typed just like in the standard types intXX_t - We did not reach an agreement about IS_LONG. My point: If we keep it, it increases the porting bugs. As we already have seen in many extensions, is confusing and devs forgot to check the type. IS_INT matches PHP's integer, which is architecture dependent, devs may be more careful. Nikita's point is about not changing it for changing it along what is described in the RFC (@Nikita, please complete :) Pretty much the same applies to the Z_STRLEN vs T_STRSIZE. Here I came back to an initial proposal, using Z_STRSIZET or Z_STRSIZE_T. > So > we are swapping one set of vague names for another set of vague names. > I'd much rather see a nice clear README.TYPES or perhaps it is simply > part of the existing README.PARAMETER_PARSING_API which defines exactly > what these various macros mean. No matter the outcome of this discussion, these documents must be updated (they are not, partially for int64 and almost not for NG). In other words, it would be nice to see more developers actually porting extensions to realize the amount of changes are introduced by NG and by the int64. The sooner is in order of magnitude must larger. It is not a bad comment, only a fact. Given that, before we choose to say that it is fine for one part to change APIs/Macros signatures or names and not for another, we should really get a better view of what has actually changed. And how we can deal with our old habit to maintain one tree for many major PHP versions. For many extensions, I do not think it will be possible, or with unreadable code with 2-3x more #ifdef all over the place. Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76829 — Rasmus Lerdorf — 2014-08-23T06:05:10+00:00 URL: https://externals.io/message/76797#76829 · In reply to: #76828 > In other words, it would be nice to see more developers actually > porting extensions to realize the amount of changes are introduced by > NG and by the int64. The sooner is in order of magnitude must larger. > It is not a bad comment, only a fact. Given that, before we choose to > say that it is fine for one part to change APIs/Macros signatures or > names and not for another, we should really get a better view of what > has actually changed. And how we can deal with our old habit to > maintain one tree for many major PHP versions. For many extensions, I > do not think it will be possible, or with unreadable code with 2-3x > more #ifdef all over the place. I knew you would make this comparison. I am willing to suffer porting pain if it gets me a 20% performance boost. I am completely unwilling to suffer any porting pain because Pierre has decided he doesn't like the names of some macros. -Rasmus --- ## #76830 — Pierre Joye — 2014-08-23T06:09:56+00:00 URL: https://externals.io/message/76797#76830 · In reply to: #76829 > > > […] > > I knew you would make this comparison. I am willing to suffer porting > pain if it gets me a 20% performance boost. I am completely unwilling to > suffer any porting pain because Pierre has decided he doesn't like the > names of some macros. Sorry Rasmus, this reply is irrelevant or off base. What in my reply makes you tell that it is about my taste? It is the exact same argument you used and about showing that it is not possible to do what you said for many cases. You also totally ignore other valid points, raised by other developers (working on porting exts, like Remi, f.e.). This is something we have to discuss and solve, no matter the outcome of this discussion. Anyway, thanks for this constructive reply. Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76831 — Rasmus Lerdorf — 2014-08-23T06:21:36+00:00 URL: https://externals.io/message/76797#76831 · In reply to: #76830 > > > […] > > Sorry Rasmus, this reply is irrelevant or off base. What in my reply > makes you tell that it is about my taste? There is no technical reason to change IS_LONG to IS_INT along with the others in that category. It is purely a taste/purity/questionable-consistency thing and doesn't need to be done to solve any real technical problem. -Rasmus --- ## #76832 — Pierre Joye — 2014-08-23T06:25:49+00:00 URL: https://externals.io/message/76797#76832 · In reply to: #76831 > > > […] > > There is no technical reason to change IS_LONG to IS_INT along with the > others in that category. It is purely a > taste/purity/questionable-consistency thing and doesn't need to be done > to solve any real technical problem. There is. Long is not used anymore. So we can argue about that but there is a change and it should be reflected imo. You still totally ignore any of my other points, which are even more important in regard to maintaining one code tree for 5.x and 7.x, which is very likely not possible in a sane way for many extensions. -- Pierre @pierrejoye | http://www.libgd.org --- ## #76833 — Dmitry Stogov — 2014-08-23T09:43:48+00:00 URL: https://externals.io/message/76797#76833 · In reply to: #76804 I think it's ok to keep zend_off_t and zend_size_t as is. Thanks. Dmitry, On Fri, Aug 22, 2014 at 4:09 PM, Anatol Belski wrote: > Hi Nikita, > > > > […] > to be complete, the RFC probably should take in account several new > identifiers which came in (zend_off_t, zend_stat_t and co.). As their > naming scheme is probably somewhat different. I've just documented those > http://git.php.net/?p=php-src.git;a=blob;f=UPGRADING.INTERNALS and > continue. > > Regards > > Anatol --- ## #76834 — Dmitry Stogov — 2014-08-23T09:55:23+00:00 URL: https://externals.io/message/76797#76834 · In reply to: #76797 100% agree with RFC. It would be great if technical improvements wouldn't be messed with this renaming :( Thanks. Dmitry, > Hi internals! > > Today the int64 RFC has been merged, despite objections regarding the > naming changes it introduces. > > As we were not given a chance to resolve this issue before the merge, a > short proposal has been created, which aims to revert all unnecessary > naming changes and instead use type names that are consistent with the > existing code base and expectations: > > https://wiki.php.net/rfc/better_type_names_for_int64 > > Due to the unexpected merge on short notice, with no chance for discussion, > this issue is blocking a number of other patches. As such I ask if we can > move through this RFC with a shortened cycle. I would not appreciate having > to wait three weeks to have a workable codebase again. > > Nikita --- ## #76835 — Pierre Joye — 2014-08-23T10:16:08+00:00 URL: https://externals.io/message/76797#76835 · In reply to: #76834 > > 100% agree with RFC. > It would be great if technical improvements wouldn't be messed with this > renaming :( You realize that 80%+ of the zval macros has to be changed as well right? And not only the name but the argument and their content. I won't battle for this naming, but seriously the arguments listed on this thread are amusing... Cheers, Pierre --- ## #76836 — Andrea Faulds — 2014-08-23T14:33:58+00:00 URL: https://externals.io/message/76797#76836 · In reply to: #76833 > I think it's ok to keep zend_off_t and zend_size_t as is. Could someone enlighten me as to why we need zend_size_t? Isn’t that just a typedef for size_t? It’s not like it’ll vary on different platforms, doing so would defeat the purpose of using size_t in the first place. -- Andrea Faulds http://ajf.me/ --- Next page: https://externals.io/message/76797.md?page=2