[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
ccotter added a comment. Herald added subscribers: mgehre, shchenz. Herald added projects: clang-tools-extra, All. bump - I know this is really old, but @lewmpk do you plan on finishing up the last remaining comments? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added a comment. IMHO the check is close to being finished. please address the notes and mark them as done if finished with it. So its clear to see whats outstanding. In my opinion the user-facing documentation misses a "Limitations" sections that shows the artificial `goto` example, that would show that the used mechanism doesn't handle it. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90 + for (auto i = 0; i < 3; i++) { +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII JonasToth wrote: > lewmpk wrote: > > JonasToth wrote: > > > I got another one that I think defeats the analysis: > > > > > > ``` > > > while (true) > > > { > > > > > > my_label: > > > m.lock(); > > > > > > if (some_condition) > > > goto my_label; > > > > > > m.unlock(); > > > } > > > ``` > > > Usage of `goto` can interfer and make the situation more complicated. I > > > am not asking you to implement that correctly (not sure if even possible > > > with pure clang-tidy) but I think a pathological example should be > > > documented for the user. > > why would this defeat the analysis? > Because `goto` allows you to reorder you code-locations, so the ordering of > what comes before what is not so ez. let me restate: You are comparing the occurences of `lock` and `unlock` by linenumber, so physical appearance in the source code. `goto` allows you to jump wildly through your code, so that physical appearance does not match actual control flow. I am not saying that you need to resolve that (not easily done within clang-tidy), but documenting it is worth it. And if you mix `goto` and locking mechanisms, you get what you deserve, which is no help from tooling ;) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 189157. lewmpk added a comment. match lock() and unlock() calls by decl 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 +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)
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90 + for (auto i = 0; i < 3; i++) { +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII lewmpk wrote: > JonasToth wrote: > > I got another one that I think defeats the analysis: > > > > ``` > > while (true) > > { > > > > my_label: > > m.lock(); > > > > if (some_condition) > > goto my_label; > > > > m.unlock(); > > } > > ``` > > Usage of `goto` can interfer and make the situation more complicated. I am > > not asking you to implement that correctly (not sure if even possible with > > pure clang-tidy) but I think a pathological example should be documented > > for the user. > why would this defeat the analysis? Because `goto` allows you to reorder you code-locations, so the ordering of what comes before what is not so ez. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk marked an inline comment as done. lewmpk added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90 + for (auto i = 0; i < 3; i++) { +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII JonasToth wrote: > I got another one that I think defeats the analysis: > > ``` > while (true) > { > > my_label: > m.lock(); > > if (some_condition) > goto my_label; > > m.unlock(); > } > ``` > Usage of `goto` can interfer and make the situation more complicated. I am > not asking you to implement that correctly (not sure if even possible with > pure clang-tidy) but I think a pathological example should be documented for > the user. why would this defeat the analysis? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
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 +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
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
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(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 ) { + + const auto *LockCallExpr = Result.Nodes.getNodeAs("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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90 + for (auto i = 0; i < 3; i++) { +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII I got another one that I think defeats the analysis: ``` while (true) { my_label: m.lock(); if (some_condition) goto my_label; m.unlock(); } ``` Usage of `goto` can interfer and make the situation more complicated. I am not asking you to implement that correctly (not sure if even possible with pure clang-tidy) but I think a pathological example should be documented for the user. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:4 +// Mock implementation of std::mutex +namespace std { +struct mutex { lewmpk wrote: > JonasToth wrote: > > Please add more tests > > > > What happens for this? > > ``` > > void foo() { > > std::mutex m; > > m.lock(); > > m.unlock(); > > m.lock(); > > m.unlock(); > > m.try_lock(); > > m.lock(); > > m.unlock(); > > } > > ``` > > > > - Please add tests for templates, where the lock-type is a template > > parameter > > - please add tests where the locking happens within macros > > - please add tests for usage within loops > > - where cases like `std::mutex m1; std::mutex = m1; // usage`. This > > should not be diagnosed, right? > I've added a test case for your example, templates, macros and loops. > I can't catch the case `std::mutex m1; std::mutex = m1; // usage`, but i > can catch trivial cases. Yes, your not supposed to catch those. But i feel things like this should be documented. In theory catching this particular case is possible (we do similar analysis for `const`. But it is totally acceptable to leave as is! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk marked 7 inline comments as done. lewmpk added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:4 +// Mock implementation of std::mutex +namespace std { +struct mutex { JonasToth wrote: > Please add more tests > > What happens for this? > ``` > void foo() { > std::mutex m; > m.lock(); > m.unlock(); > m.lock(); > m.unlock(); > m.try_lock(); > m.lock(); > m.unlock(); > } > ``` > > - Please add tests for templates, where the lock-type is a template parameter > - please add tests where the locking happens within macros > - please add tests for usage within loops > - where cases like `std::mutex m1; std::mutex = m1; // usage`. This > should not be diagnosed, right? I've added a test case for your example, templates, macros and loops. I can't catch the case `std::mutex m1; std::mutex = m1; // usage`, but i can catch trivial cases. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 189055. lewmpk added a comment. added support for boost lockable types 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 +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)
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 189054. lewmpk added a comment. refactor and improved tests 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 +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)
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:35 +void UseRaiiLocksCheck::registerMatchers(MatchFinder *Finder) { + // lock_guards require c++11 or later + if (!getLangOpts().CPlusPlus11) If we allow boost, pre c++11 is ok as well. In general, plz use proper grammar, punctuation and full sentences in comments. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:41 + hasDeclaration(cxxRecordDecl(hasAnyListedName(LockableTypes)); + // Match expressions of type mutex or mutex pointer + const auto MutexExpr = please add proper punctutation in comments. we aim for correct text you can read and understand, with proper spelling and grammar. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:65 + + const auto LockCallExpr = Result.Nodes.getNodeAs("lock"); + const auto UnlockCallExpr = please add the `*` for pointers to emphasize the difference between values and pointers. In general we do not add `const` to values (as i believe is done in later lines), but only for pointers and references. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:69 + + const auto LockObjectName = Lexer::getSourceText( + CharSourceRange::getTokenRange( Please don't retrieve the name like this. Too error prone and compilatcated. You can compare use `DeclRefExpr` (https://clang.llvm.org/doxygen/classclang_1_1DeclRefExpr.html) for your `MutexExpr` instead. From there you go to the `Decl` and compare on pointer equality. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h:28 +"LockableTypes", +"::std::mutex;::std::recursive_mutex;::std::timed_mutex;::std::" +"recursive_timed_mutex;::std::unique_lock")) {} It might be a good idea to add the `boost` types as well? I believe they are interface-compatible, given the std version is derived from them. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:4 +// Mock implementation of std::mutex +namespace std { +struct mutex { Please add more tests What happens for this? ``` void foo() { std::mutex m; m.lock(); m.unlock(); m.lock(); m.unlock(); m.try_lock(); m.lock(); m.unlock(); } ``` - Please add tests for templates, where the lock-type is a template parameter - please add tests where the locking happens within macros - please add tests for usage within loops - where cases like `std::mutex m1; std::mutex = m1; // usage`. This should not be diagnosed, right? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
riccibruno added a comment. Ugh, could you please avoid doing lots a tiny changes every 5 minutes ? This causes spam on cfe-commits /: Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
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-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 + ` check. + + Checks for explicit usage of``std::mutex`` functions ``lock()``, + ``try_lock()`` and ``unlock()``. + - New :doc:`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:
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188919. lewmpk added a comment. support 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-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 + ` check. + + Checks for explicit usage of``std::mutex`` functions ``lock()``, + ``try_lock()`` and ``unlock()``. + - New :doc:`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
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188918. lewmpk added a comment. improved tests 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-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 + ` check. + + Checks for explicit usage of``std::mutex`` functions ``lock()``, + ``try_lock()`` and ``unlock()``. + - New :doc:`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 +//
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188915. lewmpk added a comment. fixed documentation formatting 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,90 @@ +// 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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-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 + ` check. + + Checks for explicit usage of``std::mutex`` functions ``lock()``, + ``try_lock()`` and ``unlock()``. + - New :doc:`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 +//
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk marked 3 inline comments as done. lewmpk added inline comments. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:26 + recordType(hasDeclaration(cxxRecordDecl(hasName("::std::mutex")); + // Match expressions of type mutex or mutex pointer + const auto MutexExpr = lebedev.ri wrote: > Terminology: *this* doesn't match anything. > It's a matcher, yes, but it's just a lambda. > The actual match happens at the end. I was trying to describe its intent, not its action. Does anyone have any suggestions for a clearer comment? Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h:13-14 +#include "../ClangTidy.h" +#include +namespace clang { +namespace tidy { lebedev.ri wrote: > Separate with newline it seems that most checks are this way. it was autogenerated by the ``add_new_check.py`` script. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188913. lewmpk added a comment. fixed documentation formatting 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,90 @@ +// 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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-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 + ` check. + + Checks for explicit usage of``std::mutex`` functions ``lock()``, + ``try_lock()`` and ``unlock()``. + - New :doc:`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 +//
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188912. lewmpk added a comment. fixed documentation again 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,90 @@ +// 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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-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,18 @@ +.. 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 + ` check. + + Checks for explicit usage of``std::mutex`` functions ``lock()``, + ``try_lock()`` and ``unlock()``. + - New :doc:`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 +//
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188911. lewmpk added a comment. fixed documentation 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,90 @@ +// 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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-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,18 @@ +.. title:: clang-tidy - cppcoreguidelines-use-raii-locks + +cppcoreguidelines-use-raii-locks + + +The explicit use of std::mutex functions ``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,11 @@ 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 + ` check. + + Checks for explicit usage of ``std::mutex`` functions ``lock()`` and ``unlock``. + - New :doc:`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 +// +//===--===// +
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188910. lewmpk added a comment. renamed option for cppcoreguidelines-use-raii-locks 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,90 @@ +// 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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-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,16 @@ +.. title:: clang-tidy - cppcoreguidelines-use-raii-locks + +cppcoreguidelines-use-raii-locks + + +The explicit use of std::mutex functions ``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; Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ 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 + ` check. + + Checks for explicit usage of ``std::mutex`` functions ``lock()`` and ``unlock``. + - New :doc:`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
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188908. lewmpk added a comment. handle other basiclockable types 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,90 @@ +// 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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-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,8 @@ +.. title:: clang-tidy - cppcoreguidelines-use-raii-locks + +cppcoreguidelines-use-raii-locks + + +The explicit use of std::mutex functions ``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``. Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ 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 + ` check. + + Checks for explicit usage of ``std::mutex`` functions ``lock()`` and ``unlock``. + - New :doc:`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
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188900. lewmpk added a comment. fixed nested use case 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,80 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t + +// Mock implementation of std::mutex +namespace std { +struct mutex { + void lock(); + void unlock(); +}; +} // 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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(); +} 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-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,8 @@ +.. title:: clang-tidy - cppcoreguidelines-use-raii-locks + +cppcoreguidelines-use-raii-locks + + +The explicit use of std::mutex functions ``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``. Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ 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 + ` check. + + Checks for explicit usage of ``std::mutex`` functions ``lock()`` and ``unlock``. + - New :doc:`google-readability-avoid-underscore-in-googletest-name ` check. Index: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h === --- /dev/null +++ clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h @@ -0,0 +1,34 @@ +//===--- 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" +#include +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/// Check for instances of explicit std::mutex lock() and unlock() calls +/// +/// For the user-facing documentation see: +///
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188901. lewmpk added a comment. removed unneccesary include 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,80 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t + +// Mock implementation of std::mutex +namespace std { +struct mutex { + void lock(); + void unlock(); +}; +} // 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]]:3: warning: use RAII + m1.unlock(); + m2.lock(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII + m2.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(); +} 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-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,8 @@ +.. title:: clang-tidy - cppcoreguidelines-use-raii-locks + +cppcoreguidelines-use-raii-locks + + +The explicit use of std::mutex functions ``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``. Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ 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 + ` check. + + Checks for explicit usage of ``std::mutex`` functions ``lock()`` and ``unlock``. + - New :doc:`google-readability-avoid-underscore-in-googletest-name ` check. Index: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h === --- /dev/null +++ clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h @@ -0,0 +1,34 @@ +//===--- 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: +///
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lebedev.ri added a comment. +1 i was just going to comment - this needs **much** more tests. Also, it would be really great if you could supply the differential's description.T Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:25 + const auto MutexDecl = type(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl(hasName("::std::mutex")); + // Match expressions of type mutex or mutex pointer This should be a config param Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:26 + recordType(hasDeclaration(cxxRecordDecl(hasName("::std::mutex")); + // Match expressions of type mutex or mutex pointer + const auto MutexExpr = Terminology: *this* doesn't match anything. It's a matcher, yes, but it's just a lambda. The actual match happens at the end. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h:13-14 +#include "../ClangTidy.h" +#include +namespace clang { +namespace tidy { Separate with newline Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk added a comment. I'll improve the tests :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
MyDeveloperDay added a comment. This looks good to me but I would wait for one of @JonasToth or @alexfh to perhaps take a look, maybe you should add some nested scope example too using the same name, I know the compiler should warn about a shadow variable anyway but std::mutex m; m.lock(); { std::mutex m; m.lock(); m.unlock(); } m_unlock(); and what about std::mutex m1; std::mutex m2; m1.lock(); m1.unlock(); m2.lock(); m2.unlock(); and std::mutex m1; std::mutex m2; m1.lock(); m2.lock(); m2.unlock(); m1.unlock(); std::mutex m1; std::mutex m2; m1.lock(); m1.unlock(); m2.lock(); m2.unlock(); std::mutex m1; std::mutex m2; m1.lock(); m2.lock(); m1.unlock(); m2.unlock(); Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 188893. lewmpk added a comment. fixed ordering of cppcoreguidelines module checks 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,31 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t + +// Mock implementation of std::mutex +namespace std { +struct mutex { +void lock(); +void unlock(); +}; +} + +void warn_me() { +std::mutex m; +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII +m.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(); +m2.unlock(); +// CHECK-MESSAGES-NOT: warning: +} 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-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,8 @@ +.. title:: clang-tidy - cppcoreguidelines-use-raii-locks + +cppcoreguidelines-use-raii-locks + + +The explicit use of std::mutex functions ``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``. Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ 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 + ` check. + + Checks for explicit usage of ``std::mutex`` functions ``lock()`` and ``unlock``. + - New :doc:`google-readability-avoid-underscore-in-googletest-name ` check. Index: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h === --- /dev/null +++ clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h @@ -0,0 +1,34 @@ +//===--- 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" +#include +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) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult ) override; +}; + +} // 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,84 @@ +//===--- UseRaiiLocksCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
MyDeveloperDay added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:47 +"cppcoreguidelines-use-raii-locks"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-c-arrays"); Nit: by and large this list looks to be in alphabetical based on the checker name (except the last one), not sure if thats by accident or design Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58818/new/ https://reviews.llvm.org/D58818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk created this revision. lewmpk added reviewers: hokein, aaron.ballman, alexfh, JonasToth. Herald added subscribers: cfe-commits, jdoerfert, kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: clang. lewmpk added a subscriber: clang-tools-extra. Repository: rCTE Clang Tools Extra 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,31 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t + +// Mock implementation of std::mutex +namespace std { +struct mutex { +void lock(); +void unlock(); +}; +} + +void warn_me() { +std::mutex m; +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII +m.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(); +m2.unlock(); +// CHECK-MESSAGES-NOT: warning: +} 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-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,8 @@ +.. title:: clang-tidy - cppcoreguidelines-use-raii-locks + +cppcoreguidelines-use-raii-locks + + +The explicit use of std::mutex functions ``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``. Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ 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 + ` check. + + Checks for explicit usage of ``std::mutex`` functions ``lock()`` and ``unlock``. + - New :doc:`google-readability-avoid-underscore-in-googletest-name ` check. Index: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h === --- /dev/null +++ clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h @@ -0,0 +1,34 @@ +//===--- 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" +#include +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) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult ) override; +}; + +} // 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,84 @@ +//===--- UseRaiiLocksCheck.cpp - clang-tidy ---===// +// +// Part of the LLVM Project,