[libc++] Fix the locale base API on Linux with musl #128936

androm3da · 2025-02-26T19:20:54Z

Since 363bfd6090b0 ([libc++] Use the new locale base API on Linux (#128007), 2025-02-24), musl targets will fail to build with errors like the below:

In file included from /[b/f/w/src/git/out/stage2/include/c++/v1/__locale:14](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale?l=14):
In file included from /[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/locale_base_api.h:123](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/locale_base_api.h?l=123):
/[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:98](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=98):12: error: no member named 'strtoll_l' in the global namespace
   98 |   return ::strtoll_l(__nptr, __endptr, __base, __loc);
      |          ~~^
/[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:103](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=103):10: error: no member named 'strtoull_l' in the global namespace; did you mean '__strtoull'?
  103 |   return ::strtoull_l(__nptr, __endptr, __base, __loc);
      |          ^~~~~~~~~~~~
      |          __strtoull
/[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:102](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=102):1: note: '__strtoull' declared here
  102 | __strtoull(const char* __nptr, char** __endptr, int __base, __locale_t __loc) {
      | ^
2 errors generated.

github-actions · 2025-02-26T19:24:28Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Since 363bfd6 ([libc++] Use the new locale base API on Linux (llvm#128007), 2025-02-24), musl targets will fail to build with errors like the below: In file included from /[b/f/w/src/git/out/stage2/include/c++/v1/__locale:14](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale?l=14): In file included from /[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/locale_base_api.h:123](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/locale_base_api.h?l=123): /[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:98](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=98):12: error: no member named 'strtoll_l' in the global namespace 98 | return ::strtoll_l(__nptr, __endptr, __base, __loc); | ~~^ /[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:103](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=103):10: error: no member named 'strtoull_l' in the global namespace; did you mean '__strtoull'? 103 | return ::strtoull_l(__nptr, __endptr, __base, __loc); | ^~~~~~~~~~~~ | __strtoull /[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:102](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=102):1: note: '__strtoull' declared here 102 | __strtoull(const char* __nptr, char** __endptr, int __base, __locale_t __loc) { | ^ 2 errors generated.

llvmbot · 2025-02-26T22:15:11Z

@llvm/pr-subscribers-libcxx

Author: Brian Cain (androm3da)

Changes

Since 363bfd6090b0 ([libc++] Use the new locale base API on Linux (#128007), 2025-02-24), musl targets will fail to build with errors like the below:

In file included from /[b/f/w/src/git/out/stage2/include/c++/v1/__locale:14](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale?l=14):
In file included from /[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/locale_base_api.h:123](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/locale_base_api.h?l=123):
/[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:98](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=98):12: error: no member named 'strtoll_l' in the global namespace
   98 |   return ::strtoll_l(__nptr, __endptr, __base, __loc);
      |          ~~^
/[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:103](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=103):10: error: no member named 'strtoull_l' in the global namespace; did you mean '__strtoull'?
  103 |   return ::strtoull_l(__nptr, __endptr, __base, __loc);
      |          ^~~~~~~~~~~~
      |          __strtoull
/[b/f/w/src/git/out/stage2/include/c++/v1/__locale_dir/support/linux.h:102](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/stage2/include/c%2B%2B/v1/__locale_dir/support/linux.h?l=102):1: note: '__strtoull' declared here
  102 | __strtoull(const char* __nptr, char** __endptr, int __base, __locale_t __loc) {
      | ^
2 errors generated.

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

1 Files Affected:

(modified) libcxx/include/__locale_dir/support/linux.h (+8)

diff --git a/libcxx/include/__locale_dir/support/linux.h b/libcxx/include/__locale_dir/support/linux.h
index f1662c0112603..00c99eb5ea351 100644
--- a/libcxx/include/__locale_dir/support/linux.h
+++ b/libcxx/include/__locale_dir/support/linux.h
@@ -95,12 +95,20 @@ inline _LIBCPP_HIDE_FROM_ABI long double __strtold(const char* __nptr, char** __
 }
 
 inline _LIBCPP_HIDE_FROM_ABI long long __strtoll(const char* __nptr, char** __endptr, int __base, __locale_t __loc) {
+#if !defined(_LIBCPP_HAS_MUSL_LIBC)
   return ::strtoll_l(__nptr, __endptr, __base, __loc);
+#else
+  return ::strtoll(__nptr, __endptr, __base);
+#endif
 }
 
 inline _LIBCPP_HIDE_FROM_ABI unsigned long long
 __strtoull(const char* __nptr, char** __endptr, int __base, __locale_t __loc) {
+#if !defined(_LIBCPP_HAS_MUSL_LIBC)
   return ::strtoull_l(__nptr, __endptr, __base, __loc);
+#else
+  return ::strtoull(__nptr, __endptr, __base);
+#endif
 }
 
 //

ldionne

LGTM, thanks for the fix and sorry for the slow reaction time.

Do you know if this means we can actually get rid of this code path in locale_base_api.h?

#  elif defined(__wasi__) || _LIBCPP_HAS_MUSL_LIBC
#    include <__locale_dir/locale_base_api/musl.h>

libcxx/include/__locale_dir/support/linux.h

ldionne · 2025-02-26T22:21:34Z

This can be landed as soon as the CI is green. If someone lands this before I do, please edit the commit message to avoid butchering git log.

Thanks for the fix @androm3da

Instead of `#if defined(_LIBCPP_HAS_MUSL_LIBC)`, we can rely on `#if _LIBCPP_HAS_MUSL_LIBC` directly. This should address these failures: # .---command stdout------------ # | /__w/llvm-project/llvm-project/build/frozen-cxx03-headers/libcxx/test-suite-install/include/c++/v1/__locale_dir/support/linux.h:98:6: error: '_LIBCPP_HAS_MUSL_LIBC' is always defined to 1 or 0. [libcpp-internal-ftms,-warnings-as-errors] # | 98 | #if !defined(_LIBCPP_HAS_MUSL_LIBC) # | | ^ # | /__w/llvm-project/llvm-project/build/frozen-cxx03-headers/libcxx/test-suite-install/include/c++/v1/__locale_dir/support/linux.h:108:6: error: '_LIBCPP_HAS_MUSL_LIBC' is always defined to 1 or 0. [libcpp-internal-ftms,-warnings-as-errors] # | 108 | #if !defined(_LIBCPP_HAS_MUSL_LIBC) # | | ^ # `-----------------------------

androm3da · 2025-02-27T02:33:48Z

This can be landed as soon as the CI is green. If someone lands this before I do, please edit the commit message to avoid butchering git log.

Thanks for the fix @androm3da

Note that this fix was actually proposed by @pirama-arumuga-nainar.

Even though @pirama-arumuga-nainar is the author of the first commit in this PR I think GH will take my name as the author when it's merged. So I'll add a Co-authored-by to the message if that's satisfactory @pirama-arumuga-nainar

androm3da · 2025-02-27T02:36:32Z

please edit the commit message to avoid butchering git log.

Will GH catenate the commit messages in the squash-and-merge? In which case I'm happy to edit it so that it reflects the original commit message in the first commit. Is that what you meant, or what butchering specifically should I try to avoid?

androm3da · 2025-02-27T02:44:52Z

LGTM, thanks for the fix and sorry for the slow reaction time.

Do you know if this means we can actually get rid of this code path in locale_base_api.h?
#  elif defined(__wasi__) || _LIBCPP_HAS_MUSL_LIBC
#    include <__locale_dir/locale_base_api/musl.h>

This change shouldn't impact __wasi__ targets. Should we include __wasi__ in these changes to __strtoll(), __strtoull()?

pirama-arumuga-nainar · 2025-02-27T06:17:07Z

LGTM, thanks for the fix and sorry for the slow reaction time.
Do you know if this means we can actually get rid of this code path in locale_base_api.h?
#  elif defined(__wasi__) || _LIBCPP_HAS_MUSL_LIBC
#    include <__locale_dir/locale_base_api/musl.h>
This change shouldn't impact __wasi__ targets. Should we include __wasi__ in these changes to __strtoll(), __strtoull()?

The separate definitions are still needed for wasi - it's not a Linux environment. . We could rename the file to wasi.h to make this explicit.

So I'll add a Co-authored-by to the message if that's satisfactory @pirama-arumuga-nainar

Thanks for the credit, but no need to go out of the way for this. I'm happy to just have the breakage fixed.

androm3da · 2025-02-27T17:02:02Z

I'll rebase this after #128950 lands to see if that addresses the CI issues.

philnik777 · 2025-02-27T17:14:59Z

Ironically, #121795 will get rid of this code anyways...

ldionne · 2025-02-28T03:48:40Z

Let's not wait to merge this. The FreeBSD failure is a fluke that will be fixed by #128950.

As for #121795, it can remove that code when it lands, but let's get main out of a broken state.

philnik777 · 2025-02-28T09:19:12Z

Let's not wait to merge this. The FreeBSD failure is a fluke that will be fixed by #128950.

As for #121795, it can remove that code when it lands, but let's get main out of a broken state.

Just to be clear: I didn't mean that #121795 should block this. I found it just ironic that if I landed the patch a bit earlier this never would have happened.

Since `363bfd6090b0 ([libc++] Use the new locale base API on Linux (llvm#128007), 2025-02-24)`, musl targets will fail to build with errors due to missing strtoll_l functions. Co-authored-by: Pirama Arumuga Nainar <[email protected]>

androm3da requested review from pirama-arumuga-nainar and ldionne February 26, 2025 19:21

androm3da force-pushed the bcain/strto_musl branch from b8ab545 to 08a49e8 Compare February 26, 2025 19:26

androm3da mentioned this pull request Feb 26, 2025

[libc++] Use the new locale base API on Linux #128007

Merged

androm3da force-pushed the bcain/strto_musl branch from 08a49e8 to 2fbb3b0 Compare February 26, 2025 19:44

ldionne marked this pull request as ready for review February 26, 2025 22:14

ldionne requested a review from a team as a code owner February 26, 2025 22:14

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 26, 2025

ldionne changed the title ~~draft: [libcxx] avoid the "_l" calls on musl platforms~~ [libc++] Fix the locale base API on Linux with musl Feb 26, 2025

ldionne approved these changes Feb 26, 2025

View reviewed changes

libcxx/include/__locale_dir/support/linux.h Outdated Show resolved Hide resolved

libcxx/include/__locale_dir/support/linux.h Outdated Show resolved Hide resolved

Apply suggestions from code review

98f7dc5

ldionne added the pending-ci Merging the PR is only pending completion of CI label Feb 26, 2025

ldionne merged commit 39c6c8b into llvm:main Feb 28, 2025
81 of 85 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Fix the locale base API on Linux with musl #128936

[libc++] Fix the locale base API on Linux with musl #128936

Uh oh!

androm3da commented Feb 26, 2025

Uh oh!

github-actions bot commented Feb 26, 2025 •

edited

Loading

Uh oh!

llvmbot commented Feb 26, 2025

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

Uh oh!

ldionne commented Feb 26, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

pirama-arumuga-nainar commented Feb 27, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

philnik777 commented Feb 27, 2025

Uh oh!

ldionne commented Feb 28, 2025

Uh oh!

Uh oh!

philnik777 commented Feb 28, 2025

Uh oh!

Uh oh!

[libc++] Fix the locale base API on Linux with musl #128936

[libc++] Fix the locale base API on Linux with musl #128936

Uh oh!

Conversation

androm3da commented Feb 26, 2025

Uh oh!

github-actions bot commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 26, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ldionne commented Feb 26, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

pirama-arumuga-nainar commented Feb 27, 2025

Uh oh!

androm3da commented Feb 27, 2025

Uh oh!

philnik777 commented Feb 27, 2025

Uh oh!

ldionne commented Feb 28, 2025

Uh oh!

Uh oh!

philnik777 commented Feb 28, 2025

Uh oh!

Uh oh!

github-actions bot commented Feb 26, 2025 •

edited

Loading