llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Congcong Cai (HerrCai0907) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/108706.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp (+16-5) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3) - (modified) clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c (+16-7) ``````````diff diff --git a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp index a9363837597867..ecaa078cc44fa5 100644 --- a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp +++ b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp @@ -9,22 +9,33 @@ #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"), - this); + Finder->addMatcher(forStmt(hasIncrement(forEachDescendant( + declRefExpr(hasType(realFloatingPointType()), + to(varDecl().bind("var"))))), + hasCondition(forEachDescendant(declRefExpr( + hasType(realFloatingPointType()), + to(varDecl(equalsBoundNode("var"))))))) + .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"); + + 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 88b92830fda6b4..12638ec66c913a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -111,6 +111,9 @@ 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 only used in increment expr. + - Improved :doc:`modernize-use-std-format <clang-tidy/checks/modernize/use-std-format>` check to support replacing member function calls too. 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) {} } `````````` </details> https://github.com/llvm/llvm-project/pull/108706 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
