[CLANG] Fix INF/NAN warning. #80290

zahiraam · 2024-02-01T13:31:39Z

In #76873 a warning was added when the macros INFINITY and NAN are used in binary expressions when -menable-no-nans or -menable-no-infs are used. If the user uses an option that nullifies these two options, the warning will still be generated. This patch adds an additional information to the warning comment to let the user know about this. It also suppresses the warning when #ifdef INFINITY, #ifdef NAN, #ifdef NAN or #ifndef NAN are used in the code.

github-actions · 2024-02-01T13:34:11Z

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

clang/include/clang/Basic/DiagnosticCommonKinds.td

AaronBallman · 2024-02-01T16:01:15Z

clang/include/clang/Lex/Preprocessor.h

+      bool UnsafeMath = getLangOpts().UnsafeFPMath;
+      if (Info->getName() == "INFINITY")
+        if (getLangOpts().NoHonorInfs)
+          emitRestrictInfNaNWarning(Identifier, 0, UnsafeMath);
+      if (Info->getName() == "NAN")
+        if (getLangOpts().NoHonorNaNs)
+          emitRestrictInfNaNWarning(Identifier, 1, UnsafeMath);


Suggested change

bool UnsafeMath = getLangOpts().UnsafeFPMath;

if (Info->getName() == "INFINITY")

if (getLangOpts().NoHonorInfs)

emitRestrictInfNaNWarning(Identifier, 0, UnsafeMath);

if (Info->getName() == "NAN")

if (getLangOpts().NoHonorNaNs)

emitRestrictInfNaNWarning(Identifier, 1, UnsafeMath);

bool UnsafeMath = getLangOpts().UnsafeFPMath;

if (Info->getName() == "INFINITY" && getLangOpts().NoHonorInfs)

emitRestrictInfNaNWarning(Identifier, 0, UnsafeMath);

if (Info->getName() == "NAN" && getLangOpts().NoHonorNaNs)

emitRestrictInfNaNWarning(Identifier, 1, UnsafeMath);

clang/lib/Sema/SemaChecking.cpp

clang/lib/Lex/PPDirectives.cpp

AaronBallman

You should also add test coverage for #if defined(INFINITY) and #elifndef NAN to make sure we're catching the other forms of checking whether an identifier is a defined macro or not.

clang/lib/Lex/PPDirectives.cpp

llvmbot · 2024-02-02T13:35:56Z

@llvm/pr-subscribers-clang

Author: Zahira Ammarguellat (zahiraam)

Changes

In #76873 a warning was added when the macros INFINITY and NAN are used in binary expressions when -menable-no-nans or -menable-no-infs are used. If the user uses an option that nullifies these two options, the warning will still be generated. This patch adds an additional information to the warning comment to let the user know about this. It also suppresses the warning when #ifdef INFINITY, #ifdef NAN, #ifdef NAN or #ifndef NAN are used in the code.

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

7 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticCommonKinds.td (+1-1)
(modified) clang/include/clang/Basic/DiagnosticDocs.td (+7)
(modified) clang/include/clang/Lex/Preprocessor.h (+6-5)
(modified) clang/lib/Lex/PPDirectives.cpp (+1-1)
(modified) clang/lib/Lex/PPExpressions.cpp (+3-1)
(modified) clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp (+31-7)
(modified) clang/test/Sema/warn-infinity-nan-disabled-win.cpp (+31-7)

diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index b1bada65cb6b2..a9779992a49c2 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -73,7 +73,7 @@ def warn_pragma_debug_unexpected_argument : Warning<
 def warn_fp_nan_inf_when_disabled : Warning<
   "use of %select{infinity|NaN}0%select{| via a macro}1 is undefined behavior "
   "due to the currently enabled floating-point options">,
-  InGroup<DiagGroup<"nan-infinity-disabled">>;
+  InGroup<DiagGroup<"NanInfDisabledDocs">>;
 }
 
 // Parse && Sema
diff --git a/clang/include/clang/Basic/DiagnosticDocs.td b/clang/include/clang/Basic/DiagnosticDocs.td
index e9862422b4997..df153bae4567d 100644
--- a/clang/include/clang/Basic/DiagnosticDocs.td
+++ b/clang/include/clang/Basic/DiagnosticDocs.td
@@ -87,3 +87,10 @@ program by treating all string literals as having type ``const char *``
 instead of ``char *``. This can cause unexpected behaviors with type-sensitive
 constructs like ``_Generic``.
 }];
