This revision was automatically updated to reflect the committed changes.
Closed by commit rL358356: [clang-tidy] Add MagnitudeBitsUpperLimit option to  
bugprone-too-small-loop… (authored by ztamas, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59870?vs=194261&id=195058#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59870/new/

https://reviews.llvm.org/D59870

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
  
clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
  clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- -- --target=x86_64-linux
+
+// MagnitudeBitsUpperLimit = 16 (default value)
+
+unsigned long size() { return 294967296l; }
+
+void voidFilteredOutForLoop1() {
+  for (long i = 0; i < size(); ++i) {
+    // no warning
+  }
+}
+
+void voidCaughtForLoop1() {
+  for (int i = 0; i < size(); ++i) {
+    // no warning
+  }
+}
+
+void voidCaughtForLoop2() {
+  for (short i = 0; i < size(); ++i) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'unsigned long' [bugprone-too-small-loop-variable]
+  }
+}
Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- -- --target=x86_64-linux
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [{key: bugprone-too-small-loop-variable.MagnitudeBitsUpperLimit, \
+// RUN:               value: 1024}]}" \
+// RUN:   -- --target=x86_64-linux
 
 long size() { return 294967296l; }
 
Index: clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -27,6 +27,17 @@
 static constexpr llvm::StringLiteral LoopIncrementName =
     llvm::StringLiteral("loopIncrement");
 
+TooSmallLoopVariableCheck::TooSmallLoopVariableCheck(StringRef Name,
+                                                     ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      MagnitudeBitsUpperLimit(Options.get<unsigned>(
+          "MagnitudeBitsUpperLimit", 16)) {}
+
+void TooSmallLoopVariableCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "MagnitudeBitsUpperLimit", MagnitudeBitsUpperLimit);
+}
+
 /// \brief The matcher for loops with suspicious integer loop variable.
 ///
 /// In this general example, assuming 'j' and 'k' are of integral type:
@@ -84,9 +95,9 @@
       this);
 }
 
-/// Returns the positive part of the integer width for an integer type.
-static unsigned calcPositiveBits(const ASTContext &Context,
-                                 const QualType &IntExprType) {
+/// Returns the magnitude bits of an integer type.
+static unsigned calcMagnitudeBits(const ASTContext &Context,
+                                  const QualType &IntExprType) {
   assert(IntExprType->isIntegerType());
 
   return IntExprType->isUnsignedIntegerType()
@@ -94,13 +105,13 @@
              : Context.getIntWidth(IntExprType) - 1;
 }
 
-/// \brief Calculate the upper bound expression's positive bits, but ignore
+/// \brief Calculate the upper bound expression's magnitude bits, but ignore
 /// constant like values to reduce false positives.
-static unsigned calcUpperBoundPositiveBits(const ASTContext &Context,
-                                           const Expr *UpperBound,
-                                           const QualType &UpperBoundType) {
+static unsigned calcUpperBoundMagnitudeBits(const ASTContext &Context,
+                                            const Expr *UpperBound,
+                                            const QualType &UpperBoundType) {
   // Ignore casting caused by constant values inside a binary operator.
-  // We are interested in variable values' positive bits.
+  // We are interested in variable values' magnitude bits.
   if (const auto *BinOperator = dyn_cast<BinaryOperator>(UpperBound)) {
     const Expr *RHSE = BinOperator->getRHS()->IgnoreParenImpCasts();
     const Expr *LHSE = BinOperator->getLHS()->IgnoreParenImpCasts();
@@ -122,15 +133,15 @@
     if (RHSEIsConstantValue && LHSEIsConstantValue)
       return 0;
     if (RHSEIsConstantValue)
-      return calcPositiveBits(Context, LHSEType);
+      return calcMagnitudeBits(Context, LHSEType);
     if (LHSEIsConstantValue)
-      return calcPositiveBits(Context, RHSEType);
+      return calcMagnitudeBits(Context, RHSEType);
 
-    return std::max(calcPositiveBits(Context, LHSEType),
-                    calcPositiveBits(Context, RHSEType));
+    return std::max(calcMagnitudeBits(Context, LHSEType),
+                    calcMagnitudeBits(Context, RHSEType));
   }
 
-  return calcPositiveBits(Context, UpperBoundType);
+  return calcMagnitudeBits(Context, UpperBoundType);
 }
 
 void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) {
@@ -149,14 +160,17 @@
 
   ASTContext &Context = *Result.Context;
 
-  unsigned LoopVarPosBits = calcPositiveBits(Context, LoopVarType);
-  unsigned UpperBoundPosBits =
-      calcUpperBoundPositiveBits(Context, UpperBound, UpperBoundType);
+  unsigned LoopVarMagnitudeBits = calcMagnitudeBits(Context, LoopVarType);
+  unsigned UpperBoundMagnitudeBits =
+      calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType);
+
+  if (UpperBoundMagnitudeBits == 0)
+    return;
 
-  if (UpperBoundPosBits == 0)
+  if (LoopVarMagnitudeBits > MagnitudeBitsUpperLimit)
     return;
 
-  if (LoopVarPosBits < UpperBoundPosBits)
+  if (LoopVarMagnitudeBits < UpperBoundMagnitudeBits)
     diag(LoopVar->getBeginLoc(), "loop variable has narrower type %0 than "
                                  "iteration's upper bound %1")
         << LoopVarType << UpperBoundType;
Index: clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
+++ clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
@@ -29,10 +29,14 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-too-small-loop-variable.html
 class TooSmallLoopVariableCheck : public ClangTidyCheck {
 public:
-  TooSmallLoopVariableCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  TooSmallLoopVariableCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const unsigned MagnitudeBitsUpperLimit;
 };
 
 } // namespace bugprone
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -119,6 +119,12 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`bugprone-too-small-loop-variable
+  <clang-tidy/checks/bugprone-too-small-loop-variable>` now supports
+  `MagnitudeBitsUpperLimit` option. The default value was set to 16,
+  which greatly reduces warnings related to loops which are unlikely to
+  cause an actual functional bug.
+
 - The :doc:`google-runtime-int <clang-tidy/checks/google-runtime-int>`
   check has been disabled in Objective-C++.
 
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
@@ -27,3 +27,20 @@
 
 This algorithm works for small amount of objects, but will lead to freeze for a
 a larger user input.
+
+.. option:: MagnitudeBitsUpperLimit
+
+  Upper limit for the magnitude bits of the loop variable. If it's set the check
+  filters out those catches in which the loop variable's type has more magnitude
+  bits as the specified upper limit. The default value is 16.
+  For example, if the user sets this option to 31 (bits), then a 32-bit ``unsigend int``
+  is ignored by the check, however a 32-bit ``int`` is not (A 32-bit ``signed int``
+  has 31 magnitude bits).
+
+.. code-block:: c++
+
+  int main() {
+    long size = 294967296l;
+    for (unsigned i = 0; i < size; ++i) {} // no warning with MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
+    for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit = 31 on a system where int is 32-bit
+  }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to