[libc++] Use proper functions instead of macros in bsd_locale_defaults.h #113759

ldionne · 2024-10-26T13:04:42Z

We were using macros instead of functions, leading to the inability to properly qualify calls to those symbols inside . This is also a step towards making the locale API modules-correct.

llvmbot · 2024-10-26T13:05:13Z

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

We were using macros instead of functions, leading to the inability to properly qualify calls to those symbols inside <locale>. This is also a step towards making the locale API modules-correct.

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

4 Files Affected:

(modified) libcxx/include/__locale_dir/locale_base_api.h (+8)
(modified) libcxx/include/__locale_dir/locale_base_api/bsd_locale_defaults.h (+97-14)
(modified) libcxx/include/locale (+1-11)
(modified) libcxx/include/module.modulemap (+2-1)

diff --git a/libcxx/include/__locale_dir/locale_base_api.h b/libcxx/include/__locale_dir/locale_base_api.h
index b6c80255b4d199..64615b96b784f8 100644
--- a/libcxx/include/__locale_dir/locale_base_api.h
+++ b/libcxx/include/__locale_dir/locale_base_api.h
@@ -9,6 +9,8 @@
 #ifndef _LIBCPP___LOCALE_DIR_LOCALE_BASE_API_H
 #define _LIBCPP___LOCALE_DIR_LOCALE_BASE_API_H
 
+#include <__config>
+
 #if defined(_LIBCPP_MSVCRT_LIKE)
 #  include <__locale_dir/locale_base_api/win32.h>
 #elif defined(_AIX) || defined(__MVS__)
@@ -27,6 +29,12 @@
 #  include <__locale_dir/locale_base_api/freebsd.h>
 #endif
 
+#ifdef _LIBCPP_LOCALE__L_EXTENSIONS
+#  include <__locale_dir/locale_base_api/bsd_locale_defaults.h>
+#else
+#  include <__locale_dir/locale_base_api/bsd_locale_fallbacks.h>
+#endif
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
diff --git a/libcxx/include/__locale_dir/locale_base_api/bsd_locale_defaults.h b/libcxx/include/__locale_dir/locale_base_api/bsd_locale_defaults.h
index e88eb4fa41d7af..56b43198b4a6ef 100644
--- a/libcxx/include/__locale_dir/locale_base_api/bsd_locale_defaults.h
+++ b/libcxx/include/__locale_dir/locale_base_api/bsd_locale_defaults.h
@@ -14,23 +14,106 @@
 #ifndef _LIBCPP___LOCALE_DIR_LOCALE_BASE_API_BSD_LOCALE_DEFAULTS_H
 #define _LIBCPP___LOCALE_DIR_LOCALE_BASE_API_BSD_LOCALE_DEFAULTS_H
 
