[clang-tools-extra] 0c55ad1 - [clang-tidy] fix false positive that floating point variable only used in increment expr in cert-flp30-c (#108706)

[via cfe-commits]

Author: Congcong Cai
Date: 2024-09-16T10:16:48+08:00
New Revision: 0c55ad11ab3857056bb3917fdf087c4aa811b790

URL: https://github.com/llvm/llvm-project/commit/0c55ad11ab3857056bb3917fdf087c4aa811b790
DIFF: https://github.com/llvm/llvm-project/commit/0c55ad11ab3857056bb3917fdf087c4aa811b790.diff

LOG: [clang-tidy] fix false positive that floating point variable only used in increment expr in cert-flp30-c (#108706)

Fixes: #108049
cert-flp30-c only provides non-compliant example with normal loop.
Essentially it wants to avoid that floating point variables are used as
loop counters which are checked in condition expr and modified in
increment expr.
This patch wants to give more precise matcheres to identify this cases.

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp
index a9363837597867..46acc9f4716c5d 100644
--- a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp
+++ b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp
@@ -9,6 +9,7 @@
 #include "FloatLoopCounter.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 
 using namespace clang::ast_matchers;
 
@@ -16,15 +17,30 @@ namespace clang::tidy::cert {
 
 void FloatLoopCounter::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-      forStmt(hasIncrement(expr(hasType(realFloatingPointType())))).bind("for"),
+      forStmt(hasIncrement(forEachDescendant(
+                  declRefExpr(hasType(realFloatingPointType()),
+                              to(varDecl().bind("var")))
+                      .bind("inc"))),
+              hasCondition(forEachDescendant(
+                  declRefExpr(hasType(realFloatingPointType()),
+                              to(varDecl(equalsBoundNode("var"))))
+                      .bind("cond"))))
+          .bind("for"),
       this);
 }
 
 void FloatLoopCounter::check(const MatchFinder::MatchResult &Result) {
   const auto *FS = Result.Nodes.getNodeAs<ForStmt>("for");
 
-  diag(FS->getInc()->getExprLoc(), "loop induction expression should not have "
-                                   "floating-point type");
+  diag(FS->getInc()->getBeginLoc(), "loop induction expression should not have "
+                                    "floating-point type")
+      << Result.Nodes.getNodeAs<DeclRefExpr>("inc")->getSourceRange()
+      << Result.Nodes.getNodeAs<DeclRefExpr>("cond")->getSourceRange();
+
+  if (!FS->getInc()->getType()->isRealFloatingType())
+    if (const auto *V = Result.Nodes.getNodeAs<VarDecl>("var"))
+      diag(V->getBeginLoc(), "floating-point type loop induction variable",
+           DiagnosticIDs::Note);
 }
 
 } // namespace clang::tidy::cert

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index aea428016159b8..8d0c093b312dd5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/casting-through-void>` check to suggest replacing
   the offending code with ``reinterpret_cast``, to more clearly express intent.
 
+- Improved :doc:`cert-flp30-c<clang-tidy/checks/cert/flp30-c>` check to 
+  fix false positive that floating point variable is only used in increment
+  expression.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to avoid
   false positive when member initialization depends on a structured binging

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c b/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c
index eee16bee3d82f4..b9985887a81c53 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c
@@ -1,19 +1,28 @@
 // RUN: %check_clang_tidy %s cert-flp30-c %t
 
 float g(void);
+int c(float);
+float f = 1.0f;
+
+void match(void) {
 
-void func(void) {
   for (float x = 0.1f; x <= 1.0f; x += 0.1f) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: loop induction expression should not have floating-point type [cert-flp30-c]
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: loop induction expression should not have floating-point type [cert-flp30-c]
 
-  float f = 1.0f;
   for (; f > 0; --f) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop induction expression
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop induction expression should not have floating-point type [cert-flp30-c]
 
-  for (;;g()) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop induction expression
+  for (float x = 0.0f; c(x); x = g()) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: loop induction expression should not have floating-point type [cert-flp30-c]
 
-  for (int i = 0; i < 10; i += 1.0f) {}
+  for (int i=0; i < 10 && f < 2.0f; f++, i++) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: loop induction expression should not have floating-point type [cert-flp30-c]
+  // CHECK-MESSAGES: :5:1: note: floating-point type loop induction variable
+}
 
+void not_match(void) {
+  for (int i = 0; i < 10; i += 1.0f) {}
   for (int i = 0; i < 10; ++i) {}
+  for (int i = 0; i < 10; ++i, f++) {}
+  for (int i = 0; f < 10.f; ++i) {}
 }