-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Compile error on MacOS with C++ extension when using ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX #12123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@kocsismate Can you take a look? |
Considering the intl extensions compiles just fine on MacOS, the only difference is that it defines the arg infos in an autogenerated header file. So maybe the new macro |
Not sure which Windows issue you're referring to, as I've been compiling the linked C++ extension on Windows and *nix for years across several different PHP versions with no issues. If the intl arginfos are included inside an |
I am talking about the introduction of: /* FIXME: We could add (zend_type) here at some point but this breaks in MSVC because
* (zend_type)(zend_type){} is no longer considered constant. */
# define _ZEND_TYPE_PREFIX Which as far as I can see was introduced because of a MSVC issue, and MSVC is the Windows compiler. But yes, they include the arginfos via an |
The empty macro appears to be working correct. The other branch under |
Thanks for the confirmation ! I guess we have to make it only apply to MSVC. |
I'm just a little puzzled as to what this new macro actually fixes. As I said, the C++ extension I linked has been compiled with MSVC, clang and gcc on Linux, MacOS and Windows for several years, and had no issues without this macro. I found this commit in the PR that added it, but I don't see any specifics on what exactly it fixed: 62341c0 |
Finally, I found it: https://github.com/php/php-src/actions/runs/5442753605/job/14732712171#step:7:1318
For reference: the following line made untyped constants and properties use the construct unsupported by MSVC: https://github.com/php/php-src/pull/10170/files#diff-5df7a541a3649c4a1ad7870312ebdcd836b6c36dc1dc65c8fda8271c210e014dR2267 (line 2267 in gen_stub.php) The test failure appeared when I merged my original PR to master (#10170). On the other hand, the build was green on PHP-8.2. Then the PR was reverted, and the new one was applied along with Ilija's fix a few weeks later. |
Okay so the need for this type macro prefix is only when we include the arg infos in an I wonder if it makes more sense to move the arginfo includes out of the extern C for intl and see if we can remove the macro altogether? |
I'm not sure if that helps. AFAIK (not knowing C++ at all), designated initializers are a g++ extension, until C++20 where they have finally been standardized with C compatibility. The usual way to initialize structs in C++ is |
* PHP-8.3: Fix #12123 Make _ZEND_TYPE_PREFIX apply only for MSVC
Description
https://github.com/pmmp/ext-morton/blob/2532e9f09d3bc6f6975d78b6632086b894dcef75/morton.cpp#L45
This code compiles fine with GCC on Ubuntu 20.04 on 8.3.0RC1, but fails on MacOS with clang with the following errors:
I believe this problem was caused by a change in #11978.
PHP Version
8.3.0RC1
Operating System
MacOS 11 (GitHub Actions)
The text was updated successfully, but these errors were encountered: