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

[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