Skip to content

release/19.x: [libcxx] don't #include <cwchar> if wide chars aren't enabled (#99911) #106788

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
Sep 1, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 30, 2024

Backport ec56790

Requested by: @ldionne

@llvmbot llvmbot requested a review from a team as a code owner August 30, 2024 20:00
@llvmbot llvmbot added this to the LLVM 19.X Release milestone Aug 30, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 30, 2024

@stuij What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from stuij August 30, 2024 20:00
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 30, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 30, 2024

@llvm/pr-subscribers-libcxx

Author: None (llvmbot)

Changes

Backport ec56790

Requested by: @ldionne


Full diff: https://github.com/llvm/llvm-project/pull/106788.diff

1 Files Affected:

  • (modified) libcxx/include/format (+6-6)
diff --git a/libcxx/include/format b/libcxx/include/format
index c3f2b45f0f7305..a88b3ef8528e2d 100644
--- a/libcxx/include/format
+++ b/libcxx/include/format
@@ -237,21 +237,21 @@ namespace std {
 #  include <cstdint>
 #  include <cstdlib>
 #  include <cstring>
-#  include <cwchar>
 #  include <initializer_list>
 #  include <limits>
+#  include <locale>
 #  include <new>
 #  include <optional>
+#  include <queue>
+#  include <stack>
 #  include <stdexcept>
 #  include <string>
 #  include <string_view>
 #  include <tuple>
-#endif
 
-#if !defined(_LIBCPP_REMOVE_TRANSITIVE_INCLUDES) && _LIBCPP_STD_VER <= 20
-#  include <locale>
-#  include <queue>
-#  include <stack>
+#  if !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS)
+#    include <cwchar>
+#  endif
 #endif
 
 #endif // _LIBCPP_FORMAT

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We saw some breakage internally that is fixed by this patch and that's why I believe this regression fix is worth cherry-picking.

Copy link
Member

@stuij stuij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, makes sense to cherry-pick.

…#99911)

Pull request llvm#96032 unconditionall adds the `cwchar` include in the
`format` umbrella header. However support for wchar_t can be disabled in
the build system (LIBCXX_ENABLE_WIDE_CHARACTERS).

This patch guards against inclusion of `cwchar` in `format` by checking
the `_LIBCPP_HAS_NO_WIDE_CHARACTERS` define.

For clarity I've also merged the include header section that `cwchar`
was in with the one above as they were both guarded by the same `#if`
logic.

(cherry picked from commit ec56790)
@tru tru merged commit 6f62347 into llvm:release/19.x Sep 1, 2024
9 of 10 checks passed
Copy link

github-actions bot commented Sep 1, 2024

@ldionne (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
Development

Successfully merging this pull request may close these issues.

4 participants