[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

I've thought about your concerns and the important thing is: I didn't actually test this on a larger code base. The change makes sense to me, but maybe I'm missing something. So maybe instead of pushing through further restrictions, I should focus on rolling out the analysis on our own messy code base. Maybe I'm going to see the false positives right there.

However, as messy as our code may be, it might be messy in different ways than Google's code. So I might not see the same issues. Then on the other hand, there might be (probably smaller) code bases which don't have a problem with stricter rules. So I think additional flags are probably not the worst idea, but we need to keep their number small.

> Second, there needs to be a thread-safety-variant of "const_cast" -- i.e. a way to pass a reference to a function without triggering the warning.  That may already be possible via clever use of our current annotations, or you may have to fiddle with something, but it needs to appear in the test cases.

There is such a mechanism in the test suite. It uses that we don't check the arguments for functions annotated with `no_thread_safety_analysis`, introduced by r246806 <https://reviews.llvm.org/rC246806>:

  // Takes a reference to a guarded data member, and returns an unguarded
  // reference.
  template <class T>
  inline const T& ts_unchecked_read(const T& v) NO_THREAD_SAFETY_ANALYSIS {
    return v;
  }
  
  template <class T>
  inline T& ts_unchecked_read(T& v) NO_THREAD_SAFETY_ANALYSIS {
    return v;
  }


Repository:
  rC Clang

https://reviews.llvm.org/D52395