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
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_int64Due 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 <PHP7 and PHP7, we don't need it to get harder, it is not
enough to say that "we have to work something out".
Fix it, fix it, fix it !!
Cheers
Joe
Am 22.8.2014 um 13:16 schrieb Nikita Popov nikita.ppv@gmail.com:
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
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
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.
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
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_int64Due 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
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
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 anatol.php@belski.net
wrote:
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
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/
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.
Was not desired back then as it "changed too much". So it simply matched
what already exists for years.
To me we should just use size_t and uint32_t for zend_uint (solving the
zend_uint vs zend_into_t).
Cheers,
Pierre
uint32_t for zend_uint (solving the
zend_uint vs zend_into_t).
uint32_t isn’t universally available. MSVC 2010 does not have it, for example.
Andrea Faulds
http://ajf.me/
uint32_t for zend_uint (solving the
zend_uint vs zend_into_t).uint32_t isn’t universally available. MSVC 2010 does not have it, for
example.
It would be nice to at least read the code before commenting in this thread.
We have types detections in place, since long for windows, and 5.4 or 5.5
for other platforms. It is only matter to use the php_stdint file in the
engine.
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_int64Due 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
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_int64Due 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
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
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/
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.
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.
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.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
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
...
#endifand 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/
hi Rasmus,
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.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
...
#endifand 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
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
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.
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
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.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
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.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.
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
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.
I ignored it because it is irrelevant. The fact that many related things
are changing does not justify piling on more changes. Every change
should have a very solid technical reason behind it. "Long is not used
anymore" is not a solid technical reason. The code will compile
perfectly fine with it being IS_LONG. Also, these are userspace-facing
in the sense that as an extension author we are dealing with PHP's type
system consisting of lval, dval, string, array, object and resource. How
exactly an lval or IS_LONG is implemented at the C level on any given
platform is an implementation detail and could change, but we are still
providing an lval to userspace. As C developers, extension authors are
more than capable of checking the actual macro definition if they want
to know the details.
I'd also appreciate if you would drop your toxicity level a few notches
in your emails to the list, irc and twitter.
Thanks
-Rasmus
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.I ignored it because it is irrelevant.
These points are not irrelevant:
- 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
And we can add:
- zend_size_t can be dropped in favor of size_t
And dangerous APIs signature changes are not irrelevant either. And
many of them are not related to performance at all but a choice made
at the implementation time. So the solid technical reason you mention
later does not apply here.
On the same area, I would also prefer not to use STR_* defines but
directly called zend_string_*, it will drastically reduce errors
during porting or later.
The fact that many related things
are changing does not justify piling on more changes.
RIght.
Every change
should have a very solid technical reason behind it. "Long is not used
anymore" is not a solid technical reason.
Consistent APIs, in my book, is a solid technical reason.
The code will compile
perfectly fine with it being IS_LONG. Also, these are userspace-facing
in the sense that as an extension author we are dealing with PHP's type
system consisting of lval, dval, string, array, object and resource. How
exactly an lval or IS_LONG is implemented at the C level on any given
platform is an implementation detail and could change, but we are still
providing an lval to userspace.
Extensions write should not use the zend_value member directly but use
the provided APIs or macros, which is called Z_IVAL btw. Want to
change to Z_LVAL? WIll be more consistent.
As C developers, extension authors are
more than capable of checking the actual macro definition if they want
to know the details.
I have not doubt about the capabilities of the extension developers,
but improving APIs quality reduces the # of errors.
I'd also appreciate if you would drop your toxicity level a few notches
in your emails to the list, irc and twitter.
I do not agree with your definition, but as long as there are double
standards and related behaviors, I do not see how things can improve.
The way this RFC has been handled is a shame. If NG would have got as
much code reviews, got blocked twice while being accepted, etc. I do
not think Dmitry or Laruence would be that happy at this point. And
seriously, this is a pretty bad behavior. It is also not the 1st time
it happens. Only difference is that I do not care much when something
happens, I know each of you long enough, but other simply left.
Anyway, it does not help anyone to argue about that over and over, I
can agree with you on that.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
On Sat, Aug 23, 2014 at 6:40 PM, Rasmus Lerdorf rasmus@lerdorf.com
wrote: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.I ignored it because it is irrelevant.
These points are not irrelevant:
- 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_tAnd we can add:
- zend_size_t can be dropped in favor of size_t
And dangerous APIs signature changes are not irrelevant either. And
many of them are not related to performance at all but a choice made
at the implementation time. So the solid technical reason you mention
later does not apply here.On the same area, I would also prefer not to use STR_* defines but
directly called zend_string_*, it will drastically reduce errors
during porting or later.The fact that many related things
are changing does not justify piling on more changes.RIght.
Every change
should have a very solid technical reason behind it. "Long is not used
anymore" is not a solid technical reason.Consistent APIs, in my book, is a solid technical reason.
The code will compile
perfectly fine with it being IS_LONG. Also, these are userspace-facing
in the sense that as an extension author we are dealing with PHP's type
system consisting of lval, dval, string, array, object and resource. How
exactly an lval or IS_LONG is implemented at the C level on any given
platform is an implementation detail and could change, but we are still
providing an lval to userspace.Extensions write should not use the zend_value member directly but use
the provided APIs or macros, which is called Z_IVAL btw. Want to
change to Z_LVAL? WIll be more consistent.As C developers, extension authors are
more than capable of checking the actual macro definition if they want
to know the details.I have not doubt about the capabilities of the extension developers,
but improving APIs quality reduces the # of errors.I'd also appreciate if you would drop your toxicity level a few notches
in your emails to the list, irc and twitter.I do not agree with your definition, but as long as there are double
standards and related behaviors, I do not see how things can improve.
The way this RFC has been handled is a shame. If NG would have got as
much code reviews, got blocked twice while being accepted, etc. I do
not think Dmitry or Laruence would be that happy at this point. And
seriously, this is a pretty bad behavior. It is also not the 1st time
it happens. Only difference is that I do not care much when something
happens, I know each of you long enough, but other simply left.
Anyway, it does not help anyone to argue about that over and over, I
can agree with you on that.Cheers,
Pierre
@pierrejoye | http://www.libgd.org
--
I have a quick question for the people debating this issue: Aside from the
dispute over whether or not it's necessary / unimportant, are there any
pressing reasons not to implement the changes that Pierre is advocating?
I.e. would it break anything, waste a large amount of time, make the code
harder to maintain, etc? Or is it just that you don't think it's worth
doing?
--Kris
I have a quick question for the people debating this issue: Aside from the
dispute over whether or not it's necessary / unimportant, are there any
pressing reasons not to implement the changes that Pierre is advocating?
I.e. would it break anything, waste a large amount of time, make the code
harder to maintain, etc? Or is it just that you don't think it's worth
doing?
I can think of two:
-
It breaks most extensions out there, perhaps unnecessarily.
-
If bigints are implemented, we’d have to rename everything again.
--
Andrea Faulds
http://ajf.me/
I have a quick question for the people debating this issue: Aside from the
dispute over whether or not it's necessary / unimportant, are there any
pressing reasons not to implement the changes that Pierre is advocating?
I.e. would it break anything, waste a large amount of time, make the code
harder to maintain, etc? Or is it just that you don't think it's worth
doing?I can think of two:
- It breaks most extensions out there, perhaps unnecessarily.
Please try to port one. That will solve this never ending ping pong
game. Extensions are broken per se with ng, almost every zval macros
usage must change (some disappeared, like the _PP ones), all hash APIs
call must be change (a must, not detectable at compile time), etc.
IS_LONG to IS_INT is a joke in comparison. But as nobody agrees on
that, I won't discuss it to death.
- If bigints are implemented, we’d have to rename everything again.
Ah, and that will be acceptable then, right? ;-)
Also hurry up with that, even if not totally completed. Many
extensions may have to deal with it and it will just double the
porting work if it is not done soon.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Please try to port one. That will solve this never ending ping pong
game. Extensions are broken per se with ng, almost every zval macros
usage must change (some disappeared, like the _PP ones), all hash APIs
call must be change (a must, not detectable at compile time), etc.
IS_LONG to IS_INT is a joke in comparison. But as nobody agrees on
that, I won't discuss it to death.
You previously told me on this list that if I maintained an extension, I’d know that replacing macro names with a find/replace is a big deal.
- If bigints are implemented, we’d have to rename everything again.
Ah, and that will be acceptable then, right? ;-)
It wouldn’t be needless, it would be done to reduce confusion.
Also hurry up with that, even if not totally completed. Many
extensions may have to deal with it and it will just double the
porting work if it is not done soon.
I’ll look into it.
Andrea Faulds
http://ajf.me/
Please try to port one. That will solve this never ending ping pong
game. Extensions are broken per se with ng, almost every zval macros
usage must change (some disappeared, like the _PP ones), all hash APIs
call must be change (a must, not detectable at compile time), etc.
IS_LONG to IS_INT is a joke in comparison. But as nobody agrees on
that, I won't discuss it to death.You previously told me on this list that if I maintained an extension, I’d know that replacing macro names with a find/replace is a big deal.
Excatly, and IS_LONG/INT are not macros and a very very tiny change in
comparison to anything else, be ng alone, or ng+int64.
- If bigints are implemented, we’d have to rename everything again.
Ah, and that will be acceptable then, right? ;-)
It wouldn’t be needless, it would be done to reduce confusion.
Also hurry up with that, even if not totally completed. Many
extensions may have to deal with it and it will just double the
porting work if it is not done soon.I’ll look into it.
If you need help (windows or other), let me know. Want it in :)
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
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_int64Due 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
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