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

2023-03-26 Thread Chris Cotter via Phabricator via cfe-commits
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

2019-03-07 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-03-07 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-03-04 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-03 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-03-03 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-03 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:29
+
+DeclRefExpr *findDeclRefExpr(const CXXMemberCallExpr *MemberCallExpr) {
+  auto *ObjectExpr = MemberCallExpr->getImplicitObjectArgument();

Please use static for functions instead of anonymous namespaces. See LLVM 
coding guidelines.



Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:30
+DeclRefExpr *findDeclRefExpr(const CXXMemberCallExpr *MemberCallExpr) {
+  auto *ObjectExpr = MemberCallExpr->getImplicitObjectArgument();
+  if (auto *ObjectCast = dyn_cast(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

2019-03-02 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-03-02 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-03-02 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-02 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-02 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-03-01 Thread Bruno Ricci via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread Roman Lebedev via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread MyDeveloperDay via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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

2019-03-01 Thread MyDeveloperDay via Phabricator via cfe-commits
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

2019-03-01 Thread Lewis Clark via Phabricator via cfe-commits
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,