[libc-commits] [PATCH] D106086: [libc] Add a new test matcher for tests raising floating point exceptions.

[Roland McGrath via Phabricator via libc-commits]

mcgrathr added a comment.

For Fuchsia you should just define the new macro to continue to use zxtest's ASSERT_DEATH macro.



================
Comment at: libc/utils/FPUtil/TestHelpers.cpp:77
 
+#if defined(__WIN32) || defined(_WIN64)
+static thread_local jmp_buf jumpBuffer;
----------------
It seems like it would be cleaner to just do
```
#define sigjmp_buf jmp_buf
#define sigsetjmp(buf, save) setjmp(buf)
#define siglongjmp(buf) longjmp(buf)
```
and not have any more conditionals below.


================
Comment at: libc/utils/FPUtil/TestHelpers.cpp:86
+static void sigfpeHandler(int sig) {
+  if (sig == SIGFPE) {
+    caughtExcept = true;
----------------
Your handler is only called for the signals you've set it on, so this will never be entered with a different value and you don't need to test it.


================
Comment at: libc/utils/FPUtil/TestHelpers.cpp:98
+  auto oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
+  std::unique_ptr<FunctionCaller> funcUP(func);
+
----------------
Just make unique_ptr the argument type so callers construct it in place.


================
Comment at: libc/utils/FPUtil/TestHelpers.cpp:105
+#else
+  int enabledExcepts = fegetexcept();
+  if (sigsetjmp(jumpBuffer, 1) == 0)
----------------
It's probably wise to use fegetenv / fesetenv here.



================
Comment at: libc/utils/FPUtil/TestHelpers.h:79
+      Func f;
+      explicit Callable(Func f) : f(f) {}
+      void call() override { f(); }
----------------
This ctor is superfluous if you just construct with `Callable{func}`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106086