[CLANG] Emit warning in finite math mode when INF and NAN are used. #99672

zahiraam · 2024-07-19T17:35:50Z

In finite math mode when special math builtins __builtin_inf and __builtin_nan are used a warning is emitted when the builtin is expanded and at call point.
This warning at call point was missing for __builtin_inf and this patch fixes the issue (#98018).

llvmbot · 2024-07-19T18:45:16Z

@llvm/pr-subscribers-clang

Author: Zahira Ammarguellat (zahiraam)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/99672.diff

3 Files Affected:

(modified) clang/lib/Sema/SemaChecking.cpp (+2-1)
(modified) clang/test/Headers/float.c (+2-1)
(modified) clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp (+20-8)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 45b9bbb23dbf7..4bf115d6efb8b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -8208,7 +8208,8 @@ void Sema::CheckInfNaNFunction(const CallExpr *Call,
         << 1 << 0 << Call->getSourceRange();
   else if ((IsStdFunction(FDecl, "isinf") ||
             (IsStdFunction(FDecl, "isfinite") ||
-             (FDecl->getIdentifier() && FDecl->getName() == "infinity"))) &&
+             (FDecl->getIdentifier() && FDecl->getName() == "infinity") ||
+             (Call->getBuiltinCallee() == Builtin::BI__builtin_inff))) &&
            FPO.getNoHonorInfs())
     Diag(Call->getBeginLoc(), diag::warn_fp_nan_inf_when_disabled)
         << 0 << 0 << Call->getSourceRange();
diff --git a/clang/test/Headers/float.c b/clang/test/Headers/float.c
index d524d0e53f3fd..7e156e7dd4a34 100644
--- a/clang/test/Headers/float.c
+++ b/clang/test/Headers/float.c
@@ -224,7 +224,8 @@
     #error "Mandatory macro NAN is missing."
   #endif
   // FIXME: the NAN diagnostic should only be issued once, not twice.
