aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Mostly just nits for the check, otherwise this LGTM.
================
Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:65
+ }
+ if (isa<WhileStmt>(Loop) || isa<DoStmt>(Loop)) {
+ diag(Loop->getBeginLoc(),
----------------
================
Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:83-84
+UnrollLoopsCheck::unrollType(const Stmt *Statement, ASTContext *Context) {
+ const clang::DynTypedNodeList Parents =
Context->getParents<Stmt>(*Statement);
+ for (const clang::DynTypedNode &Parent : Parents) {
+ const auto *ParentStmt = Parent.get<AttributedStmt>();
----------------
Pretty sure you don't need to use a qualified name here.
================
Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:116-120
+ if (isa<CXXForRangeStmt>(Statement)) {
+ if (CXXLoopBound)
+ return true;
+ return false;
+ }
----------------
================
Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:123
+ // unrolling for these.
+ if (isa<WhileStmt>(Statement) || isa<DoStmt>(Statement))
+ return false;
----------------
================
Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:176-177
+ if (isa<CXXForRangeStmt>(Statement)) {
+ const auto *LoopBoundExpr = dyn_cast<Expr>(CXXLoopBound);
+ return exprHasLargeNumIterations(LoopBoundExpr, Context);
+ }
----------------
The cast isn't necessary (`IntergerLiteral` is already an `Expr`). You should
probably assert that `CXXLoopBound` isn't null here though.
================
Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:180-181
+ const auto *ForLoop = dyn_cast<ForStmt>(Statement);
+ if (!ForLoop)
+ llvm_unreachable("Unknown loop");
+ const Stmt *Initializer = ForLoop->getInit();
----------------
================
Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:196-197
+ }
+ if (!isa<BinaryOperator>(Conditional))
+ llvm_unreachable("Conditional is not a binary operator");
+ int EndValue;
----------------
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72235/new/
https://reviews.llvm.org/D72235
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
