Re: [RFC] 64 bit platform improvements for string length and integer

On Fri, Jan 10, 2014 at 3:11 PM, Anatol Belski <ab@php.net> wrote: > Hi Nikita, > > On Fri, January 10, 2014 22:42, Nikita Popov wrote: >> On Fri, Jan 10, 2014 at 3:58 PM, Anatol Belski <ab@php.net> wrote: >> >> >>> Hi, >>> >>> >>> https://wiki.php.net/rfc/size_t_and_int64 >>> >>> >>> The discussion time has come. The work on the feature branch continues. >>> The current patch is stable enough to be discussed. >>> >>> >> >> What is the reason behind the renames of IS_LONG to IS_INT (and Z_LVAL to >> Z_IVAL etc), as well as the renames in zpp (s -> S, etc)? Why can't we >> keep the old names here? That should reduce the amount of ifndefs involved >> a lot, as you'd only have to do it for the type declarations themselves, >> not for every single usage. Or is the point here to intentionally provide >> a maximum amount of BC breakage, so code doesn't "accidentally" continue >> to run (e.g. I think that without the renames extensions could continue to >> run mostly without issue on 32bit.) >> >> Nikita >> > the renames you mention like IS_LONG -> IS_INT are thought more for > correct semantic, as there is no firm 'long' anymore. The same for > Z_STRLEN -> Z_STRSIZE and others. That kind of thing should be done the > most obvious way. Well, the max BC reason you gave I like too :) > > For the same reason zend_parse_parameters() formats was changed, as it'll > issue an error on runtime. However here I still scratch my head as that's > a runtime issue, but it should break the compilation as well. > > Without semantic replacements many extensions would just continue to > compile/run on 32 bit, indeed. Though because of the size_t one could > still have some unpleasant surprise in some situation. Besides that, I'd > really see that more like a side effect with not very clear use. > > With the #ifdef's - there shouldn't be any or should be very few. The > compatibility header I've mentioned in one of the previous responses > should do it one for all. It can look like > > #if PHP_MAJOR_VERSION < 6 > # define IS_INT IS_LONG > # define php_size_t int > ....... > #endif > > Once included and given an extension is compiled with an older PHP > version, that defines should cover the compatibility with older semantics. > Some more complicated solution will probably be needed for zpp to replace > "i" with "l" and co. for the older PHP. But generally, such a header > should make the same ext source in 6.x style compatible with 5.x branch. > Of course some exceptions will have to take place, but i think those will > be not more than the current mainstream already contains to separate the > code for minor versions in 5.x branch. > > The migration path is the very next thing I have to do. I am really confused on that. How is the compatibility header useful if I still need a ifdef else for zpp and things that boil down to printfs, like error reporting, with 2 different arguments? Wouldn't it be better to not change them, but maybe force extensions to define a _I_SUPPORT_PHP6, if not defined refuse to build the ext against PHP6? Then all I have to do in extension is to include the compat header, change some of my types passed to zpp/printfs and define that macro.. and I could support PHP5 and PHP6. And if I try to build against PHP6 it would result in compiler failure... We'd maybe need to tweak the build system a tiny bit, and phpize, for that to happen - but that seem to be much easier then supporting PHP5 and 6 for extensions in the same codebase with renamed zpp values? -Hannes