[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

[Delesley Hutchins via Phabricator via cfe-commits]

delesley added inline comments.


================
Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4570
     mu_.AssertHeld();
-    mu_.Unlock();
-  }  // should this be a warning?
+    mu_.Unlock(); // should this be a warning?
+  }
----------------
This function should have a warning because is not marked with UNLOCK_FUNCTION.  The lock was held on entry, and not held on exit.  So the original location of the comment, on the closing curly brace, was correct.  


================
Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4629
+      mu_.AssertHeld(); // expected-note {{the other acquisition of mutex 'mu_' is here}}
+    // FIXME: should instead warn because it's unclear whether we need to release or not.
+    int b = a;
----------------
I'm not wild about having FIXMEs in test code.  I would prefer to have the patch actually issue a warning here (but see below).

HOWEVER, does the current code issue a warning in this situation?  If not, leave it as-is and keep the FIXME.  You can't make the analysis more strict without checking with the C++ compiler team at Google (which I am no longer on), because it may break the build on our end.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102026