ztamas updated this revision to Diff 193017.
ztamas added a comment.
Make 16 the default value of MagnitudeBitsUpperLimit option.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59870/new/
https://reviews.llvm.org/D59870
Files:
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
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/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable.cpp
Index: clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable.cpp
+++ clang-tools-extra/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/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/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/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
+++ clang-tools-extra/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 magnitue 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
+ }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,10 @@
`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 :doc:`google-runtime-int <clang-tidy/checks/google-runtime-int>`
check has been disabled in Objective-C++.
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
+++ clang-tools-extra/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/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ clang-tools-extra/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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
