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

[Evgeny Shulgin via Phabricator via libcxx-commits]

Izaron marked 5 inline comments as done.
Izaron added inline comments.


================
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.
----------------
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?


================
Comment at: libcxx/test/std/numerics/c.math/constexpr.cmath.pass.cpp:13
+
+#include "test_macros.h"
+
----------------
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.


================
Comment at: libcxx/test/std/numerics/c.math/constexpr.cmath.pass.cpp:19
+
+#  define ASSERT_CONSTEXPR_CXX23(Expr) static_assert(__builtin_constant_p(Expr) && (Expr))
+#  define ASSERT_NOT_CONSTEXPR_CXX23(Expr)                                                                             \
----------------
philnik wrote:
> What does `_builtin_constant_p` do?
`__builtin_constant_p(Expr)` is a GNU builtin (has been ported to Clang) that determines whether the `Expr` expression is known to be constant in compile time - https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

This is somewhat similar with `__has_constexpr_builtin`:
```
static_assert(__builtin_constant_p(__builtin_fmax(12.34, 45.67) == 45.67));
static_assert(__has_constexpr_builtin(__builtin_fmax));
```

I think `__has_constexpr_builtin` is better for "conditionally constexpr" math functions in the headers, and `__builtin_constant_p` is better for tests checking that expression is constant evaluable.

`ASSERT_CONSTEXPR_CXX23` ensures that the `Expr` is constant evaluable, `ASSERT_NOT_CONSTEXPR_CXX23` ensures that isn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136538