lewmpk updated this revision to Diff 188920.
lewmpk added a comment.
remove support for try_lock_for and try_lock_until
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,92 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t
+
+// Mock implementation of std::mutex
+namespace std {
+struct mutex {
+ void lock();
+ void unlock();
+};
+typedef mutex recursive_mutex;
+} // namespace std
+
+void warn_me1() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me2() {
+ 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() {
+ 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() {
+ 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() {
+ 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 ignore_me1() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me2() {
+ 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();
+}
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,19 @@
+.. title:: clang-tidy - cppcoreguidelines-use-raii-locks
+
+cppcoreguidelines-use-raii-locks
+================================
+
+The explicit use of ``std::mutex`` functions ``lock()``, ``try_lock()`` and
+``unlock()`` 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``.
+
+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::unique_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()``.
+
- 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,42 @@
+//===--- 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),
+ LockableTypes(Options.get(
+ "LockableTypes",
+ "::std::mutex;::std::recursive_mutex;::std::timed_mutex;::std::"
+ "recursive_timed_mutex;::std::unique_lock")) {}
+ 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,100 @@
+//===--- 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 {
+
+namespace {
+Matcher<RecordDecl> hasAnyListedName(const std::string &TypeNames) {
+ const std::vector<std::string> NameList =
+ utils::options::parseStringList(TypeNames);
+ return hasAnyName(std::vector<StringRef>(NameList.begin(), NameList.end()));
+}
+} // namespace
+
+void UseRaiiLocksCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "LockableTypes", LockableTypes);
+}
+
+void UseRaiiLocksCheck::registerMatchers(MatchFinder *Finder) {
+ // lock_guards require c++11 or later
+ if (!getLangOpts().CPlusPlus11)
+ return;
+
+ const auto MutexDecl = type(hasUnqualifiedDesugaredType(recordType(
+ hasDeclaration(cxxRecordDecl(hasAnyListedName(LockableTypes))))));
+ // Match expressions of type mutex or mutex pointer
+ const auto MutexExpr =
+ expr(anyOf(hasType(MutexDecl), hasType(qualType(pointsTo(MutexDecl)))));
+
+ // Match a call to mutex 'lock' or 'try_lock'
+ const auto MutexLockCallExpr = cxxMemberCallExpr(
+ on(MutexExpr), callee(memberExpr()),
+ callee(cxxMethodDecl(
+ hasAnyName("lock", "try_lock"))));
+
+ // Match a call to mutex 'unlock'
+ const auto MutexUnlockCallExpr =
+ cxxMemberCallExpr(on(MutexExpr), callee(memberExpr()),
+ callee(cxxMethodDecl(hasAnyName("unlock"))));
+
+ // Match a scope with a calls to both 'lock' and 'unlock'
+ const 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 LockObjectName = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ LockCallExpr->getImplicitObjectArgument()->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+ const auto UnlockObjectName = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ UnlockCallExpr->getImplicitObjectArgument()->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+
+ // Ignore m.lock(); m2.unlock()
+ if (LockObjectName != UnlockObjectName)
+ return;
+
+ const auto LockLineNum =
+ Result.SourceManager->getPresumedLoc(LockCallExpr->getBeginLoc())
+ .getLine();
+ const auto UnlockLineNum =
+ Result.SourceManager->getPresumedLoc(UnlockCallExpr->getBeginLoc())
+ .getLine();
+
+ // Ignore unlock before lock
+ if (UnlockLineNum < LockLineNum)
+ return;
+
+ diag(LockCallExpr->getBeginLoc(),
+ "use RAII-style locking mechanisms such as 'std::lock_guard' "
+ "or 'std::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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
