Convert return type of various object handlers from int to zend_result

[iluuu1994]

No description provided.

[Girgias]

I hesitated doing such a change as this risks breaking more or less every extension which uses objects.

But if we are going to do this, wouldn't it make more sense to convert all relevant object handlers to zend_result?

[iluuu1994]

zend_result has the same size as int on x64. I'm not sure how it is for other architectures but on x64 + Linux the compiler is perfectly happy keeping the implementation as int.

I converted all handlers that return zend_result typed as int, unless I missed any. I in turn didn't change int params to bools because that actually fails compilation because of different sizing.

[Girgias]

zend_result has the same size as int on x64. I'm not sure how it is for other architectures but on x64 + Linux the compiler is perfectly happy keeping the implementation as int.

I converted all handlers that return zend_result typed as int, unless I missed any. I in turn didn't change int params to bools because that actually fails compilation because of different sizing.

MSVC is more strict about function pointers and will complain about them, case in point the build currently fails on Windows because of:

ext\simplexml\simplexml.c(2382): error C2220: the following warning is treated as an error
ext\simplexml\simplexml.c(2382): warning C4133: 'initializing': incompatible types - from 'zend_result (__cdecl *)(zend_object_iterator *)' to 'int (__cdecl *)(zend_object_iterator *)'
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\cl.exe"' : return code '0x2'
Stop.

I do think this change is a good change as we have this weird situation where some stuff expects boolean returns and others zend_result and using the appropriate type makes it explicit.

[iluuu1994]

case in point the build currently fails on Windows because of

Yeah, it's just a warning though, it would compile without -Werror. But I see your point.

[iluuu1994]

Nevertheless, @Girgias objections to merging this? (after I fix the Windows build of course)

[Girgias]

Nope I'm good, but I think it might make sense to change all object handlers in that case instead of just those, but that can be done as a follow up PR.

[iluuu1994]

Which are left? I checked all of them, maybe I missed some.

[Girgias]

Oh yes indeed, I think it's the various object handlers which lead me to believe there where still some other ones which could be changed to zend_result but those are bool (which also might make sense as a follow up)

[iluuu1994]

@Girgias Yes. As mentioned, converting int to bool does require changing your implementation to bool or you'll get a compilation error. I'm fine with that, as long as others are too 🙂 I'll create a follow up PR for that though.