[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ



================
Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+  rm1.lock(); // no-warning
+  rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}
----------------
NoQ wrote:
> ASDenysPetrov wrote:
> > NoQ wrote:
> > > I repeat, this is a false positive. Recursive mutexes can be locked twice, that's the whole point.
> > Yes, I remember. I think you've mixed up with my previous attempts of introducting a new checks for recursive mutexes.
> > This is not this case. This kind of checks already exists in PthreadLockChecker before.
> > I've just reused this check for all kind of STL mutexes.
> > 
> > If you look at `void bad1()` in clang\test\Analysis\pthreadlock.c you can find the same case.
> >  I've just reused this check for all kind of STL mutexes.
> 
> You're taking code for mushing potatoes and applying it to differential calculus.
> 
> Different kinds of mutexes behave differently. This is not the same case. That test is a true positive because that mutex is non-recursive, your test is a false positive because your mutex is recursive. In that test the program immediately hangs and there's no valid way to unhang it, in your test the programmer simply has to unlock the mutex twice and they're good to go.
@Noq, sorry. This might be a simple misunderstanging. Previously there was a question in the summary of what if we will consider a twice lock for recursive mutex as a correct case:
```
recursive_mutex rm;
m.lock();
m.lock(); // OK
```
You've answered that recursive locks are much more complicated than that (https://reviews.llvm.org/D85984#2237242) and offered to make this separately. So I decided not to make any changes and handle recursive_mutexes as all the rest. But if you suggest to do this in a single patch, so I will do.


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

https://reviews.llvm.org/D85984