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

Thu Jul 15 13:47:06 PDT 2021

sivachandra marked an inline comment as done.
sivachandra added a comment.

In D106086#2881171 <https://reviews.llvm.org/D106086#2881171>, @abrachet wrote:

> 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`.

Yes, I would like to extend this to match the actual raised exception. So, some kind of reordering might be required.

> 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))`

The appeal of the matcher over an `ASSERT_TRUE(...)` kind of check is the custom error message.

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

That is the primary motivation. However, I am not yet sure how this will fit with Fuchsia's scheme of things.

================
Comment at: libc/utils/FPUtil/TestHelpers.h:138
+      __llvm_libc::fputil::testing::FPExceptMatcher(                           \
+          __llvm_libc::fputil::testing::FPExceptMatcher::getFunctionCaller(    \
+              func)))
----------------
abrachet wrote:
> Can this be `__llvm_libc::testing::Test::createCallable(func)`
Yes, we could. I am trying not to grow a dependency from `testutils/ExecuteFunction.h`. I want to move `testutils` in to `UnitTest` as soon as I can. So, we can do that cleanup at that time.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106086