Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<cmath>, <limits>: Locally disable -Wnan-infinity-disabled for Clang #4990

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Sep 29, 2024

Fixes #4931.

Possibly because of that numeric_limits<FP> are implemeted as full specializations, when one attempts use min() or epsilon(), Clang emits warning for Nan and Inf related functions, even if these functions are not themselves called.

This PR suppresses -Wnan-infinity-disabled in <limits> and <cmath> locally to avoid such false positives. When a user explicitly calls infinity(), quiet_NaN(), or signaling_NaN(), the warning is still triggered (Godbolt link).

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner September 29, 2024 06:52
@frederick-vs-ja frederick-vs-ja changed the title <limits>: Locally disable -Wnan-infinity-disabled for Clang <cmath>, <limits>: Locally disable -Wnan-infinity-disabled for Clang Sep 29, 2024
@frederick-vs-ja frederick-vs-ja force-pushed the locally-no-nan-infinity-disabled branch from fda6ed1 to fe88346 Compare September 29, 2024 07:27
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Sep 29, 2024
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppressing these warnings at template parse time while allowing them at instantiation is an excellent solution. I didn't know this was possible before.

@StephanTLavavej StephanTLavavej self-assigned this Sep 29, 2024
@StephanTLavavej
Copy link
Member

Thanks! 😸 ♾️ @CaseyCarter I pushed a macro refactoring after you approved, moving them to <yvals_core.h> below our usual Clang suppression macros, using the same escape-hatch pattern (just in case), and recording the warnings that they're suppressing in comments.

@StephanTLavavej StephanTLavavej removed their assignment Sep 30, 2024
@StephanTLavavej StephanTLavavej self-assigned this Oct 11, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 4a08b52 into microsoft:main Oct 12, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

♾️ 🚫 #️⃣

@StephanTLavavej StephanTLavavej added bug Something isn't working and removed enhancement Something can be improved labels Oct 12, 2024
@frederick-vs-ja frederick-vs-ja deleted the locally-no-nan-infinity-disabled branch October 12, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Clang 18+ /fp:fast emits -Wnan-infinity-disabled in VSO_0000000_vector_algorithms_floats
3 participants