[PATCH] D72235: [clang-tidy] new altera unroll loops check
Eugene Zelenko via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 5 17:39:35 PST 2020
Eugene.Zelenko added a comment.
I think will be good idea to separate module code in own review or refer to previous one of previous reviews as dependency.
================
Comment at: clang-tidy/altera/UnrollLoopsCheck.cpp:31
+void UnrollLoopsCheck::check(const MatchFinder::MatchResult &Result) {
+ const Stmt *MatchedLoop = Result.Nodes.getNodeAs<Stmt>("loop");
+ const ASTContext *Context = Result.Context;
----------------
Could be const auto *, because type is spelled in same sentence. See modernize-use-auto.
================
Comment at: clang-tidy/altera/UnrollLoopsCheck.cpp:69
+ for (const Attr* Attribute: ParentStmt->getAttrs()) {
+ const LoopHintAttr *LoopHint;
+ if ((LoopHint = static_cast<const LoopHintAttr *>(Attribute))) {
----------------
May be merge declaration with initialization and use const auto *?
================
Comment at: clang-tidy/altera/UnrollLoopsCheck.cpp:96
+ if (isa<BinaryOperator>(Conditional)) {
+ const BinaryOperator *BinaryOp =
+ static_cast<const BinaryOperator *>(Conditional);
----------------
Could be const auto *, because type is spelled in same sentence.
================
Comment at: clang-tidy/altera/UnrollLoopsCheck.cpp:113
+ if (isa<ForStmt>(Statement)) {
+ const ForStmt *ForLoop = static_cast<const ForStmt *>(Statement);
+ Conditional = ForLoop->getCond();
----------------
Could be const auto *, because type is spelled in same sentence.
================
Comment at: clang-tidy/altera/UnrollLoopsCheck.cpp:117
+ if (isa<WhileStmt>(Statement)) {
+ const WhileStmt *WhileLoop = static_cast<const WhileStmt *>(Statement);
+ Conditional = WhileLoop->getCond();
----------------
Could be const auto *, because type is spelled in same sentence.
================
Comment at: clang-tidy/altera/UnrollLoopsCheck.cpp:121
+ if (isa<DoStmt>(Statement)) {
+ const DoStmt *DoWhileLoop = static_cast<const DoStmt *>(Statement);
+ Conditional = DoWhileLoop->getCond();
----------------
Could be const auto *, because type is spelled in same sentence.
================
Comment at: clang-tidy/altera/UnrollLoopsCheck.cpp:134
+ if (isa<BinaryOperator>(Conditional)) {
+ const BinaryOperator *BinaryOp =
+ static_cast<const BinaryOperator *>(Conditional);
----------------
Could be const auto *, because type is spelled in same sentence.
================
Comment at: docs/ReleaseNotes.rst:79
+
+ Finds inner loops that have not been unrolled, as well as fully unrolled loops
+ with unknown loops bounds or a large number of iterations.
----------------
Please synchronize with first statement in documentation.
================
Comment at: docs/clang-tidy/checks/altera-unroll-loops.rst:68
+
+ Defines the maximum number of loop iterations that a fully unrolled loop
+ can have.
----------------
Please document default value.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72235/new/
https://reviews.llvm.org/D72235
More information about the cfe-commits
mailing list