[libcxx-commits] [PATCH] D108630: [libc++] Remove _LIBCPP_HAS_NO_LONG_LONG in favour of using_if_exists

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/stdlib.h:153
 }
-#ifndef _LIBCPP_HAS_NO_LONG_LONG
+#if !(defined(__FreeBSD__) && !defined(__LONG_LONG_SUPPORTED))
 inline _LIBCPP_INLINE_VISIBILITY lldiv_t div(long long __x,
----------------
dim wrote:
> ldionne wrote:
> > @emaste @dim
> > 
> > What does `!defined(__LONG_LONG_SUPPORTED)` mean exactly? It doesn't mean that the `long long` type isn't provided by the compiler, because there are several other uses of `long long` in the library that are not guarded by that check. Is it only about the C Standard Library on FreeBSD not providing `::lldiv`? What about `__builtin_llabs()` -- I would that if `long long` is a valid type, the compiler does implement `__builtin_llabs` as used above and we don't need to guard the above use of it?
> > @emaste @dim
> > 
> > What does `!defined(__LONG_LONG_SUPPORTED)` mean exactly? It doesn't mean that the `long long` type isn't provided by the compiler, because there are several other uses of `long long` in the library that are not guarded by that check. Is it only about the C Standard Library on FreeBSD not providing `::lldiv`? What about `__builtin_llabs()` -- I would that if `long long` is a valid type, the compiler does implement `__builtin_llabs` as used above and we don't need to guard the above use of it?
> 
> It's a historic thing in our https://github.com/freebsd/freebsd-src/blob/main/include/stdlib.h#L126 and various other standard headers:
> 
> ```
> /*
>  * Functions added in C99 which we make conditionally available in the
>  * BSD^C89 namespace if the compiler supports `long long'.
>  * The #if test is more complicated than it ought to be because
>  * __BSD_VISIBLE implies __ISO_C_VISIBLE == 1999 *even if* `long long'
>  * is not supported in the compilation environment (which therefore means
>  * that it can't really be ISO C99).
>  *
>  * (The only other extension made by C99 in thie header is _Exit().)
>  */
> ```
> 
> The definition of `__LONG_LONG_SUPPORTED` comes from https://github.com/freebsd/freebsd-src/blob/main/sys/sys/cdefs.h#L391 :
> 
> ```
> #if (defined(__GNUC__) && __GNUC__ >= 2) && !defined(__STRICT_ANSI__) || __STDC_VERSION__ >= 199901
> #define	__LONG_LONG_SUPPORTED
> #endif
> 
> /* C++11 exposes a load of C99 stuff */
> #if defined(__cplusplus) && __cplusplus >= 201103L
> #define	__LONG_LONG_SUPPORTED
> #ifndef	__STDC_LIMIT_MACROS
> #define	__STDC_LIMIT_MACROS
> #endif
> #ifndef	__STDC_CONSTANT_MACROS
> #define	__STDC_CONSTANT_MACROS
> #endif
> #endif
> ```
> 
> Ergo, C++ standards before C++11 have `__LONG_LONG_SUPPORTED` undefined. Unfortunately there are still quite a few programs requiring `-std=c++98` (or more usually `-std=gnu++98`)...
> 
Okay, thanks a lot for the background. In that case, I think we can even remove the guard from around the use of `__builtin_llabs` and this patch should be non-breaking.

Would it be possible to add build bots for FreeBSD so we can test changes like those automatically?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108630