Skip to content

ICU 75.1 requires C++17 #15678

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

Merged
merged 1 commit into from
Aug 31, 2024
Merged

ICU 75.1 requires C++17 #15678

merged 1 commit into from
Aug 31, 2024

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 31, 2024

This needs to be explicitly enabled for MSVC (and probably clang on Windows); otherwise the default is C++14, which is no longer sufficient for ICU[1].

While the official PHP 8.4 builds for Windows do not yet use ICU 75.1, that may change[2]. And even if not, it would be nice for custom builds to be able to build against ICU 75.1 (or later).

Anyhow, using std:c++17 is fine for ICU 72.1 which we are currently using (and likely for some older ICU versions).

[1] https://github.com/unicode-org/icu/releases/tag/release-75-1
[2] https://github.com/winlibs/icu4c/pulls


It seems that this is already catered to in ext/intl/config.m4 for other platforms. And maybe it would be better to enable the flag on Windows conditionally, depending on the ICU version (would likely need a GREP_HEADER() check).

This needs to be explicitly enabled for MSVC (and probably clang on
Windows); otherwise the default is C++14, which is no longer sufficient
for ICU[1].

While the official PHP 8.4 builds for Windows do not yet use ICU 75.1,
that may change[2].  And even if not, it would be nice for custom
builds to be able to build against ICU 75.1 (or later).

Anyhow, using `std:c++17` is fine for ICU 72.1 which we are currently
using (and likely for some older ICU versions).

[1] <https://github.com/unicode-org/icu/releases/tag/release-75-1>
[2] <https://github.com/winlibs/icu4c/pulls>
@devnexen
Copy link
Member

It seems that this is already catered to in ext/intl/config.m4 for other platforms. And maybe it would be better to enable the flag on Windows conditionally, depending on the ICU version (would likely need a GREP_HEADER() check).

What s the minimal MSVC version supported for this branch ?

@cmb69
Copy link
Member Author

cmb69 commented Aug 31, 2024

What s the minimal MSVC version supported for this branch ?

Currently there are basically no restrictions (except that vc15 will not compile ext/bcmath). If we merge PR #15403, it would be vs16 (aka. Visual Studio 2019).

@devnexen
Copy link
Member

ok VS 2019 seems to have full support of C++17.

@cmb69
Copy link
Member Author

cmb69 commented Aug 31, 2024

ok VS 2019 seems to have full support of C++17.

Right, but even if it had not, to build ICU 75.1 you need C++17 anyway.

@devnexen
Copy link
Member

sure I was referring to your conditional/GREP_HEADER sentence.

@cmb69 cmb69 merged commit 0f8259e into php:master Aug 31, 2024
9 of 10 checks passed
@cmb69 cmb69 deleted the cmb/intl-c++17 branch August 31, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants