[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