lewmpk updated this revision to Diff 189078.
lewmpk marked an inline comment as done.
lewmpk added a comment.
added example in docs and explicitly specified types for some variables
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58818/new/
https://reviews.llvm.org/D58818
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp
clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
Index: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
@@ -0,0 +1,194 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t
+
+// Mock implementation of std::mutex
+namespace std {
+struct mutex {
+ void lock();
+ void try_lock();
+ void unlock();
+};
+typedef mutex recursive_mutex;
+} // namespace std
+
+void warn_me1_simple() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me2_nested() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+ m.unlock();
+ }
+ m.unlock();
+}
+
+void warn_me3_nested_extra() {
+ std::mutex m1;
+ std::mutex m2;
+ {
+ m1.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+ {
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use RAII
+ m2.unlock();
+ }
+ m1.unlock();
+ }
+}
+
+void warn_me4_multi_mutex() {
+ std::mutex m1;
+ std::mutex m2;
+
+ m1.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m1.unlock();
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m2.unlock();
+}
+
+void warn_me5_multi_mutex_extra() {
+ std::mutex m1;
+ std::mutex m2;
+
+ m1.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m1.unlock();
+ m2.unlock();
+}
+
+void warn_me6() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+ m.try_lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me7_loop() {
+ std::mutex m;
+ for (auto i = 0; i < 3; i++) {
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+ m.unlock();
+ }
+}
+
+template <typename M>
+void attempt(M m) {
+ m.lock();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me8_template_func() {
+ std::mutex m;
+ attempt(m);
+}
+
+// clang-format off
+#define ATTEMPT(m) {\
+ m.lock();\
+ m.unlock();\
+ }
+// clang-format on
+
+void warn_me9_macro() {
+ std::mutex m;
+ ATTEMPT(m);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use RAII
+}
+
+void warn_me10_inline() {
+ std::mutex m;
+ // clang-format off
+ m.lock(); m.unlock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ // clang-format on
+}
+
+void warn_me11_recursive_mutex() {
+ std::recursive_mutex m;
+ m.lock();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void ignore_me1_rev_order() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me2_diff_mtx() {
+ std::mutex m1;
+ std::mutex m2;
+ m1.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ m2.unlock();
+}
+
+void ignore_me3() {
+ std::recursive_mutex m1;
+ std::recursive_mutex m2;
+ m1.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ m2.unlock();
+}
+
+void ignore_me4() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me5_inline() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me6_seperate_scopes() {
+ std::mutex m;
+ {
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ }
+ {
+ m.unlock();
+ }
+}
+
+void ignore_me7_seperate_scopes_nested() {
+ std::mutex m;
+ {
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ {
+ m.unlock();
+ }
+ }
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+ cppcoreguidelines-use-raii-locks
fuchsia-default-arguments
fuchsia-header-anon-namespaces (redirects to google-build-namespaces) <fuchsia-header-anon-namespaces>
fuchsia-multiple-inheritance
Index: docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - cppcoreguidelines-use-raii-locks
+
+cppcoreguidelines-use-raii-locks
+================================
+
+Checks for explicit usage of ``std::mutex`` functions ``lock()``,
+``try_lock()`` and ``unlock()`` which is error-prone and should be avoided.
+It's recommended to use RAII-style locking mechanisms such as
+``std::lock_guard`` or ``std::unique_lock``.
+See `C++ Core Guidelines <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rconc-raii>`_.
+
+.. code-block:: c++
+
+ std::mutex m;
+ // Warns the following lock() call
+ m.lock()
+
+ m.unlock()
+
+Options
+-------
+
+.. option:: LockableTypes
+
+ Semicolon-separated list of fully qualified names of types that support
+ locking and unlocking a mutex.
+ Defaults to:
+ `::std::mutex;::std::recursive_mutex;::std::timed_mutex;
+ ::std::recursive_timed_mutex;std::shared_mutex;
+ ::std::unique_lock;::std::shared_lock;
+ ::boost::mutex;::boost::recursive_mutex;::boost::timed_mutex;
+ ::boost::recursive_timed_mutex;::boost::shared_mutex;
+ ::boost::unique_lock;::boost::shared_lock`.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
in the Time domain instead of the numeric domain.
+- New :doc:`cppcoreguidelines-use-raii-locks
+ <clang-tidy/checks/cppcoreguidelines-use-raii-locks>` check.
+
+ Checks for explicit usage of ``std::mutex`` functions ``lock()``,
+ ``try_lock()`` and ``unlock()`` which is error-prone and should be avoided.
+
- New :doc:`google-readability-avoid-underscore-in-googletest-name
<clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name>`
check.
Index: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h
@@ -0,0 +1,48 @@
+//===--- UseRaiiLocksCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_USERAIILOCKSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_USERAIILOCKSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Check for instances of explicit std::mutex lock() and unlock() calls
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-use-raii-locks.html
+class UseRaiiLocksCheck : public ClangTidyCheck {
+public:
+ UseRaiiLocksCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ // clang-format off
+ LockableTypes(Options.get(
+ "LockableTypes",
+ "::std::mutex;::std::recursive_mutex;::std::timed_mutex;"
+ "::std::recursive_timed_mutex;std::shared_mutex;"
+ "::std::unique_lock;::std::shared_lock;"
+ "::boost::mutex;::boost::recursive_mutex;::boost::timed_mutex;"
+ "::boost::recursive_timed_mutex;::boost::shared_mutex;"
+ "::boost::unique_lock;::boost::shared_lock")) {}
+ // clang-format on
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const std::string LockableTypes;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_USERAIILOCKSCHECK_H
Index: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp
@@ -0,0 +1,105 @@
+//===--- UseRaiiLocksCheck.cpp - clang-tidy -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseRaiiLocksCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+static Matcher<RecordDecl> hasAnyListedName(const std::string &TypeNames) {
+ std::vector<std::string> NameList =
+ utils::options::parseStringList(TypeNames);
+ return hasAnyName(std::vector<StringRef>(NameList.begin(), NameList.end()));
+}
+
+static DeclRefExpr *findDeclRefExpr(const CXXMemberCallExpr *MemberCallExpr) {
+ Expr *ObjectExpr = MemberCallExpr->getImplicitObjectArgument();
+ if (auto *ObjectCast = dyn_cast<ImplicitCastExpr>(ObjectExpr)) {
+ ObjectExpr = ObjectCast->getSubExpr();
+ }
+ return dyn_cast<DeclRefExpr>(ObjectExpr);
+}
+
+void UseRaiiLocksCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "LockableTypes", LockableTypes);
+}
+
+void UseRaiiLocksCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ auto MutexType = type(hasUnqualifiedDesugaredType(recordType(
+ hasDeclaration(cxxRecordDecl(hasAnyListedName(LockableTypes))))));
+ // Match expressions of type mutex or mutex pointer.
+ auto MutexExpr =
+ expr(anyOf(hasType(MutexType), hasType(qualType(pointsTo(MutexType)))));
+
+ // Match a call to mutex lock() or try_lock().
+ auto MutexLockCallExpr =
+ cxxMemberCallExpr(on(MutexExpr), callee(memberExpr()),
+ callee(cxxMethodDecl(hasAnyName("lock", "try_lock"))));
+
+ // Match a call to mutex unlock().
+ auto MutexUnlockCallExpr =
+ cxxMemberCallExpr(on(MutexExpr), callee(memberExpr()),
+ callee(cxxMethodDecl(hasAnyName("unlock"))));
+
+ // Match a scope with a calls to both lock() and unlock().
+ auto ScopedProblem =
+ compoundStmt(forEach(MutexLockCallExpr.bind("lock")),
+ forEach(MutexUnlockCallExpr.bind("unlock")));
+ Finder->addMatcher(ScopedProblem, this);
+}
+
+void UseRaiiLocksCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *LockCallExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("lock");
+ const auto *UnlockCallExpr =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("unlock");
+
+ const auto *LockDeclRef = findDeclRefExpr(LockCallExpr);
+ const auto *UnlockDeclRef = findDeclRefExpr(UnlockCallExpr);
+
+ assert(LockDeclRef && UnlockDeclRef);
+
+ StringRef LockObjectName = LockDeclRef->getFoundDecl()->getName();
+ StringRef UnlockObjectName = UnlockDeclRef->getFoundDecl()->getName();
+
+ // Ignore m.lock(); m2.unlock().
+ if (LockObjectName != UnlockObjectName)
+ return;
+
+ auto LockLocation =
+ Result.SourceManager->getPresumedLoc(LockCallExpr->getBeginLoc());
+ auto UnlockLocation =
+ Result.SourceManager->getPresumedLoc(UnlockCallExpr->getBeginLoc());
+
+ // Ignore unlock() before lock().
+ if (UnlockLocation.getLine() < LockLocation.getLine() ||
+ (UnlockLocation.getLine() == LockLocation.getLine() &&
+ UnlockLocation.getColumn() < LockLocation.getColumn()))
+ return;
+
+ diag(LockCallExpr->getBeginLoc(),
+ "use RAII-style locking mechanisms such as %0.")
+ << (getLangOpts().CPlusPlus11
+ ? "'std::lock_guard' "
+ "or 'std::unique_lock'"
+ : "'boost:lock_guard' or 'boost::unique_lock'");
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -32,6 +32,7 @@
#include "ProTypeVarargCheck.h"
#include "SlicingCheck.h"
#include "SpecialMemberFunctionsCheck.h"
+#include "UseRaiiLocksCheck.h"
namespace clang {
namespace tidy {
@@ -83,6 +84,8 @@
CheckFactories.registerCheck<SpecialMemberFunctionsCheck>(
"cppcoreguidelines-special-member-functions");
CheckFactories.registerCheck<SlicingCheck>("cppcoreguidelines-slicing");
+ CheckFactories.registerCheck<UseRaiiLocksCheck>(
+ "cppcoreguidelines-use-raii-locks");
CheckFactories.registerCheck<misc::UnconventionalAssignOperatorCheck>(
"cppcoreguidelines-c-copy-assignment-signature");
}
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -20,6 +20,7 @@
ProTypeVarargCheck.cpp
SlicingCheck.cpp
SpecialMemberFunctionsCheck.cpp
+ UseRaiiLocksCheck.cpp
LINK_LIBS
clangAST
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
