[libcxx-commits] [PATCH] D122861: [libcxx] Add some missing xlocale wrapper functions for OpenBSD

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__support/openbsd/xlocale.h:24
+
+static inline long strtol_l(const char *nptr, char **endptr, int base,
+                            locale_t) {
----------------
Mordante wrote:
> brad wrote:
> > Mordante wrote:
> > > Why the `static inline` instead of `inline`?
> > > Please add `_LIBCPP_HIDE_FROM_ABI`.
> > > Why the `static inline` instead of `inline`?
> > > Please add `_LIBCPP_HIDE_FROM_ABI`.
> > 
> > I copied bits from libcxx/include/__support/solaris/xlocale.h, but the same goes for libcxx/include/__support/ibm/xlocale.h and libcxx/include/__support/musl/xlocale.h.
> > 
> > I don't see _LIBCPP_HIDE_FROM_ABI used anywhere in that sub-dir.
> This is used in `_support/xlocale/__strtonum_fallback.h`
> ```
> inline _LIBCPP_INLINE_VISIBILITY long long                                      
> strtoll_l(const char *nptr, char **endptr, int base, locale_t) {
>   return ::strtoll(nptr, endptr, base);
> }                                                                               
> ```
> Note that `_LIBCPP_INLINE_VISIBILITY` is the deprecated name for `_LIBCPP_HIDE_FROM_ABI`
> 
> @ldionne Do you know why these locale headers use `static inline` instead of `inline LIBCPP_HIDE_FROM_ABI`?
I don't know why, but it's certainly unusual for us to define `static inline` functions in headers. I think this should be `inline` only (and `_LIBCPP_HIDE_ROM_ABI` like everything else).

Edit: It might be because those are meant to bridge a gap between the C headers on the system, and that's a C like way of defining those functions? Regardless, I think we should go for just `inline`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122861/new/

https://reviews.llvm.org/D122861