diff --git a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp @@ -9,6 +9,7 @@ #include "TooSmallLoopVariableCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include using namespace clang::ast_matchers; @@ -50,8 +51,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 +95,29 @@ } /// Returns the magnitude bits of an integer type. -static unsigned calcMagnitudeBits(const ASTContext &Context, - const QualType &IntExprType) { +static std::pair +calcMagnitudeBits(const ASTContext &Context, const QualType &IntExprType, + const Expr *IntExpr) { assert(IntExprType->isIntegerType()); - return IntExprType->isUnsignedIntegerType() - ? Context.getIntWidth(IntExprType) - : Context.getIntWidth(IntExprType) - 1; + unsigned IntWidth = 0U; + unsigned BitFieldWidth = 0U; + + if (const auto *BitField = IntExpr->getSourceBitField()) + BitFieldWidth = BitField->getBitWidthValue(Context); + else + IntWidth = Context.getIntWidth(IntExprType); + + return {std::max(BitFieldWidth, IntWidth) - + (IntExprType->isUnsignedIntegerType() ? 0U : 1U), + BitFieldWidth}; } /// 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 std::pair +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(UpperBound)) { @@ -117,7 +128,7 @@ QualType LHSEType = LHSE->getType(); if (!RHSEType->isIntegerType() || !LHSEType->isIntegerType()) - return 0; + return {}; bool RHSEIsConstantValue = RHSEType->isEnumeralType() || RHSEType.isConstQualified() || @@ -128,17 +139,17 @@ // 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); } void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) { @@ -157,20 +168,38 @@ ASTContext &Context = *Result.Context; - unsigned LoopVarMagnitudeBits = calcMagnitudeBits(Context, LoopVarType); - unsigned UpperBoundMagnitudeBits = + std::pair LoopVarMagnitudeBits = + calcMagnitudeBits(Context, LoopVarType, LoopVar); + std::pair UpperBoundMagnitudeBits = calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType); - if (UpperBoundMagnitudeBits == 0) + if (UpperBoundMagnitudeBits.first == 0) + return; + + if (LoopVarMagnitudeBits.first > MagnitudeBitsUpperLimit) return; - if (LoopVarMagnitudeBits > MagnitudeBitsUpperLimit) + if (LoopVarMagnitudeBits.first >= UpperBoundMagnitudeBits.first) return; - if (LoopVarMagnitudeBits < UpperBoundMagnitudeBits) - diag(LoopVar->getBeginLoc(), "loop variable has narrower type %0 than " - "iteration's upper bound %1") - << LoopVarType << UpperBoundType; + auto Msg = + diag(LoopVar->getBeginLoc(), "loop variable has narrower type '%0%1%2' " + "than iteration's upper bound '%3%4%5'"); + Msg << LoopVarType.getAsString(); + if (LoopVarMagnitudeBits.second) { + Msg << ":" << LoopVarMagnitudeBits.second; + } else { + Msg << "" + << ""; + } + + Msg << UpperBoundType.getAsString(); + if (UpperBoundMagnitudeBits.second) { + Msg << ":" << UpperBoundMagnitudeBits.second; + } else { + Msg << "" + << ""; + } } } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -103,6 +103,10 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`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 --- 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 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] + } +} +