Skip to content

[libc++] Fix check for _LIBCPP_HAS_NO_WIDE_CHARACTERS in features.py #131675

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
Mar 18, 2025

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Mar 17, 2025

The patch that added the new locale Lit features was created before we switched to a 0-1 macro for _LIBCPP_HAS_WIDE_CHARACTERS, leading to that patch referring to the obsolete _LIBCPP_HAS_NO_WIDE_CHARACTERS macro that is never defined nowadays.

The patch that added the new locale Lit features was created before we
switched to a 0-1 macro for _LIBCPP_HAS_WIDE_CHARACTERS, leading to
that patch referring to the obsolete _LIBCPP_HAS_NO_WIDE_CHARACTERS
macro that is never defined nowadays.
@ldionne ldionne requested a review from mstorsjo March 17, 2025 21:32
@ldionne ldionne requested a review from a team as a code owner March 17, 2025 21:32
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 17, 2025

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

The patch that added the new locale Lit features was created before we switched to a 0-1 macro for _LIBCPP_HAS_WIDE_CHARACTERS, leading to that patch referring to the obsolete _LIBCPP_HAS_NO_WIDE_CHARACTERS macro that is never defined nowadays.


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

1 Files Affected:

  • (modified) libcxx/utils/libcxx/test/features.py (+2-1)
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index a83dcd16b16f8..10fc4b0afde6b 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -440,7 +440,8 @@ def _mingwSupportsModules(cfg):
                 cfg, locale, alts, provide_locale_conversions[locale]
             )
             if locale in provide_locale_conversions
-            and "_LIBCPP_HAS_NO_WIDE_CHARACTERS" not in compilerMacros(cfg)
+            and ("_LIBCPP_HAS_WIDE_CHARACTERS" not in compilerMacros(cfg) or
+                 compilerMacros(cfg)["_LIBCPP_HAS_WIDE_CHARACTERS"] == "1")
             else [],
         ),
     )

Copy link

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r 24e88b0e6bc04f16d7353ad9ef07398836adf244...303b8c45b468e5f9d13eb2f04e4feeadccec3ae3 libcxx/utils/libcxx/test/features.py
View the diff from darker here.
--- features.py	2025-03-17 21:30:45.000000 +0000
+++ features.py	2025-03-17 21:35:53.863524 +0000
@@ -438,12 +438,14 @@
             when=lambda cfg, alts=alts: hasAnyLocale(cfg, alts),
             actions=lambda cfg, locale=locale, alts=alts: _getLocaleFlagsAction(
                 cfg, locale, alts, provide_locale_conversions[locale]
             )
             if locale in provide_locale_conversions
-            and ("_LIBCPP_HAS_WIDE_CHARACTERS" not in compilerMacros(cfg) or
-                 compilerMacros(cfg)["_LIBCPP_HAS_WIDE_CHARACTERS"] == "1")
+            and (
+                "_LIBCPP_HAS_WIDE_CHARACTERS" not in compilerMacros(cfg)
+                or compilerMacros(cfg)["_LIBCPP_HAS_WIDE_CHARACTERS"] == "1"
+            )
             else [],
         ),
     )
 
 

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

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

LGTM

@ldionne ldionne merged commit 297f6d9 into llvm:main Mar 18, 2025
87 of 88 checks passed
@ldionne ldionne deleted the review/fix-locale-feature branch March 18, 2025 02:13
mstorsjo pushed a commit to mstorsjo/llvm-project that referenced this pull request May 11, 2025
…lvm#131675)

The patch that added the new locale Lit features was created before we
switched to a 0-1 macro for _LIBCPP_HAS_WIDE_CHARACTERS, leading to that
patch referring to the obsolete _LIBCPP_HAS_NO_WIDE_CHARACTERS macro
that is never defined nowadays.

(cherry picked from commit 297f6d9)
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants