# Naming issues, solutions - Thread: https://externals.io/message/76848 - Original archive: https://news-web.php.net/php.internals/76848 - Started by Pierre Joye on 2014-08-24T12:30:51+00:00 - Messages: 10 - 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 --- ## #76848 — Pierre Joye — 2014-08-24T12:30:51+00:00 URL: https://externals.io/message/76848#76848 Hi, Nikita, Xinchen and I have sit down together this morning to solve a couple of naming issues, those having caused some flames, trolls and FUDs in the last couple of days. The good news is we have found a consensus that solves our concerns, consistency and reduce the amount of changes in existing code. Please not that this mail is not my personal taste or wishes, but the results of a refreshing, constructive and good discussions with Xinchen and Nikita earlier this morning. Summary: zend_uint > int32_t zend_size_t > size_t zend_int_t > zend_long IS_INT > IS_LONG Z_IVAL > Z_LVAL Z_STRSIZE > Z_STRLEN zend_str_* > zend_string_* STR_* macros > droped Details: We have a header available by default to define c9* stdint, see main/php_stdint.h. This header ensures consistency accross platforms and that the size of each type is correct. It is important to keep that in mind while reading the rest of this mail. * zend_uint zend_uint is used exclusevely in the engine (maybe some debugger or engine related extensions) and has been around for a while. We like to drop it and use int32_t instead anywhere where zend_uint is used. Using int32_t is good because this will ensure that we use the correct type even on ILP64 - zend_uint would currently totally blow up on those systems. *zend_size_t Given that size_t is a standard type, adding zend_size_t makes little sense. * zend_int_t, zend_long, IS_INT/IS_LONG One of my main concerns is consistency between APIs and macros names and the underlying types. A solution to this problem was quite obvious (only not on Friday ;). zend_int_t will be renamed to zend_long, IS_INT and Z_IVAL will renamed to their old names, IS_LONG and Z_LVAL, respectively. * Strings First change, Z_STRSIZE will be renamed to Z_STRLEN (old name). We also like to drop STR_ macros and rely directly on the zend_string_* APIs. While doing at it the zend_string APIs will be prefix zend_string and not zend_str_* as it is now, for the same reason: During the NG works and the ports of many extensions, numerous bugs appear due to bad usage of STR_* macros with either char* or other char/string related types. Making the name more obvious will help. PS: IRC log available if anyone likes to double check it, #php.pecl Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76849 — Laruence — 2014-08-24T12:34:37+00:00 URL: https://externals.io/message/76848#76849 · In reply to: #76848 Hey: > Hi, > > Nikita, Xinchen and I have sit down together this morning to solve a > couple of naming issues, those having > caused some flames, trolls and FUDs in the last couple of days. The > good news is we have found a consensus > that solves our concerns, consistency and reduce the amount of changes > in existing code. > > Please not that this mail is not my personal taste or wishes, but the > results of a refreshing, constructive > and good discussions with Xinchen and Nikita earlier this morning. > > Summary: > zend_uint > int32_t > zend_size_t > size_t > zend_int_t > zend_long > IS_INT > IS_LONG > Z_IVAL > Z_LVAL > Z_STRSIZE > Z_STRLEN > zend_str_* > zend_string_* > STR_* macros > droped > > Details: > > We have a header available by default to define c9* stdint, see > main/php_stdint.h. This header ensures > consistency accross platforms and that the size of each type is > correct. It is important to keep > that in mind while reading the rest of this mail. > > * zend_uint > zend_uint is used exclusevely in the engine (maybe some debugger or > engine related extensions) and has been around for a while. We like to > drop it and use int32_t instead anywhere where zend_uint is used. > Using int32_t is good because this will ensure that we use the correct > type even on ILP64 - zend_uint would currently totally blow up on > those systems. > > *zend_size_t > Given that size_t is a standard type, adding zend_size_t makes little sense. > > * zend_int_t, zend_long, IS_INT/IS_LONG > One of my main concerns is consistency between APIs and macros names > and the underlying types. A solution to this problem was quite > obvious (only not on Friday ;). zend_int_t will be renamed to > zend_long, IS_INT and Z_IVAL will renamed to their old names, IS_LONG > and Z_LVAL, respectively. > > * Strings > First change, Z_STRSIZE will be renamed to Z_STRLEN (old name). We > also like to drop STR_ macros and rely directly on the zend_string_* > APIs. While doing at it the zend_string APIs will be prefix > zend_string and not zend_str_* as it is now, for the same reason: > > During the NG works and the ports of many extensions, numerous bugs > appear due to bad usage of STR_* macros with either char* or other > char/string related types. Making the name more obvious will help. > > > PS: IRC log available if anyone likes to double check it, #php.pecl thanks. I am glad we have reach a agreement here. actually to me, the naming issue has blocked me to start porting work on my own exts, so if no objections, I'd like to see it settled down and commit to trunk asap :) thanks > > Cheers, > -- > Pierre > > @pierrejoye | http://www.libgd.org > > -- > > > -- Xinchen Hui @Laruence http://www.laruence.com/ --- ## #76850 — me@rouvenwessling.de — 2014-08-24T13:20:17+00:00 URL: https://externals.io/message/76848#76850 · In reply to: #76848 > Am 24.08.2014 um 14:30 schrieb Pierre Joye : > > zend_uint > int32_t Is that a typo? If it's not, why are you changing it to signed? Best regards Rouven --- ## #76851 — Pierre Joye — 2014-08-24T13:21:52+00:00 URL: https://externals.io/message/76848#76851 · In reply to: #76850 > > > > […] > > Is that a typo? If it's not, why are you changing it to signed? typo. It should be: zend_uint > uint32_t Thanks :) > Best regards > Rouven -- Pierre @pierrejoye | http://www.libgd.org --- ## #76853 — Dmitry Stogov — 2014-08-25T07:22:57+00:00 URL: https://externals.io/message/76848#76853 · In reply to: #76851 Hi Pierre, I'm glad, you agree to rename IS_INT back to IS_LONG. zend_long and size_t usage looks fine. I see no problems changing zend_string related API and related macros introduced in NG. They are new for everyone. I hope, the changes won't make API less clear or useful (so it would be great to review them). I don't see a big reason to rename "zend_uint" into "uint32_t", but it's just my own opinion. I would prefer keep it as is, or at worse case rename into "zend_uint32" or "uint32". Anyway, I'll agree with majority. How are we going to proceed? Do you like voting in https://wiki.php.net/rfc/better_type_names_for_int64 or you may just revert part of the int64 patch related to IS_LONG -> IS_INT renaming? Who is going to implement the rest? When? Do we need RFC? May be it should be a general RFC about internal PHP APIs cleanup? I know, there are a lot of other inconsistencies... Thanks. Dmitry. > On Sun, Aug 24, 2014 at 3:20 PM, Rouven Weßling > wrote: > > […] > > typo. It should be: > > zend_uint > uint32_t > > Thanks :) > > > […] > > > > -- > Pierre > > @pierrejoye | http://www.libgd.org --- ## #76860 — Pierre Joye — 2014-08-25T10:45:02+00:00 URL: https://externals.io/message/76848#76860 · In reply to: #76853 > > Hi Pierre, > > I'm glad, you agree to rename IS_INT back to IS_LONG. > > zend_long and size_t usage looks fine. > > I see no problems changing zend_string related API and related macros introduced in NG. They are new for everyone. > I hope, the changes won't make API less clear or useful (so it would be great to review them). > > I don't see a big reason to rename "zend_uint" into "uint32_t", but it's just my own opinion. I would prefer keep it as is, or at worse case rename into "zend_uint32" or "uint32". Anyway, I'll agree with majority. > uint32_t :) > How are we going to proceed? > Do you like voting in https://wiki.php.net/rfc/better_type_names_for_int64 or you may just revert part of the int64 patch related to IS_LONG -> IS_INT renaming? We will just do the changes listed in the initial mail. The sooner the better. Anatol or I will do it. It should be ready by Wednesday. If we can avoid a 2-3 weeks delay let not do the rfc. As these changes match what was discussed and fit with the majority (subjective part as only the vocal ones), we should not see any objection. We will simply apply it on Wednesday if nobody complained. > Who is going to implement the rest? When? Do we need RFC? > May be it should be a general RFC about internal PHP APIs cleanup? > I know, there are a lot of other inconsistencies... Yes there are other but less wisely spread within extension code. Let check them later. Cheers, Pierre --- ## #76861 — Dmitry Stogov — 2014-08-25T10:56:33+00:00 URL: https://externals.io/message/76848#76861 · In reply to: #76860 > > > > […] > introduced in NG. They are new for everyone. > > […] > great to review them). > > […] > just my own opinion. I would prefer keep it as is, or at worse case rename > into "zend_uint32" or "uint32". Anyway, I'll agree with majority. > > […] > > uint32_t :) > 3 people are not the majority (or may be I missed discussion on IRC). It's better to think before changing something in many places without a real reason. > > […] > https://wiki.php.net/rfc/better_type_names_for_int64 or you may just > revert part of the int64 patch related to IS_LONG -> IS_INT renaming? > > We will just do the changes listed in the initial mail. The sooner the > better. Anatol or I will do it. It should be ready by Wednesday. If we can > avoid a 2-3 weeks delay let not do the rfc. As these changes match what was > discussed and fit with the majority (subjective part as only the vocal > ones), we should not see any objection. We will simply apply it on > Wednesday if nobody complained. > Agree, we need it ASAP. I'll try not to commit anything big to not make you additional troubles. Thanks. Dmitry. > > […] > > Yes there are other but less wisely spread within extension code. Let > check them later. > > Cheers, > Pierre --- ## #76862 — Pierre Joye — 2014-08-25T10:59:57+00:00 URL: https://externals.io/message/76848#76862 · In reply to: #76861 > > > > > > > […] introduced in NG. They are new for everyone. > > […] be great to review them). > > […] it's just my own opinion. I would prefer keep it as is, or at worse case rename into "zend_uint32" or "uint32". Anyway, I'll agree with majority. > > […] > > 3 people are not the majority (or may be I missed discussion on IRC). > It's better to think before changing something in many places without a real reason. Why do we need it when the standard type exists and does exactly what we want? Same for size_t. It is mostly contained in the engine, no other impact and avoid yet another type definition for existing standard type (we have php_stdint.h to make them always available). > > […] https://wiki.php.net/rfc/better_type_names_for_int64 or you may just revert part of the int64 patch related to IS_LONG -> IS_INT renaming? > > […] better. Anatol or I will do it. It should be ready by Wednesday. If we can avoid a 2-3 weeks delay let not do the rfc. As these changes match what was discussed and fit with the majority (subjective part as only the vocal ones), we should not see any objection. We will simply apply it on Wednesday if nobody complained. > > Agree, we need it ASAP. > I'll try not to commit anything big to not make you additional troubles. > > Thanks. Dmitry. Thanks Cheers, Pierre --- ## #76863 — Dmitry Stogov — 2014-08-25T11:14:38+00:00 URL: https://externals.io/message/76848#76863 · In reply to: #76862 > > > > […] > wrote: > > […] > introduced in NG. They are new for everyone. > > […] > be great to review them). > > […] > it's just my own opinion. I would prefer keep it as is, or at worse case > rename into "zend_uint32" or "uint32". Anyway, I'll agree with majority. > > […] > real reason. > > Why do we need it when the standard type exists and does exactly what we > want? Same for size_t. It is mostly contained in the engine, no other > impact and avoid yet another type definition for existing standard type (we > have php_stdint.h to make them always available). > We never had zend_size_t before and used size_t. but we had zend_uint for ages and changing it to new name in thousand places won't make a lot of sense. If you like to make it always be a 32-bit number, just change the definition. If you are going to rename zend_uint, should we expect renaming of all the similar zend_* types from zend_types.h? typedef unsigned char zend_bool; typedef unsigned char zend_uchar; typedef unsigned int zend_uint; typedef unsigned long zend_ulong; typedef unsigned short zend_ushort; Thanks. Dmitry. > > […] > https://wiki.php.net/rfc/better_type_names_for_int64 or you may just > revert part of the int64 patch related to IS_LONG -> IS_INT renaming? > > […] > better. Anatol or I will do it. It should be ready by Wednesday. If we can > avoid a 2-3 weeks delay let not do the rfc. As these changes match what was > discussed and fit with the majority (subjective part as only the vocal > ones), we should not see any objection. We will simply apply it on > Wednesday if nobody complained. > > […] > > Thanks > > Cheers, > Pierre --- ## #76864 — Pierre Joye — 2014-08-25T11:20:07+00:00 URL: https://externals.io/message/76848#76864 · In reply to: #76863 > > > > > > > […] wrote: > > […] macros introduced in NG. They are new for everyone. > > […] would be great to review them). > > […] it's just my own opinion. I would prefer keep it as is, or at worse case rename into "zend_uint32" or "uint32". Anyway, I'll agree with majority. > > […] a real reason. > > […] want? Same for size_t. It is mostly contained in the engine, no other impact and avoid yet another type definition for existing standard type (we have php_stdint.h to make them always available). > > We never had zend_size_t before and used size_t. > but we had zend_uint for ages and changing it to new name in thousand places won't make a lot of sense. > If you like to make it always be a 32-bit number, just change the definition. > If you are going to rename zend_uint, should we expect renaming of all the similar zend_* types from zend_types.h? > > typedef unsigned char zend_bool; > typedef unsigned char zend_uchar; > typedef unsigned int zend_uint; > typedef unsigned long zend_ulong; > typedef unsigned short zend_ushort; That would be a very good thing IMHO. These types exist because of the lack of stdint. Now that we have it it could be easier to use them when we can. Nikita?