+// <xlocale.h> must come first since it tells the other C headers to include additional declarations.
+// For some reason we must include <xlocale.h> before each C header.
+// clang-format off
+#include <xlocale.h>
+#include <stdio.h>
+
+#include <xlocale.h>
+#include <stdlib.h>
+
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+#  include <xlocale.h>
+#  include <wchar.h>
+#endif
+
+#include <xlocale.h>
+#include <ctype.h>
+// clang-format on
+
+#include <__config>
+#include <__cstddef/size_t.h>
+#include <__std_mbstate_t.h>
+#include <cstdarg>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
-#define __libcpp_mb_cur_max_l(loc) MB_CUR_MAX_L(loc)
-#define __libcpp_btowc_l(ch, loc) btowc_l(ch, loc)
-#define __libcpp_wctob_l(wch, loc) wctob_l(wch, loc)
-#define __libcpp_wcsnrtombs_l(dst, src, nwc, len, ps, loc) wcsnrtombs_l(dst, src, nwc, len, ps, loc)
-#define __libcpp_wcrtomb_l(src, wc, ps, loc) wcrtomb_l(src, wc, ps, loc)
-#define __libcpp_mbsnrtowcs_l(dst, src, nms, len, ps, loc) mbsnrtowcs_l(dst, src, nms, len, ps, loc)
-#define __libcpp_mbrtowc_l(pwc, s, n, ps, l) mbrtowc_l(pwc, s, n, ps, l)
-#define __libcpp_mbtowc_l(pwc, pmb, max, l) mbtowc_l(pwc, pmb, max, l)
-#define __libcpp_mbrlen_l(s, n, ps, l) mbrlen_l(s, n, ps, l)
-#define __libcpp_localeconv_l(l) localeconv_l(l)
-#define __libcpp_mbsrtowcs_l(dest, src, len, ps, l) mbsrtowcs_l(dest, src, len, ps, l)
-#define __libcpp_snprintf_l(...) snprintf_l(__VA_ARGS__)
-#define __libcpp_asprintf_l(...) asprintf_l(__VA_ARGS__)
-#define __libcpp_sscanf_l(...) sscanf_l(__VA_ARGS__)
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+inline _LIBCPP_HIDE_FROM_ABI decltype(MB_CUR_MAX) __libcpp_mb_cur_max_l(locale_t __loc) { return MB_CUR_MAX_L(__loc); }
+
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+inline _LIBCPP_HIDE_FROM_ABI wint_t __libcpp_btowc_l(int __c, locale_t __loc) { return ::btowc_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __libcpp_wctob_l(wint_t __c, locale_t __loc) { return ::wctob_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __libcpp_wcsnrtombs_l(
+    char* __dest, const wchar_t** __src, size_t __nwc, size_t __len, mbstate_t* __ps, locale_t __loc) {
+  return ::wcsnrtombs_l(__dest, __src, __nwc, __len, __ps, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __libcpp_wcrtomb_l(char* __s, wchar_t __wc, mbstate_t* __ps, locale_t __loc) {
+  return ::wcrtomb_l(__s, __wc, __ps, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __libcpp_mbsnrtowcs_l(
+    wchar_t* __dest, const char** __src, size_t __nms, size_t __len, mbstate_t* __ps, locale_t __loc) {
+  return ::mbsnrtowcs_l(__dest, __src, __nms, __len, __ps, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t
+__libcpp_mbrtowc_l(wchar_t* __pwc, const char* __s, size_t __n, mbstate_t* __ps, locale_t __loc) {
+  return ::mbrtowc_l(__pwc, __s, __n, __ps, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI int __libcpp_mbtowc_l(wchar_t* __pwc, const char* __pmb, size_t __max, locale_t __loc) {
+  return ::mbtowc_l(__pwc, __pmb, __max, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __libcpp_mbrlen_l(const char* __s, size_t __n, mbstate_t* __ps, locale_t __loc) {
+  return ::mbrlen_l(__s, __n, __ps, __loc);
+}
+#endif // _LIBCPP_HAS_NO_WIDE_CHARACTERS
+
+inline _LIBCPP_HIDE_FROM_ABI lconv* __libcpp_localeconv_l(locale_t __loc) { return ::localeconv_l(__loc); }
+
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+inline _LIBCPP_HIDE_FROM_ABI size_t
+__libcpp_mbsrtowcs_l(wchar_t* __dest, const char** __src, size_t __len, mbstate_t* __ps, locale_t __loc) {
+  return ::mbsrtowcs_l(__dest, __src, __len, __ps, __loc);
+}
+#endif
+
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 4, 5) int __libcpp_snprintf_l(
+    char* __s, size_t __n, locale_t __loc, const char* __format, ...) {
+  va_list __va;
+  va_start(__va, __format);
+  int __res = ::vsnprintf_l(__s, __n, __loc, __format, __va);
+  va_end(__va);
+  return __res;
+}
+
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 3, 4) int __libcpp_asprintf_l(
+    char** __s, locale_t __loc, const char* __format, ...) {
+  va_list __va;
+  va_start(__va, __format);
+  int __res = ::vasprintf_l(__s, __loc, __format, __va);
+  va_end(__va);
+  return __res;
+}
+
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_ATTRIBUTE_FORMAT(__scanf__, 3, 4) int __libcpp_sscanf_l(
+    const char* __s, locale_t __loc, const char* __format, ...) {
+  va_list __va;
+  va_start(__va, __format);
+  int __res = ::vsscanf_l(__s, __loc, __format, __va);
+  va_end(__va);
+  return __res;
+}
+
+_LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___LOCALE_DIR_LOCALE_BASE_API_BSD_LOCALE_DEFAULTS_H
diff --git a/libcxx/include/locale b/libcxx/include/locale
index 782475ea7e0eb8..4706515b0a6c86 100644
--- a/libcxx/include/locale
+++ b/libcxx/include/locale
@@ -215,7 +215,7 @@ template <class charT> class messages_byname;
 #  include <streambuf>
 #  include <version>
 
-// TODO: Fix __bsd_locale_defaults.h
+// TODO: Properly qualify calls now that __bsd_locale_defaults.h defines functions instead of macros
 // NOLINTBEGIN(libcpp-robust-against-adl)
 
 #  if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
@@ -230,16 +230,6 @@ template <class charT> class messages_byname;
 #    define _LIBCPP_HAS_CATOPEN 0
 #  endif
 
-#  ifdef _LIBCPP_LOCALE__L_EXTENSIONS
-#    include <__locale_dir/locale_base_api/bsd_locale_defaults.h>
-#  else
-#    include <__locale_dir/locale_base_api/bsd_locale_fallbacks.h>
-#  endif
-
-#  if defined(__APPLE__) || defined(__FreeBSD__)
-#    include <xlocale.h>
-#  endif
-
 #  if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #    pragma GCC system_header
 #  endif
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 05d08cfbd7cd29..be9df55e896a5d 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -1447,7 +1447,7 @@ module std [system] {
     module locale_base_api {
       textual header "__locale_dir/locale_base_api/android.h"
       textual header "__locale_dir/locale_base_api/apple.h"
-      textual header "__locale_dir/locale_base_api/bsd_locale_defaults.h"
+      header "__locale_dir/locale_base_api/bsd_locale_defaults.h"
       textual header "__locale_dir/locale_base_api/bsd_locale_fallbacks.h"
       textual header "__locale_dir/locale_base_api/freebsd.h"
       textual header "__locale_dir/locale_base_api/fuchsia.h"
@@ -1455,6 +1455,7 @@ module std [system] {
       textual header "__locale_dir/locale_base_api/musl.h"
       textual header "__locale_dir/locale_base_api/openbsd.h"
       textual header "__locale_dir/locale_base_api/win32.h"
+      export *
     }
     export *
   }

github-actions · 2024-10-30T09:47:22Z

✅ With the latest revision this PR passed the Python code formatter.

libcxx/include/__locale_dir/locale_base_api/bsd_locale_defaults.h

libcxx/include/__locale_dir/locale_base_api.h

We were using macros instead of functions, leading to the inability to properly qualify calls to those symbols inside <locale>. This is also a step towards making the locale API modules-correct.

…m to like that

ldionne · 2024-11-01T19:16:05Z

The CI failure is something that failed on main, which I fixed separately. Merging!

…s.h (llvm#113759) We were using macros instead of functions, leading to the inability to properly qualify calls to those symbols inside <locale>. This is also a step towards making the locale API modules-correct.

nico · 2024-11-25T21:03:27Z

This breaks building on mac in some scenarios: #117630

ldionne requested a review from a team as a code owner October 26, 2024 13:04

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

ldionne force-pushed the review/locale-5-get-rid-of-macros-in-bsd-defaults branch 2 times, most recently from fe9e37d to 08b9bc6 Compare October 29, 2024 18:10

philnik777 reviewed Oct 30, 2024

View reviewed changes

libcxx/include/__locale_dir/locale_base_api/bsd_locale_defaults.h Outdated Show resolved Hide resolved

libcxx/include/__locale_dir/locale_base_api.h Show resolved Hide resolved

ldionne force-pushed the review/locale-5-get-rid-of-macros-in-bsd-defaults branch from dcd12e2 to de4397e Compare November 1, 2024 13:40

ldionne added 7 commits November 1, 2024 09:43

[libc++] Use proper functions instead of macros in bsd_locale_defaults.h

1463f2d

We were using macros instead of functions, leading to the inability to properly qualify calls to those symbols inside <locale>. This is also a step towards making the locale API modules-correct.

XFAIL modules build on older macOS

c51b48b

Use variadics instead of non-existent function on Windows

0ffaeed

XFAIL -> UNSUPPORTED

5373c0a

Pass by ref in localeconv to avoid copying, since Windows doesn't see…

cf90006

…m to like that

Add TODO comment about locale_t by reference in localeconv

57492af

Use attribute format

94f7133

ldionne force-pushed the review/locale-5-get-rid-of-macros-in-bsd-defaults branch from de4397e to 94f7133 Compare November 1, 2024 13:44

philnik777 approved these changes Nov 1, 2024

View reviewed changes

ldionne merged commit 1a18767 into llvm:main Nov 1, 2024
62 of 63 checks passed

ldionne deleted the review/locale-5-get-rid-of-macros-in-bsd-defaults branch November 1, 2024 19:16

ShabbyX mentioned this pull request Nov 27, 2024

Mac build is failing due to ::wcsnrtombs_l and ::mbsnrtowcs_l #117895

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Use proper functions instead of macros in bsd_locale_defaults.h #113759

[libc++] Use proper functions instead of macros in bsd_locale_defaults.h #113759

Uh oh!

ldionne commented Oct 26, 2024

Uh oh!

llvmbot commented Oct 26, 2024

Uh oh!

github-actions bot commented Oct 30, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

ldionne commented Nov 1, 2024

Uh oh!

Uh oh!

nico commented Nov 25, 2024

Uh oh!

Uh oh!

[libc++] Use proper functions instead of macros in bsd_locale_defaults.h #113759

[libc++] Use proper functions instead of macros in bsd_locale_defaults.h #113759

Uh oh!

Conversation

ldionne commented Oct 26, 2024

Uh oh!

llvmbot commented Oct 26, 2024

Uh oh!

github-actions bot commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldionne commented Nov 1, 2024

Uh oh!

Uh oh!

nico commented Nov 25, 2024

Uh oh!

Uh oh!

github-actions bot commented Oct 30, 2024 •

edited

Loading