[libcxx-commits] [PATCH] D136538: [libc++] Add test for checking progress on P0533R9

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D136538#3880021 <https://reviews.llvm.org/D136538#3880021>, @Izaron wrote:

>> Could you also add a test for GCC and the global namespace versions of the functions?
>
> Does "test for GCC" mean testing how GCC would compile libc++?
>
> I think we could make it by copy-pasting the file with changing `// REQUIRES: clang` to `// REQUIRES: gcc`.

Yes, that's what I had in mind.

> But we need to think how we could declare functions constexpr giving that GCC doesn't support `__has_constexpr_builtin`? Since GCC supposedly supports a ton of constexpr builtins now, we could check `__has_constexpr_builtin` for Clang and the compiler's version for GCC. Would that work?
>
>   #ifdef __clang__
>   #  if __has_constexpr_builtin(__builtin_fmax)
>        constexpr
>   #  endif
>   #elifdef __GNU__
>   #  if __GNU_VERSION__ >= 1234
>        constexpr
>   #  endif
>   #endif
>   double fmax(double a, double b) { return __builtin_fmax(a, b); }

We only support GCC 12, so we could simplify this to

name=__config
  #ifndef __has_constexpr_builtin
  #  define __has_constexpr_builtin(...) 0
  #endif

name=math.h
  #if __has_constexpr_builtin(__builtin_fmax) || defined(_LIBCPP_COMPILER_GCC)
  constexpr
  #endif
  double fmax(double a, double b) { return __builtin_fmax(a, b); }

but yes, that's the general idea.



================
Comment at: libcxx/test/std/numerics/c.math/constexpr.cmath.pass.cpp:2
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
----------------
Izaron wrote:
> philnik wrote:
> > Could you also add a test for GCC and the global namespace versions of the functions?
> What are "global namespace versions of the functions"? Functions outside the `std::` namespace?
yes


================
Comment at: libcxx/test/std/numerics/c.math/constexpr.cmath.pass.cpp:13
+
+#include "test_macros.h"
+
----------------
Izaron wrote:
> philnik wrote:
> > What do you need this include for?
> I couldn't use `TEST_STD_VER` without this include. But now this is irrelevant since I don't need this macro with `// REQUIRES` and `// UNSUPPORTED`. I don't include this anymore.
I missed that, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136538