Skip to content

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

Merged
vitaut merged 2 commits into
fmtlib:masterfrom
alexdewar:fix_isnan
May 2, 2024
Merged

vitaut merged 2 commits into
fmtlib:masterfrom
alexdewar:fix_isnan

Conversation

@alexdewar

@alexdewar alexdewar commented May 2, 2024

Copy link
Copy Markdown
Contributor

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

phprus commented May 2, 2024

Copy link
Copy Markdown
Contributor

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

@vitaut

vitaut commented May 2, 2024

Copy link
Copy Markdown
Contributor

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
vitaut merged commit 9234fe8 into fmtlib:master May 2, 2024
@vitaut

vitaut commented May 2, 2024

Copy link
Copy Markdown
Contributor

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fmt::isnan triggers floating-point exception for NaN values

3 participants