[libcxx-commits] [PATCH] D143033: [libc++] Disable some tests in `math_nodiscard_extensions`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

I like this change -- depending on the fact that this is a `.verify.cpp` test to disable this on non-Clang compilers is too subtle IMO.



================
Comment at: libcxx/test/libcxx/diagnostics/math_nodiscard_extensions.verify.cpp:19
 void test() {
   std::signbit(0.f);             // expected-warning-re {{ignoring return value of function declared with {{.*}} attribute}}
   std::signbit(0.);              // expected-warning-re {{ignoring return value of function declared with {{.*}} attribute}}
----------------
We should even be able to drop this regex now since this is consistently nodiscard.


================
Comment at: libcxx/test/libcxx/diagnostics/math_nodiscard_extensions.verify.cpp:155
   std::trunc(0);                 // expected-warning-re {{ignoring return value of function declared with {{.*}} attribute}}
   std::trunc(0U);                // expected-warning-re {{ignoring return value of function declared with {{.*}} attribute}}
 }
----------------
I think this is what I would do:

```
// These tests rely on Clang's behaviour of adding [[gnu::const]] to the double overload of most of the functions below.
// Without that attribute being added implicitly, this test can't be checked consistently because its result depends on whether we're
// getting libc++'s own std::foo(double) or the underlying C library's foo(double).
#ifdef TEST_COMPILER_CLANG
std::round(0.); // expected-warning-re {{....}}
std::trunc(0.); // expected-warning-re {{....}}
etc...
#endif
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143033