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
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 > dropedDetails:
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/
Am 24.08.2014 um 14:30 schrieb Pierre Joye pierre.php@gmail.com:
zend_uint > int32_t
Is that a typo? If it's not, why are you changing it to signed?
Best regards
Rouven
Am 24.08.2014 um 14:30 schrieb Pierre Joye pierre.php@gmail.com:
zend_uint > int32_t
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
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 me@rouvenwessling.de
wrote:Am 24.08.2014 um 14:30 schrieb Pierre Joye pierre.php@gmail.com:
zend_uint > int32_t
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
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
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 :)
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.
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.
Agree, we need it ASAP.
I'll try not to commit anything big to not make you additional troubles.
Thanks. Dmitry.
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
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 :)
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).
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.Agree, we need it ASAP.
I'll try not to commit anything big to not make you additional troubles.Thanks. Dmitry.
Thanks
Cheers,
Pierre
On Mon, Aug 25, 2014 at 2:45 PM, Pierre Joye pierre.php@gmail.com
wrote: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 :)
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).
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.
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.Agree, we need it ASAP.
I'll try not to commit anything big to not make you additional troubles.Thanks. Dmitry.
Thanks
Cheers,
Pierre
On Mon, Aug 25, 2014 at 2:45 PM, Pierre Joye pierre.php@gmail.com
wrote: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 :)
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).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?