Skip to content
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

Windows should distinguish between CFLAGS and CXXFLAGS #17612

Open
cmb69 opened this issue Jan 28, 2025 · 1 comment
Open

Windows should distinguish between CFLAGS and CXXFLAGS #17612

cmb69 opened this issue Jan 28, 2025 · 1 comment

Comments

@cmb69
Copy link
Member

cmb69 commented Jan 28, 2025

Description

So far this is not a big problem: .c and .cpp files are passed to cl.exe (or clang-cl.exe), and the compiler figures our whether to compile as C or C++ according to the file extension. Always passing the same flags is also not an issue.

Enter newer C/C++ standards: recent versions of ICU require C++17, so ext/intl passes /std:c++17. cl.exe is still fine with this, but clang-cl.exe (18.1.8) complains about an unused argument when compiling .c files of ext/intl. Not a big deal. Now php-src requires C11, so I tried passing /std:c11. Besides hitting #17107, that generally works fine (get a warning in xxhash.h). However, when trying to build ext/intl, MSVC errors, reporting incompatible /std flags.

Thus I think we need at least a minimum support for CXXFLAGS. Full fledged support might be overkill, and likely raises issues with some external extensions. I'm not really sure how to support this, though. Ideas welcome!

Note that such support would be an issue for /MP enabled builds. Currently, these pass all source files added via a single ADD_SOURCES() call to the compiler in one go. Passing individual files basically circumvents /MP. If we split the sets (C files vs. C++ files), that would reduce the usefulness of /MP. Not a big deal for php-src, but might be some problem for external extensions.

@cmb69
Copy link
Member Author

cmb69 commented Jan 28, 2025

get a warning in xxhash.h

Would be fixed with Cyan4973/xxHash@6078dd6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant