[libc-commits] [libc] [libc][math] adds more FP test macros and fixes bug in `{EXPECT, ASSERT}_FP_EXCEPTION` (PR #88816)

Michael Flanders via libc-commits libc-commits at lists.llvm.org
Mon Apr 29 14:03:36 PDT 2024


Flandini wrote:

> Sorry for the late review! Can you double check if this PR might be related / intersected with #89658? Thanks,

This PR adds the `{EXCEPT,ASSERT}_FP_EXCEPTION*` macros for individual invocations of a function call, #89658 adds a wrapper for the `TEST_F` calls to check that `fenv_t` state hasn't changed. Noted in #89658, [here](https://github.com/llvm/llvm-project/pull/89658/files#diff-e696ffb0d7cc0d21488991de1b3397ca5d71923468e9fb767bfa72dc93fa41bcR87 ), we still want both of these, so this PR is still relevant. 

Also, this PR starts to address the point in that above comment to start strengthening expectations of more individual function invocations to also check their errno/exceptions, along the lines of #61092 and #88819.

If we move towards better organizing tests around function invocations that _should set_ errno/exceptions, i.e., move them to their own `TEST_F`, then an approach like #89658 but for checking that fenv_t _has changed_, would probably be preferable, and this PR could drop some of the macros like `EXPECT_FP_EQ_WITH_ERRNO` and `EXPECT_FP_EQ_WITH_EXCEPTION`. Thoughts?

For functions that _should not set_ errno/exceptions, I think we still want the individual `ASSERT_FP_EQ_NO_ERRNO_EXCEPTION` and `EXPECT_FP_EQ_ALL_ROUNDING_NO_ERRNO_EXCEPTION` from this PR. There is an intersection noted in #89658, that test fixtures already reset fenv_t on test entry, so do we want all FP tests wrapped in `FEnvSafeTest`?


https://github.com/llvm/llvm-project/pull/88816


More information about the libc-commits mailing list