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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
