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

[Alex Brachet via Phabricator via libc-commits]

abrachet added a comment.

Do you anticipate this will be used later to match against if any exception was raised? I imagine in that case the current logic would be moved from the constructor into `match`.

If not, I think this would be a lot simpler if the logic inside of `FPExceptMatcher::FPExceptMatcher` was just a function called `testSigfpeRaised` which returned `caughtExcept` and then `RAISES_FP_EXCEPT` would just be `ASSERT_TRUE(testSigfpeRaised(func))`

Also, what does this solve that the death tests do not, Windows support?



================
Comment at: libc/utils/FPUtil/TestHelpers.cpp:19
 
+#include <iostream>
+
----------------
Not used


================
Comment at: libc/utils/FPUtil/TestHelpers.cpp:88
+#else
+    siglongjmp(jumpBuffer, -1);
+#endif
----------------
`siglongjmp` takes a `sigjmp_buf` not `jmp_buf`. I'm guessing `[sig]jmp_buf` are macros to the same type but we should still use `sigjmp_buf` when not in Windows.


================
Comment at: libc/utils/FPUtil/TestHelpers.h:86
+
+  using FuncType = void(void);
+  explicit FPExceptMatcher(FunctionCaller *Func);
----------------
Not used


================
Comment at: libc/utils/FPUtil/TestHelpers.h:134
 
+#define RAISES_FP_EXCEPT(func)                                                 \
+  ASSERT_THAT(                                                                 \
----------------
Should this be called `ASSERT_RAISES_FP_EXCEPT`?


================
Comment at: libc/utils/FPUtil/TestHelpers.h:138
+      __llvm_libc::fputil::testing::FPExceptMatcher(                           \
+          __llvm_libc::fputil::testing::FPExceptMatcher::getFunctionCaller(    \
+              func)))
----------------
Can this be `__llvm_libc::testing::Test::createCallable(func)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106086