[clang-tools-extra] c7dac52 - [clang-tidy] Improved too-small-loop-variable with bit-field support
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 26 06:54:51 PST 2023
Author: Piotr Zegar
Date: 2023-02-26T14:54:36Z
New Revision: c7dac52203a0c056bfcb35735e62c37a64e50bfa
URL: https://github.com/llvm/llvm-project/commit/c7dac52203a0c056bfcb35735e62c37a64e50bfa
DIFF: https://github.com/llvm/llvm-project/commit/c7dac52203a0c056bfcb35735e62c37a64e50bfa.diff
LOG: [clang-tidy] Improved too-small-loop-variable with bit-field support
Implemented support for bit-field members as a loop variable
or upper limit. Supporting also non bit-field integer members.
Fixes issues: https://github.com/llvm/llvm-project/issues/58614
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D142587
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 3da34fcb64e17..133fcabd78392 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -25,6 +25,19 @@ static constexpr llvm::StringLiteral LoopUpperBoundName =
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::storeOptions(
///
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 @@ void TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
}
/// 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 @@ static unsigned calcUpperBoundMagnitudeBits(const ASTContext &Context,
QualType LHSEType = LHSE->getType();
if (!RHSEType->isIntegerType() || !LHSEType->isIntegerType())
- return 0;
+ return {};
bool RHSEIsConstantValue = RHSEType->isEnumeralType() ||
RHSEType.isConstQualified() ||
@@ -128,17 +149,28 @@ static unsigned calcUpperBoundMagnitudeBits(const ASTContext &Context,
// 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 @@ void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) {
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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5979b9e6d592d..e5c46de76cfcc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,10 @@ Changes in existing checks
``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
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 5a633559484e1..1505dbfef519e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -46,6 +46,16 @@ void voidBadForLoop6() {
}
}
+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 @@ void voidForLoopWithBigConstBound() {
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]
+ }
+}
+
More information about the cfe-commits
mailing list