ztamas created this revision.
Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
Herald added a project: clang.
The bugprone-too-small-loop-variable check often catches loop variables which
can represent "big enough" values, so we don't actually need to worry about
that this variable will overflow in a loop when the code iterates through a
container. For example a 32 bit signed integer type's maximum value is 2 147
483 647 and a container's size won't reach this maximum value in most of the
cases.
So the idea of this option to allow the user to specify an upper limit (using
magnitude bit of the integer type) to filter out those catches which are not
interesting for the user, so he/she can focus on the more risky integer
incompatibilities.
Next to the option I replaced the term "positive bits" to "magnitude bits"
which seems a better naming both in the code and in the name of the new option.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D59870
Files:
clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tidy/bugprone/TooSmallLoopVariableCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
Index: test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: bugprone-too-small-loop-variable.MagnitudeBitsUpperLimit, \
+// RUN: value: 31}]}" \
+// RUN: -- --target=x86_64-linux
+
+unsigned long size() { return 294967296l; }
+
+void voidFilteredOutForLoop1() {
+ for (long i = 0; i < size(); ++i) {
+ // no warning
+ }
+}
+
+void voidFilteredOutForLoop2() {
+ for (unsigned i = 0; i < size(); ++i) {
+ // no warning
+ }
+}
+
+void voidCaughtForLoop1() {
+ for (int i = 0; i < size(); ++i) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'unsigned long' [bugprone-too-small-loop-variable]
+ }
+}
+
+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: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
===================================================================
--- docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
+++ 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
+ bit as the specified upper limit.
+ For example, if the user sets this attribute to 31, 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
+ for (int i = 0; i < size; ++i) {} // warning
+ }
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -119,6 +119,10 @@
`CommentUserDefiniedLiterals`, `CommentStringLiterals`,
`CommentCharacterLiterals` & `CommentNullPtrs` options.
+- The :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-tidy/bugprone/TooSmallLoopVariableCheck.h
===================================================================
--- clang-tidy/bugprone/TooSmallLoopVariableCheck.h
+++ 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-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===================================================================
--- clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ 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", std::numeric_limits<unsigned>::max())) {}
+
+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
