Hi Hannes,
On Sat, January 11, 2014 02:33, Hannes Magnusson wrote:
> On Fri, Jan 10, 2014 at 3:11 PM, Anatol Belski <[email protected]> wrote:
>
>> Hi Nikita,
>>
>>
>> On Fri, January 10, 2014 22:42, Nikita Popov wrote:
>>
>>> On Fri, Jan 10, 2014 at 3:58 PM, Anatol Belski <[email protected]> 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?
with the php_error_docref() - very good point, the header can't help much.
Except to define something like php_error_docref_comp() as a wrapper
function in the compat header, so %pd and alike can be replaced on
runtime. The code might miss some #ifdefs then, so be cleaner.
> 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?
>
Yeah, I'd say an explicit define to indicate PHP6 readiness is good point
as well.
About zpp I've got a click - the compatibility with 5.x can be integrated
into zpp itself in 6.x. I mean look here
http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_API.c;hb=refs/heads/str_size_and_int64#l326
- 'l' and 'L' was replaced with 'i' and 'I', but nothing
prevents to turn
those formats as aliases in 6.x. That way 'l' and 'i' and another pairs
will do the same thing, new implementations can use clean semantics, and
the old formats can be removed after 5.x EOL. This solution however won't
force the new semantic.
Another way i could suggest for zpp is similarly to docrefs using a
wrapper like zend_parse_parameters_comp(), which would replace new for
old. Still not very nice but would guarantee semantically correct formats.
> 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?
>
Sounds plausible to me. Parameters parsing and spprintf are the only
cracky places I can think of now, the other macros/function renames should
be easy covered with the compat header, so can me machinable replaced with
some tool, even sed script.
The _I_SUPPORT_PHP6 def (should be defined before includes) can be checked
directly in php.h, where an #error can be thrown. That sounds like a sane
migration workflow. Like if I'm not aware of 6.x and try to compile just
right on, it'll reject. If I explicitly say, that i'm ready for 6.x, means
i've done the porting work before.
So for the migration it'd be like
- run the replacement tool on the sources for simple semantic fixes
- include compat header
- #define PHP6_SUPPORTED
- replace appropriate things with *_compat() wrappers or do some #ifdefs
- fix datatypes for function args
- fix the extension code where appropriate (that might be big or not, as
Jan meant some exts he's tried was easy)
- check if it plays good with some lib i link against, do some range
checks eventually
Sounds like a plan.
Thanks for the ideas!
Anatol