Skip to content

Commit

Permalink
[clang-tidy] fix false positive that floating point variable only use…
Browse files Browse the repository at this point in the history
…d in increment expr in cert-flp30-c (llvm#108706)

Fixes: llvm#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.
  • Loading branch information
HerrCai0907 authored Sep 16, 2024
1 parent 61ff1cb commit 0c55ad1
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 10 deletions.
22 changes: 19 additions & 3 deletions clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,38 @@
#include "FloatLoopCounter.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"

using namespace clang::ast_matchers;

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
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 16 additions & 7 deletions clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c
Original file line number Diff line number Diff line change
@@ -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) {}
}

0 comments on commit 0c55ad1

Please sign in to comment.