search

fmtlib / fmt

A modern formatting library
https://fmt.dev
Other
19.9k stars 2.43k forks source link

Fix `isnan` so it doesn't cause FP errors #3951

Closed alexdewar closed 2 months ago

alexdewar commented 2 months ago

As described in #3948, the current internal implementation of isnan triggers a floating-point exception as a side effect, unlike std::isnan. This can cause issues for users attempting to find other causes of FP exceptions in their code.

Fix by changing isnan to use value != value as a test, which does not have side effects.

I also added a test to confirm that FP exceptions are not raised (as suggested by @vitaut). I've confirmed that it fails with the current implementation and passes with this one.

Fixes #3948.

phprus commented 2 months ago

@vitaut, What do you think about adding a GCC and Clang pragma's to suppress -Wfloat-equal warning?

vitaut commented 2 months ago

What do you think about adding a GCC and Clang pragma's to suppress -Wfloat-equal warning?

Considering that -Wfloat-equal is not in -Wall -Wextra I don't think we need to do anything. If someone wants to enable this warning, they could use FMT_SYSTEM to suppress it in {fmt} similarly to how it is done for other esoteric warnings.

vitaut commented 2 months ago

Thank you!