[clang-tools-extra] a7520d9 - [Clang-tidy] bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (#81183)

[via cfe-commits]

Author: Shourya Goel
Date: 2024-02-09T17:44:04+01:00
New Revision: a7520d9727d2638047e5c464b2937581f64e2ce5

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

LOG: [Clang-tidy] bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (#81183)

Changed LibASTMatcher to give an appropriate warning
when a const loop bound is initialized with a function declaration.

Fixes: #79580

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6f..a73d46f01d9b2d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -82,10 +82,14 @@ void TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
   // We are interested in only those cases when the loop bound is a variable
   // value (not const, enum, etc.).
   StatementMatcher LoopBoundMatcher =
-      expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
-                                       unless(integerLiteral()),
-                                       unless(hasType(isConstQualified())),
-                                       unless(hasType(enumType())))))
+      expr(ignoringParenImpCasts(allOf(
+               hasType(isInteger()), unless(integerLiteral()),
+               unless(allOf(
+                   hasType(isConstQualified()),
+                   declRefExpr(to(varDecl(anyOf(
+                       hasInitializer(ignoringParenImpCasts(integerLiteral())),
+                       isConstexpr(), isConstinit())))))),
+               unless(hasType(enumType())))))
           .bind(LoopUpperBoundName);
 
   // We use the loop increment expression only to make sure we found the right

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07a..dff8dd279940f8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -117,6 +117,10 @@ Changes in existing checks
   options `HeaderFileExtensions` and `ImplementationFileExtensions` by the
   global options of the same name.
 
+- Improved :doc:`bugprone-too-small-loop-variable
+  <clang-tidy/checks/bugprone/too-small-loop-variable>` support by correctly 
+  implementing the check for const loop boundary.
+
 - Cleaned up :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>`
   by removing enforcement of rule `C.48

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe11463..2c3ded952aa022 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -28,6 +28,10 @@ In a real use case size means a container's size which depends on the user input
 This algorithm works for a small amount of objects, but will lead to freeze for
 a larger user input.
 
+It's recommended to enable the compiler warning
+`-Wtautological-constant-out-of-range-compare` as well, since check does not
+inspect compile-time constant loop boundaries to avoid overlaps with the warning.
+
 .. option:: MagnitudeBitsUpperLimit
 
   Upper limit for the magnitude bits of the loop variable. If it's set the check

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada8..113150b168650b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
   }
 }
 
+unsigned int getVal() {
+    return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+  const unsigned int l = getVal();
+  for (unsigned char i = 0; i < l; ++i) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower type 'unsigned char' than iteration's upper bound 'const unsigned int' [bugprone-too-small-loop-variable]
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// Correct loops: we should not warn here.