Eugene.Zelenko added inline comments.
================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:26 + + +/// \brief The matcher for loops with suspicious integer loop variable. ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:41 +void TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) { + + const StatementMatcher LoopVarMatcher = ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:85 + + +/// \brief Returns the positive part of the integer width for an integer type ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:88 +unsigned calcPositiveBits(const ASTContext &Context, const QualType &IntExprType) { + + assert(IntExprType->isIntegerType()); ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:96 + + +/// \brief Calculate the condition expression's positive bits, but ignore constant like values to reduce false positives ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:99 +unsigned calcCondExprPositiveBits(const ASTContext &Context, const Expr *CondExpr, const QualType &CondExprType) { + + unsigned CondExprPosBits = 0; ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:105 + // We are interested in variable values' positive bits + if(const auto *BinOperator = dyn_cast<BinaryOperator>(CondExpr)) { + const Expr *RHSE = BinOperator->getRHS()->IgnoreParenImpCasts(); ---------------- Please run Clang-format. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:108 + const Expr *LHSE = BinOperator->getLHS()->IgnoreParenImpCasts(); + + ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:114 + + + const QualType RHSEType = RHSE->getType(); ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:121 + + + if(RHSEType->isEnumeralType() || RHSEType.isConstQualified() || ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:123 + if(RHSEType->isEnumeralType() || RHSEType.isConstQualified() || + RHSE->getBeginLoc().isMacroID() || isa<IntegerLiteral>(RHSE)) { + ---------------- Braces are not needed. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:126 + CondExprPosBits = calcPositiveBits(Context, LHSEType); + + } ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:129 + else if (LHSEType->isEnumeralType() || LHSEType.isConstQualified() || + LHSE->getBeginLoc().isMacroID() || isa<IntegerLiteral>(LHSE)) { + ---------------- Braces are not needed. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:130 + LHSE->getBeginLoc().isMacroID() || isa<IntegerLiteral>(LHSE)) { + + CondExprPosBits = calcPositiveBits(Context, RHSEType); ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:132 + CondExprPosBits = calcPositiveBits(Context, RHSEType); + + } else { ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:133 + + } else { + ---------------- Braces are not needed. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:134 + } else { + + CondExprPosBits = std::max(calcPositiveBits(Context, LHSEType), ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:138 + } + } else { + ---------------- Braces are not needed. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:139 + } else { + + CondExprPosBits = calcPositiveBits(Context, CondExprType); ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:141 + CondExprPosBits = calcPositiveBits(Context, CondExprType); + + } ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:149 +void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) { + + const auto *LoopVar = Result.Nodes.getNodeAs<Expr>(loopVarName); ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:153 + const auto *LoopIncrement = Result.Nodes.getNodeAs<Expr>(loopIncrementName)->IgnoreParenImpCasts(); + + ---------------- Please remove unnecessary empty line. ================ Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:181 + if (LoopVarPosBits < CondExprPosBits) + { + diag(LoopVar->getBeginLoc(), "loop variable has a narrower type (%0) than " ---------------- Braces are not needed. ================ Comment at: docs/ReleaseNotes.rst:70 +- New :doc:`bugprone-too-small-loop-variable + <clang-tidy/checks/bugprone-too-small-loop-variable>` check. ---------------- Please sort list of new checks alphabetically. ================ Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:6 + +This check searches for those for loops which has a loop variable +with a "too small" type which means this type can't represent all values ---------------- Please synchronize with Release Notes. ================ Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:12 + int main() { + long size = 294967296l; + for (short i = 0; i < size; ++i) {} { ... } ---------------- Please insert empty line above. ================ Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:13 + long size = 294967296l; + for (short i = 0; i < size; ++i) {} { ... } + } ---------------- Braces are repeated twice. ================ Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:18 +values in the [0..size] interval. + + ---------------- Please remove unnecessary empty line. ================ Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:23 + .. code-block:: c++ + int doSometinhg(const std::vector& items) { + for (short i = 0; i < items.size(); ++i) {} { ... } ---------------- Please insert empty line above. ================ Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:24 + int doSometinhg(const std::vector& items) { + for (short i = 0; i < items.size(); ++i) {} { ... } + } ---------------- Braces are repeated twice. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits