[PATCH] D72239: [clang-tidy] new opencl recursion not supported check

[Eugene Zelenko via Phabricator via cfe-commits]

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/opencl/RecursionNotSupportedCheck.cpp:28
+void RecursionNotSupportedCheck::check(const MatchFinder::MatchResult &Result) {
+  auto MatchedFunDecl = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
+  auto MatchedFunCall = Result.Nodes.getNodeAs<DeclRefExpr>("functionCall");
----------------
const auto *


================
Comment at: clang-tidy/opencl/RecursionNotSupportedCheck.cpp:29
+  auto MatchedFunDecl = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
+  auto MatchedFunCall = Result.Nodes.getNodeAs<DeclRefExpr>("functionCall");
+
----------------
const auto *


================
Comment at: clang-tidy/opencl/RecursionNotSupportedCheck.cpp:53
+  }
+  for (std::pair<const std::string, SourceRange> &Loc : Locations) {
+    if (SM->isPointWithin(FunCall->getLocation(), Loc.second.getBegin(),
----------------
const auto &, because Loc is iterator.


================
Comment at: clang-tidy/opencl/RecursionNotSupportedCheck.cpp:76
+  if (Depth == MaxRecursionDepth) {
+    return "";
+  }
----------------
{}


================
Comment at: clang-tidy/opencl/RecursionNotSupportedCheck.cpp:78
+  }
+  for (std::pair<SourceLocation, std::string> &Caller : Callers[CallerName]) {
+    if (Caller.second.compare(FunCallName) == 0) {
----------------
const auto &, because Caller is iterator.


================
Comment at: clang-tidy/opencl/RecursionNotSupportedCheck.cpp:108
+               << FilePath << ":" << LineNum << ":" << ColNum;
+  std::string StringPath = StringStream.str();
+  return StringPath;
----------------
Are two steps and temporary variable are really necessary?


================
Comment at: docs/ReleaseNotes.rst:131
+
+  Finds recursive function calls, which are not supported by OpenCL. 
+
----------------
Please synchronize with first statement in documentation.


================
Comment at: docs/clang-tidy/checks/opencl-recursion-not-supported.rst:38
+   Defines the maximum depth of function calls through which the lint check will
+   attempt to find instances of recursion. Default is 100.
----------------
Please highlight 100 with single back-quotes.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72239/new/

https://reviews.llvm.org/D72239