This revision was automatically updated to reflect the committed changes.
Closed by commit rGc7dac52203a0: [clang-tidy] Improved too-small-loop-variable
with bit-field support (authored by PiotrZSL).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142587/new/
https://reviews.llvm.org/D142587
Files:
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -46,6 +46,16 @@
}
}
+void voidBadForLoop7() {
+ struct Int {
+ int value;
+ } i;
+
+ for (i.value = 0; i.value < size(); ++i.value) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
void voidForLoopUnsignedBound() {
unsigned size = 3147483647;
for (int i = 0; i < size; ++i) {
@@ -253,3 +263,113 @@
for (short i = 0; i < size; ++i) { // no warning
}
}
+
+// Should detect proper size of upper bound bitfield
+void voidForLoopWithBitfieldOnUpperBound() {
+ struct StructWithBitField {
+ unsigned bitfield : 5;
+ } value = {};
+
+ for(unsigned char i = 0U; i < value.bitfield; ++i) { // no warning
+ }
+}
+
+// Should detect proper size of loop variable bitfield
+void voidForLoopWithBitfieldOnLoopVar() {
+ struct StructWithBitField {
+ unsigned bitfield : 9;
+ } value = {};
+
+ unsigned char upperLimit = 100U;
+
+ for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+ }
+}
+
+// Should detect proper size of loop variable and upper bound
+void voidForLoopWithBitfieldOnLoopVarAndUpperBound() {
+ struct StructWithBitField {
+ unsigned var : 5, limit : 4;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ }
+}
+
+// Should detect proper size of loop variable and upper bound on integers
+void voidForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+ struct StructWithBitField {
+ unsigned var : 5;
+ int limit : 6;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ }
+}
+
+void badForLoopWithBitfieldOnUpperBound() {
+ struct StructWithBitField {
+ unsigned bitfield : 9;
+ } value = {};
+
+ for(unsigned char i = 0U; i < value.bitfield; ++i) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower type 'unsigned char' than iteration's upper bound 'unsigned int:9' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVar() {
+ struct StructWithBitField {
+ unsigned bitfield : 7;
+ } value = {};
+
+ unsigned char upperLimit = 100U;
+
+ for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'unsigned int:7' than iteration's upper bound 'unsigned char' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBound() {
+ struct StructWithBitField {
+ unsigned var : 5, limit : 6;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarOnIntAndUpperBound() {
+ struct StructWithBitField {
+ int var : 5;
+ unsigned limit : 5;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'int:5' than iteration's upper bound 'unsigned int:5' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+ struct StructWithBitField {
+ unsigned var : 5;
+ int limit : 7;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'int:7' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
+ struct StructWithBitField {
+ unsigned var : 5, limit : 6;
+ } value = {};
+
+ StructWithBitField* ptr = &value;
+
+ for(ptr->var = 0U; ptr->var < ptr->limit; ++ptr->var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+ }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,10 @@
``std::array`` objects to default constructed ones. The behavior for this and
other relevant classes can now be configured with a new option.
+- Improved :doc:`bugprone-too-small-loop-variable
+ <clang-tidy/checks/bugprone/too-small-loop-variable>` check. Basic support
+ for bit-field and integer members as a loop variable or upper limit were added.
+
Removed checks
^^^^^^^^^^^^^^
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
@@ -25,6 +25,19 @@
static constexpr llvm::StringLiteral LoopIncrementName =
llvm::StringLiteral("loopIncrement");
+namespace {
+
+struct MagnitudeBits {
+ unsigned WidthWithoutSignBit = 0U;
+ unsigned BitFieldWidth = 0U;
+
+ bool operator<(const MagnitudeBits &Other) const noexcept {
+ return WidthWithoutSignBit < Other.WidthWithoutSignBit;
+ }
+};
+
+} // namespace
+
TooSmallLoopVariableCheck::TooSmallLoopVariableCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -50,8 +63,9 @@
///
void TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
StatementMatcher LoopVarMatcher =
- expr(
- ignoringParenImpCasts(declRefExpr(to(varDecl(hasType(isInteger()))))))
+ expr(ignoringParenImpCasts(
+ anyOf(declRefExpr(to(varDecl(hasType(isInteger())))),
+ memberExpr(member(fieldDecl(hasType(isInteger())))))))
.bind(LoopVarName);
// We need to catch only those comparisons which contain any integer cast.
@@ -93,20 +107,27 @@
}
/// Returns the magnitude bits of an integer type.
-static unsigned calcMagnitudeBits(const ASTContext &Context,
- const QualType &IntExprType) {
+static MagnitudeBits calcMagnitudeBits(const ASTContext &Context,
+ const QualType &IntExprType,
+ const Expr *IntExpr) {
assert(IntExprType->isIntegerType());
- return IntExprType->isUnsignedIntegerType()
- ? Context.getIntWidth(IntExprType)
- : Context.getIntWidth(IntExprType) - 1;
+ unsigned SignedBits = IntExprType->isUnsignedIntegerType() ? 0U : 1U;
+
+ if (const auto *BitField = IntExpr->getSourceBitField()) {
+ unsigned BitFieldWidth = BitField->getBitWidthValue(Context);
+ return {BitFieldWidth - SignedBits, BitFieldWidth};
+ }
+
+ unsigned IntWidth = Context.getIntWidth(IntExprType);
+ return {IntWidth - SignedBits, 0U};
}
/// Calculate the upper bound expression's magnitude bits, but ignore
/// constant like values to reduce false positives.
-static unsigned calcUpperBoundMagnitudeBits(const ASTContext &Context,
- const Expr *UpperBound,
- const QualType &UpperBoundType) {
+static MagnitudeBits
+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' magnitude bits.
if (const auto *BinOperator = dyn_cast<BinaryOperator>(UpperBound)) {
@@ -117,7 +138,7 @@
QualType LHSEType = LHSE->getType();
if (!RHSEType->isIntegerType() || !LHSEType->isIntegerType())
- return 0;
+ return {};
bool RHSEIsConstantValue = RHSEType->isEnumeralType() ||
RHSEType.isConstQualified() ||
@@ -128,17 +149,28 @@
// Avoid false positives produced by two constant values.
if (RHSEIsConstantValue && LHSEIsConstantValue)
- return 0;
+ return {};
if (RHSEIsConstantValue)
- return calcMagnitudeBits(Context, LHSEType);
+ return calcMagnitudeBits(Context, LHSEType, LHSE);
if (LHSEIsConstantValue)
- return calcMagnitudeBits(Context, RHSEType);
+ return calcMagnitudeBits(Context, RHSEType, RHSE);
- return std::max(calcMagnitudeBits(Context, LHSEType),
- calcMagnitudeBits(Context, RHSEType));
+ return std::max(calcMagnitudeBits(Context, LHSEType, LHSE),
+ calcMagnitudeBits(Context, RHSEType, RHSE));
}
- return calcMagnitudeBits(Context, UpperBoundType);
+ return calcMagnitudeBits(Context, UpperBoundType, UpperBound);
+}
+
+static std::string formatIntegralType(const QualType &Type,
+ const MagnitudeBits &Info) {
+ std::string Name = Type.getAsString();
+ if (!Info.BitFieldWidth)
+ return Name;
+
+ Name += ':';
+ Name += std::to_string(Info.BitFieldWidth);
+ return Name;
}
void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) {
@@ -152,25 +184,26 @@
if (LoopVar->getType() != LoopIncrement->getType())
return;
- QualType LoopVarType = LoopVar->getType();
- QualType UpperBoundType = UpperBound->getType();
+ const QualType LoopVarType = LoopVar->getType();
+ const QualType UpperBoundType = UpperBound->getType();
ASTContext &Context = *Result.Context;
- unsigned LoopVarMagnitudeBits = calcMagnitudeBits(Context, LoopVarType);
- unsigned UpperBoundMagnitudeBits =
+ const MagnitudeBits LoopVarMagnitudeBits =
+ calcMagnitudeBits(Context, LoopVarType, LoopVar);
+ const MagnitudeBits UpperBoundMagnitudeBits =
calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType);
- if (UpperBoundMagnitudeBits == 0)
- return;
-
- if (LoopVarMagnitudeBits > MagnitudeBitsUpperLimit)
+ if ((0U == UpperBoundMagnitudeBits.WidthWithoutSignBit) ||
+ (LoopVarMagnitudeBits.WidthWithoutSignBit > MagnitudeBitsUpperLimit) ||
+ (LoopVarMagnitudeBits.WidthWithoutSignBit >=
+ UpperBoundMagnitudeBits.WidthWithoutSignBit))
return;
- if (LoopVarMagnitudeBits < UpperBoundMagnitudeBits)
- diag(LoopVar->getBeginLoc(), "loop variable has narrower type %0 than "
- "iteration's upper bound %1")
- << LoopVarType << UpperBoundType;
+ diag(LoopVar->getBeginLoc(),
+ "loop variable has narrower type '%0' than iteration's upper bound '%1'")
+ << formatIntegralType(LoopVarType, LoopVarMagnitudeBits)
+ << formatIntegralType(UpperBoundType, UpperBoundMagnitudeBits);
}
} // namespace clang::tidy::bugprone
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
