[clang-tools-extra] r358356 - [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable
Tamas Zolnai via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 14 05:47:48 PDT 2019
Author: ztamas
Date: Sun Apr 14 05:47:48 2019
New Revision: 358356
URL: http://llvm.org/viewvc/llvm-project?rev=358356&view=rev
Log:
[clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable
Summary:
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.
Reviewers: JonasToth, alexfh, aaron.ballman, hokein
Reviewed By: JonasToth
Subscribers: Eugene.Zelenko, xazax.hun, jdoerfert, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59870
Added:
clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
Modified:
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.cpp
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp?rev=358356&r1=358355&r2=358356&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp Sun Apr 14 05:47:48 2019
@@ -27,6 +27,17 @@ static constexpr llvm::StringLiteral Loo
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 @@ void TooSmallLoopVariableCheck::register
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 @@ static unsigned calcPositiveBits(const A
: 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 @@ static unsigned calcUpperBoundPositiveBi
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 @@ void TooSmallLoopVariableCheck::check(co
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;
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h?rev=358356&r1=358355&r2=358356&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/TooSmallLoopVariableCheck.h Sun Apr 14 05:47:48 2019
@@ -29,10 +29,14 @@ namespace bugprone {
/// 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
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=358356&r1=358355&r2=358356&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Sun Apr 14 05:47:48 2019
@@ -119,6 +119,12 @@ Improvements to clang-tidy
`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++.
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst?rev=358356&r1=358355&r2=358356&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst Sun Apr 14 05:47:48 2019
@@ -27,3 +27,20 @@ In a real use case size means a containe
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
+ }
Added: clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp?rev=358356&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp Sun Apr 14 05:47:48 2019
@@ -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]
+ }
+}
Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp?rev=358356&r1=358355&r2=358356&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-too-small-loop-variable.cpp Sun Apr 14 05:47:48 2019
@@ -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; }
More information about the cfe-commits
mailing list