[PATCH] D153132: [clang analysis][NFCI] Preparatory work for D153131.

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Is this actually required for the subsequent change? I don't see the connection.

That being said, I haven't spent a lot of thought on what belongs in `ThreadSafetyAnalyzer` and what in `BuildLockset`.



================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:1541
 class BuildLockset : public ConstStmtVisitor<BuildLockset> {
+  using VisitorBase = ConstStmtVisitor<BuildLockset>;
   friend class ThreadSafetyAnalyzer;
----------------
Why the alias? I find this just obfuscates.


================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:1588
 /// of at least the passed in AccessKind.
-void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
-                                      AccessKind AK, Expr *MutexExp,
-                                      ProtectedOperationKind POK,
-                                      til::LiteralPtr *Self,
-                                      SourceLocation Loc) {
+void ThreadSafetyAnalyzer::warnIfMutexNotHeld(
+    const FactSet &FSet, const NamedDecl *D, const Expr *Exp, AccessKind AK,
----------------
Hmm, functions of the same class naturally want to be together, but if you move them, it "destroys" the Git history.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153132