+
+defvar NanInfDisabledDocs = [{
+This warning is enabled when source code using the macros INFINITY and/or NAN
+is compiled with floating-point options preventing these two values. This can
+lead to undefined behavior. Some command line combinations order and pragmas
+may have an impact and a warning can be generated when not expected.
+}];
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 9d0d53129a12d..0836b7d439bb0 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2838,7 +2838,8 @@ class Preprocessor {
     return AnnotationInfos.find(II)->second;
   }
 
-  void emitMacroExpansionWarnings(const Token &Identifier) const {
+  void emitMacroExpansionWarnings(const Token &Identifier,
+                                  bool IsIfnDef = false) const {
     IdentifierInfo *Info = Identifier.getIdentifierInfo();
     if (Info->isDeprecatedMacro())
       emitMacroDeprecationWarning(Identifier);
@@ -2847,12 +2848,12 @@ class Preprocessor {
         !SourceMgr.isInMainFile(Identifier.getLocation()))
       emitRestrictExpansionWarning(Identifier);
 
-    if (Info->getName() == "INFINITY")
-      if (getLangOpts().NoHonorInfs)
+    if (!IsIfnDef) {
+      if (Info->getName() == "INFINITY" && getLangOpts().NoHonorInfs)
         emitRestrictInfNaNWarning(Identifier, 0);
-    if (Info->getName() == "NAN")
-      if (getLangOpts().NoHonorNaNs)
+      if (Info->getName() == "NAN" && getLangOpts().NoHonorNaNs)
         emitRestrictInfNaNWarning(Identifier, 1);
+    }
   }
 
   static void processPathForFileMacro(SmallVectorImpl<char> &Path,
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 9f82a6d073e3b..a980f4bcbae12 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -3288,7 +3288,7 @@ void Preprocessor::HandleIfdefDirective(Token &Result,
     return;
   }
 
-  emitMacroExpansionWarnings(MacroNameTok);
+  emitMacroExpansionWarnings(MacroNameTok, /*IsIfnDef=*/true);
 
   // Check to see if this is the last token on the #if[n]def line.
   CheckEndOfDirective(isIfndef ? "ifndef" : "ifdef");
diff --git a/clang/lib/Lex/PPExpressions.cpp b/clang/lib/Lex/PPExpressions.cpp
index 1feb0eb18d71e..8f25c67ec9dfb 100644
--- a/clang/lib/Lex/PPExpressions.cpp
+++ b/clang/lib/Lex/PPExpressions.cpp
@@ -133,7 +133,9 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
   Result.Val.setIsUnsigned(false); // Result is signed intmax_t.
   DT.IncludedUndefinedIds = !Macro;
 
-  PP.emitMacroExpansionWarnings(PeekTok);
+  PP.emitMacroExpansionWarnings(
+      PeekTok,
+      (II->getName() == "INFINITY" || II->getName() == "NAN") ? true : false);
 
   // If there is a macro, mark it used.
   if (Result.Val != 0 && ValueLive)
diff --git a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
index 8a610fa0e737e..67a2611374a13 100644
--- a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
+++ b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
@@ -1,13 +1,19 @@
-// RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan -triple powerpc64le-unknown-unknown %s \
-// RUN: -menable-no-infs -menable-no-nans
+// RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
+// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
+// RUN: -menable-no-nans -std=c++23
 
-// RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown %s
+// RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
+// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
+// RUN: -menable-no-nans -funsafe-math-optimizations -std=c++23
+
+// RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown \
+// RUN: %s -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf -triple powerpc64le-unknown-unknown %s \
-// RUN: -menable-no-infs
+// RUN: -menable-no-infs -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-nan -triple powerpc64le-unknown-unknown %s \
-// RUN: -menable-no-nans
+// RUN: -menable-no-nans -std=c++23
 
 // no-fast-no-diagnostics
 
@@ -133,13 +139,31 @@ int compareit(float a, float b) {
 // no-inf-warning@+1 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
   p = __builtin_isfinite(a);
 
-  // These should NOT warn, since they are not using NaN or infinity.
+// These should NOT warn, since they are not using NaN or infinity.
   j = a > 1.1;
   j = b < 1.1;
   j = a >= 1.1;
   j = b <= 1.1;
   j = isunorderedf(a, b);
 
+#ifndef INFINITY
+  j = a;
+#endif
+#ifndef NAN
+  j = b;
+#endif
+#ifdef INFINITY
+  j = a;
+#endif
+#ifdef NAN
+  j = b;
+#endif
+#if defined(INFINITY)
+  j = a;
+#elifndef(INFINITY)
+  j = b;
+#endif
+
 // no-inf-no-nan-warning@+4 {{use of NaN 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-nan-warning@+2 {{use of NaN via a macro is undefined behavior due to the currently enabled floating-point options}}
@@ -173,4 +197,4 @@ int compareit(float a, float b) {
   j = numeric_limits<float>::infinity();
   return 0;
 
-}  
+}
diff --git a/clang/test/Sema/warn-infinity-nan-disabled-win.cpp b/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
index 19a575386e329..a904cfbd04f75 100644
--- a/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
+++ b/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
@@ -1,16 +1,22 @@
 // Use of NAN macro will trigger a warning "infinity defined in macro" because
 // on Windows the NAN macro is defined using INFINITY. See below.
 
-// RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan -triple powerpc64le-unknown-unknown %s \
-// RUN: -menable-no-infs -menable-no-nans
+// RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
+// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
+// RUN: -menable-no-nans -std=c++23
 
-// RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown %s
+// RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
+// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
+// RUN: -menable-no-nans -funsafe-math-optimizations -std=c++23
+
+// RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown \
+// RUN: %s -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf -triple powerpc64le-unknown-unknown %s \
-// RUN: -menable-no-infs
+// RUN: -menable-no-infs -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-nan -triple powerpc64le-unknown-unknown %s \
-// RUN: -menable-no-nans
+// RUN: -menable-no-nans -std=c++23
 
 // no-fast-no-diagnostics
 
@@ -136,13 +142,31 @@ int compareit(float a, float b) {
 // no-inf-warning@+1 {{use of infinity is undefined behavior due to the currently enabled floating-point options}}
   p = __builtin_isfinite(a);
 
-  // These should NOT warn, since they are not using NaN or infinity.
+// These should NOT warn, since they are not using NaN or infinity.
   j = a > 1.1;
   j = b < 1.1;
   j = a >= 1.1;
   j = b <= 1.1;
   j = isunorderedf(a, b);
 
+#ifndef INFINITY
+  j = a;
+#endif
+#ifndef NAN
+  j = b;
+#endif
+#ifdef INFINITY
+  j = a;
+#endif
+#ifdef NAN
+  j = b;
+#endif
+#if defined(INFINITY)
+  j = a;
+#elifndef(INFINITY)
+  j = b;
+#endif
+
 // no-inf-no-nan-warning@+4 {{use of infinity via a macro is undefined behavior due to the currently enabled floating-point option}}
 // no-inf-no-nan-warning@+3 {{use of NaN via a macro 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}}
@@ -176,4 +200,4 @@ int compareit(float a, float b) {
   j = numeric_limits<float>::infinity();
   return 0;
 
-}  
+}

AaronBallman · 2024-02-05T15:29:04Z

clang/include/clang/Basic/DiagnosticCommonKinds.td

@@ -73,7 +73,7 @@ def warn_pragma_debug_unexpected_argument : Warning<
 def warn_fp_nan_inf_when_disabled : Warning<
  "use of %select{infinity|NaN}0%select{| via a macro}1 is undefined behavior "
  "due to the currently enabled floating-point options">,
-  InGroup<DiagGroup<"nan-infinity-disabled">>;
+  InGroup<DiagGroup<"NanInfDisabledDocs">>;


Suggested change

InGroup<DiagGroup<"NanInfDisabledDocs">>;

InGroup<DiagGroup<"nan-infinity-disabled", NanInfDisabledDocs>>;

You were naming the warning group NanInfDisabledDocs, you still need the group name. This suggests we're missing test coverage because no tests broke despite the warning group name changing.

Thanks @AaronBallman. I have added a few other lines in the LIT test. I can't think of anything else.

AaronBallman · 2024-02-05T15:33:06Z

clang/include/clang/Basic/DiagnosticDocs.td

+This warning is enabled when source code using the macros INFINITY and/or NAN
+is compiled with floating-point options preventing these two values. This can
+lead to undefined behavior. Some command line combinations order and pragmas
+may have an impact and a warning can be generated when not expected.


Suggested change

This warning is enabled when source code using the macros INFINITY and/or NAN

is compiled with floating-point options preventing these two values. This can

lead to undefined behavior. Some command line combinations order and pragmas

may have an impact and a warning can be generated when not expected.

This warning is enabled when source code using the macros ``INFINITY`` or ``NAN``

is compiled with floating-point options preventing these two values. This can

lead to undefined behavior. Check the order of command line arguments that modify

this behavior, such as ``-ffast-math``, ``-fhonor-infinities``, and ``-fhonor-nans`` (etc), as

well as ``#pragma`` directives if this diagnostic is generated unexpectedly.

AaronBallman

LGTM, thank you!

zahiraam · 2024-02-06T14:22:22Z

Tagging @zeroomega and @mikaelholmen for awareness.

mikaelholmen · 2024-02-06T14:24:31Z

Thanks!

In llvm#76873 a warning was added when the macros INFINITY and NAN are used in binary expressions when -menable-no-nans or -menable-no-infs are used. If the user uses an option that nullifies these two options, the warning will still be generated. This patch adds an additional information to the warning comment to let the user know about this. It also suppresses the warning when #ifdef INFINITY, #ifdef NAN, #ifdef NAN or #ifndef NAN are used in the code. (cherry picked from commit 62c352e)

Fix INF/NAN warning.

529879c

Fix format.

0deb2b6

zahiraam requested a review from AaronBallman February 1, 2024 13:49

AaronBallman reviewed Feb 1, 2024

View reviewed changes

clang/lib/Lex/PPDirectives.cpp Outdated Show resolved Hide resolved

Addressed review comments.

d33ec0e

zahiraam marked this pull request as ready for review February 2, 2024 13:35

zahiraam requested a review from AaronBallman February 2, 2024 13:35

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

zahiraam changed the title ~~Fix INF/NAN warning.~~ [CLANG] Fix INF/NAN warning. Feb 2, 2024

AaronBallman reviewed Feb 5, 2024

View reviewed changes

zahiraam added 3 commits February 5, 2024 10:57

Fixed diag dog comment and added a few lines in the LIT tests.

ad10f4a

Added 2 additional run lines to the tests.

7656878

Added 2 additional run lines requested from reviewer.

33fb103

zahiraam requested a review from AaronBallman February 6, 2024 13:21

AaronBallman approved these changes Feb 6, 2024

View reviewed changes

zahiraam merged commit 62c352e into llvm:main Feb 6, 2024
5 checks passed

zahiraam mentioned this pull request Feb 6, 2024

Please backport 62c352e13c14 to release/18.x #80843

Closed

pointhex mentioned this pull request May 7, 2024

getStyleDiagHandler #91314

Closed

aemerson mentioned this pull request May 9, 2024

release/18.x: [AArc64][GlobalISel] Fix legalizer assert for G_INSERT_VECTOR_ELT - manual merge #91672

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CLANG] Fix INF/NAN warning. #80290

[CLANG] Fix INF/NAN warning. #80290

zahiraam commented Feb 1, 2024 •

edited

Loading

github-actions bot commented Feb 1, 2024 •

edited

Loading

AaronBallman Feb 1, 2024

AaronBallman left a comment

llvmbot commented Feb 2, 2024

AaronBallman Feb 5, 2024

zahiraam Feb 5, 2024

AaronBallman Feb 5, 2024

AaronBallman left a comment

zahiraam commented Feb 6, 2024

mikaelholmen commented Feb 6, 2024

	InGroup<DiagGroup<"NanInfDisabledDocs">>;
	InGroup<DiagGroup<"nan-infinity-disabled", NanInfDisabledDocs>>;

[CLANG] Fix INF/NAN warning. #80290

[CLANG] Fix INF/NAN warning. #80290

Conversation

zahiraam commented Feb 1, 2024 • edited Loading

github-actions bot commented Feb 1, 2024 • edited Loading

AaronBallman Feb 1, 2024

Choose a reason for hiding this comment

AaronBallman left a comment

Choose a reason for hiding this comment

llvmbot commented Feb 2, 2024

AaronBallman Feb 5, 2024

Choose a reason for hiding this comment

zahiraam Feb 5, 2024

Choose a reason for hiding this comment

AaronBallman Feb 5, 2024

Choose a reason for hiding this comment

AaronBallman left a comment

Choose a reason for hiding this comment

zahiraam commented Feb 6, 2024

mikaelholmen commented Feb 6, 2024

zahiraam commented Feb 1, 2024 •

edited

Loading

github-actions bot commented Feb 1, 2024 •

edited

Loading