-  _Static_assert(_Generic(INFINITY, float : 1, default : 0), ""); // finite-warning {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+  _Static_assert(_Generic(INFINITY, float : 1, default : 0), ""); // finite-warning {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}} \
+								  finite-warning {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
   _Static_assert(_Generic(NAN, float : 1, default : 0), ""); // finite-warning {{use of NaN is undefined behavior due to the currently enabled floating-point options}} \
                                                                 finite-warning {{use of NaN via a macro is undefined behavior due to the currently enabled floating-point options}}
 
diff --git a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
index 03a432e05851d..d18aaad0e6925 100644
--- a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
+++ b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
@@ -87,11 +87,15 @@ class numeric_limits<double>  {
 
 int compareit(float a, float b) {
   volatile int i, j, k, l, m, n, o, p;
-// no-inf-no-nan-warning@+2 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+4 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+3 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-warning@+2 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
 // no-inf-warning@+1 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
   i = a == INFINITY;
 
-// no-inf-no-nan-warning@+2 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+4 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+3 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-warning@+2 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
 // no-inf-warning@+1 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
   j = INFINITY == a;
 
@@ -107,11 +111,15 @@ int compareit(float a, float b) {
 // no-nan-warning@+1 {{use of NaN via a macro is undefined behavior due to the currently enabled floating-point options}}
   j = NAN == a;
 
-// no-inf-no-nan-warning@+2 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+4 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+3 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-warning@+2 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
 // no-inf-warning@+1 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
   j = INFINITY <= a;
 
-// no-inf-no-nan-warning@+2 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+4 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+3 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-warning@+2 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
 // no-inf-warning@+1 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
   j = INFINITY < a;
 
@@ -192,7 +200,9 @@ int compareit(float a, float b) {
 // no-nan-warning@+1 {{use of NaN is undefined behavior due to the currently enabled floating-point options}}
   j = isunorderedf(a, NAN);
 
-// no-inf-no-nan-warning@+2 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+4 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+3 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-warning@+2 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
 // no-inf-warning@+1 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
   j = isunorderedf(a, INFINITY);
 
@@ -204,9 +214,11 @@ int compareit(float a, float b) {
 // no-nan-warning@+1 {{use of NaN is undefined behavior due to the currently enabled floating-point options}}
   i = std::isunordered(a, NAN);
 
-// no-inf-no-nan-warning@+4 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
-// no-inf-no-nan-warning@+3 {{use of NaN is undefined behavior due to the currently enabled floating-point options}}
-// no-inf-warning@+2 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+6 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+5 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-no-nan-warning@+4 {{use of NaN is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-warning@+3 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point options}}
+// no-inf-warning@+2 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
 // no-nan-warning@+1 {{use of NaN is undefined behavior due to the currently enabled floating-point options}}
   i = std::isunordered(a, INFINITY);

AaronBallman

Please update the patch title and description to describe what changes the patch is making rather than just linking to an issue.

AaronBallman · 2024-07-22T14:21:57Z

clang/lib/Sema/SemaChecking.cpp

@@ -8208,7 +8208,8 @@ void Sema::CheckInfNaNFunction(const CallExpr *Call,
        << 1 << 0 << Call->getSourceRange();
  else if ((IsStdFunction(FDecl, "isinf") ||
            (IsStdFunction(FDecl, "isfinite") ||
-             (FDecl->getIdentifier() && FDecl->getName() == "infinity"))) &&
+             (FDecl->getIdentifier() && FDecl->getName() == "infinity") ||
+             (Call->getBuiltinCallee() == Builtin::BI__builtin_inff))) &&


What about the other inf builtins?

case Builtin::BI__builtin_inf: case Builtin::BI__builtin_inff: case Builtin::BI__builtin_infl: case Builtin::BI__builtin_inff16: case Builtin::BI__builtin_inff128: {

github-actions · 2024-07-22T20:51:32Z

✅ With the latest revision this PR passed the C/C++ code formatter.

AaronBallman · 2024-07-24T13:30:09Z

clang/lib/Sema/SemaChecking.cpp

@@ -8198,20 +8198,48 @@ static bool IsStdFunction(const FunctionDecl *FDecl,
  return true;
 }

+enum class MathCheck { NaN, Inf };
+static bool IsSpecialMathFunction(StringRef calleeName, MathCheck Check) {


Err "special math function" may be a loaded term: https://eel.is/c++draft/numerics#sf.cmath

How about IsInfOrNanFunction() instead?

AaronBallman · 2024-07-24T13:31:14Z

clang/test/Headers/float.c

@@ -224,7 +224,8 @@
    #error "Mandatory macro NAN is missing."
  #endif
  // FIXME: the NAN diagnostic should only be issued once, not twice.


This comment should be updated because now we're issuing the inf and nan warnings twice instead of once.

AaronBallman · 2024-07-25T13:28:49Z

clang/test/Headers/float.c

@@ -223,8 +223,8 @@
  #ifndef NAN
    #error "Mandatory macro NAN is missing."
  #endif
-  // FIXME: the NAN diagnostic should only be issued once, not twice.


The fixme shouldn't be removed but updated to say the infinity and nan diagnostics should only be issued once, not twice.

into FixIssue98018

AaronBallman

LGTM!

…99672) Summary: In finite math mode when special math builtins `__builtin_inf` and `__builtin_nan` are used a warning is emitted when the builtin is expanded and at call point. This warning at call point was missing for` __builtin_inf` and this patch fixes the issue (#98018). Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250564

Fix issue llvm#98018.

452bc42

zahiraam marked this pull request as ready for review July 19, 2024 18:44

zahiraam requested a review from AaronBallman July 19, 2024 18:44

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 19, 2024

AaronBallman reviewed Jul 22, 2024

View reviewed changes

Addressed review comments.

523f73d

zahiraam changed the title ~~Fix issue https://github.com/llvm/llvm-project/issues/98018.~~ [CLANG] Emit warning in finite math mode when INF and NAN are used. Jul 22, 2024

Fix format.

36094bf

zahiraam requested a review from AaronBallman July 23, 2024 18:39

AaronBallman reviewed Jul 24, 2024

View reviewed changes

Addressed review comments.

24b1d4c

AaronBallman reviewed Jul 25, 2024

View reviewed changes

zahiraam and others added 4 commits July 25, 2024 06:36

Put back the FIXME.

057098f

Put back the FIXME.

6c38f89

Merge branch 'FixIssue98018' of https://github.com/zahiraam/llvm-project

93c7f57

into FixIssue98018

Fix LIT test.

a7cb20f

AaronBallman approved these changes Jul 25, 2024

View reviewed changes

zahiraam merged commit 342328d into llvm:main Jul 25, 2024
7 checks passed

zahiraam mentioned this pull request Jul 25, 2024

Duplicate diagnostics on use of NAN #98018

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CLANG] Emit warning in finite math mode when INF and NAN are used. #99672

[CLANG] Emit warning in finite math mode when INF and NAN are used. #99672

Uh oh!

zahiraam commented Jul 19, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jul 19, 2024

Uh oh!

AaronBallman left a comment

Uh oh!

AaronBallman Jul 22, 2024

Uh oh!

github-actions bot commented Jul 22, 2024 •

edited

Loading

Uh oh!

AaronBallman Jul 24, 2024

Uh oh!

AaronBallman Jul 24, 2024

Uh oh!

AaronBallman Jul 25, 2024

Uh oh!

AaronBallman left a comment

Uh oh!

Uh oh!

Uh oh!

[CLANG] Emit warning in finite math mode when INF and NAN are used. #99672

[CLANG] Emit warning in finite math mode when INF and NAN are used. #99672

Uh oh!

Conversation

zahiraam commented Jul 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 19, 2024

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

AaronBallman Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronBallman Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

AaronBallman Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

AaronBallman Jul 25, 2024

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zahiraam commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 22, 2024 •

edited

Loading