[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:29
+
+DeclRefExpr *findDeclRefExpr(const CXXMemberCallExpr *MemberCallExpr) {
+  auto *ObjectExpr = MemberCallExpr->getImplicitObjectArgument();
----------------
Please use static for functions instead of anonymous namespaces. See LLVM coding guidelines.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:30
+DeclRefExpr *findDeclRefExpr(const CXXMemberCallExpr *MemberCallExpr) {
+  auto *ObjectExpr = MemberCallExpr->getImplicitObjectArgument();
+  if (auto *ObjectCast = dyn_cast<ImplicitCastExpr>(ObjectExpr)) {
----------------
Please don't use auto where return type is not obvious.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:44
+void UseRaiiLocksCheck::registerMatchers(MatchFinder *Finder) {
+
+  if (!getLangOpts().CPlusPlus)
----------------
Unnecessary empty line.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:72
+void UseRaiiLocksCheck::check(const MatchFinder::MatchResult &Result) {
+
+  const auto *LockCallExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("lock");
----------------
Unnecessary empty line.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:77
+
+  const auto *LockDeclRef = findDeclRefExpr(LockCallExpr);
+  const auto *UnlockDeclRef = findDeclRefExpr(UnlockCallExpr);
----------------
Please don't use auto where return type is not obvious.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:78
+  const auto *LockDeclRef = findDeclRefExpr(LockCallExpr);
+  const auto *UnlockDeclRef = findDeclRefExpr(UnlockCallExpr);
+
----------------
Please don't use auto where return type is not obvious.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:82
+
+  auto LockObjectName = LockDeclRef->getFoundDecl()->getName();
+  auto UnlockObjectName = UnlockDeclRef->getFoundDecl()->getName();
----------------
Please don't use auto where return type is not obvious.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:83
+  auto LockObjectName = LockDeclRef->getFoundDecl()->getName();
+  auto UnlockObjectName = UnlockDeclRef->getFoundDecl()->getName();
+
----------------
Please don't use auto where return type is not obvious.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:89
+
+  auto LockLocation =
+      Result.SourceManager->getPresumedLoc(LockCallExpr->getBeginLoc());
----------------
Please don't use auto where return type is not obvious.


================
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:91
+      Result.SourceManager->getPresumedLoc(LockCallExpr->getBeginLoc());
+  auto UnlockLocation =
+      Result.SourceManager->getPresumedLoc(UnlockCallExpr->getBeginLoc());
----------------
Please don't use auto where return type is not obvious.


================
Comment at: docs/ReleaseNotes.rst:97
+
+  Checks for explicit usage of``std::mutex`` functions ``lock()``, 
+  ``try_lock()`` and ``unlock()``.
----------------
Missing space before ``std::mutex


================
Comment at: docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst:6
+
+The explicit use of ``std::mutex`` functions ``lock()``, ``try_lock()`` and 
+``unlock()`` is error prone and should be avoided. 
----------------
Please synchronize first statement with Release Notes.


================
Comment at: docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst:18
+   locking and unlocking a mutex.
+   Defaults to: ``::std::mutex;::std::recursive_mutex;::std::timed_mutex;
+   ::std::recursive_timed_mutex;::std::unique_lock``.
----------------
Please use single ` for options values.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58818