Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)

[aaronpuchert]

Summary

For now this doesn't make a whole lot of sense, but it will allow us to
store the capability kind in a CapabilityExpr and make sure it doesn't
get lost. The capabilities managed by a scoped lockable can of course be
of different kind, so we'll need to store that per entry.

[aaron.ballman]

LGTM aside from a question (if we can't use emplace_back() the code is still fine).

clang/lib/Analysis/ThreadSafety.cpp

911–919

I think we can continue to use emplace_back() here, can't we?

[aaronpuchert]

clang/lib/Analysis/ThreadSafety.cpp

911–919

For emplace_back we need a constructor and can't just do aggregate initialization. But I could omit the explicit type, like UnderlyingMutexes.push_back({M, UCK_*}).

[aaron.ballman]

Continues to LGTM

clang/lib/Analysis/ThreadSafety.cpp

911–919

Ah good point. No need to change anything, I was just hoping to be able to construct in place.

clang/lib/Analysis/ThreadSafety.cpp

 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ImmutableMap.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>

 class ScopedLockableFactEntry : public FactEntry {
 private:
   enum UnderlyingCapabilityKind {
     UCK_Acquired,          ///< Any kind of acquired capability.
     UCK_ReleasedShared,    ///< Shared capability that was released.
     UCK_ReleasedExclusive, ///< Exclusive capability that was released.
   };
 
-  using UnderlyingCapability =
-      llvm::PointerIntPair<const til::SExpr *, 2, UnderlyingCapabilityKind>;
+  struct UnderlyingCapability {
+    CapabilityExpr Cap;
+    UnderlyingCapabilityKind Kind;
+  };
 
-  SmallVector<UnderlyingCapability, 4> UnderlyingMutexes;
+  SmallVector<UnderlyingCapability, 2> UnderlyingMutexes;
 
 public:
   ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
       : FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
 
   void addLock(const CapabilityExpr &M) {
-    UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+    UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired});
   }
 
   void addExclusiveUnlock(const CapabilityExpr &M) {
-    UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+    UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_ReleasedExclusive});
   }
 
   void addSharedUnlock(const CapabilityExpr &M) {
-    UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
+    UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_ReleasedShared});
   }
 
   void
   handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
                                 SourceLocation JoinLoc, LockErrorKind LEK,
                                 ThreadSafetyHandler &Handler) const override {
     for (const auto &UnderlyingMutex : UnderlyingMutexes) {
-      const auto *Entry = FSet.findLock(
-          FactMan, CapabilityExpr(UnderlyingMutex.getPointer(), false));
-      if ((UnderlyingMutex.getInt() == UCK_Acquired && Entry) ||
-          (UnderlyingMutex.getInt() != UCK_Acquired && !Entry)) {
+      const auto *Entry = FSet.findLock(FactMan, UnderlyingMutex.Cap);
+      if ((UnderlyingMutex.Kind == UCK_Acquired && Entry) ||
+          (UnderlyingMutex.Kind != UCK_Acquired && !Entry)) {
         // If this scoped lock manages another mutex, and if the underlying
         // mutex is still/not held, then warn about the underlying mutex.
         Handler.handleMutexHeldEndOfScope(
-            "mutex", sx::toString(UnderlyingMutex.getPointer()), loc(), JoinLoc,
-            LEK);
+            "mutex", UnderlyingMutex.Cap.toString(), loc(), JoinLoc, LEK);
       }
     }
   }
 
   void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
                   ThreadSafetyHandler &Handler,
                   StringRef DiagKind) const override {
     for (const auto &UnderlyingMutex : UnderlyingMutexes) {
-      CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
-
-      if (UnderlyingMutex.getInt() == UCK_Acquired)
-        lock(FSet, FactMan, UnderCp, entry.kind(), entry.loc(), &Handler,
-             DiagKind);
+      if (UnderlyingMutex.Kind == UCK_Acquired)
+        lock(FSet, FactMan, UnderlyingMutex.Cap, entry.kind(), entry.loc(),
+             &Handler, DiagKind);
       else
-        unlock(FSet, FactMan, UnderCp, entry.loc(), &Handler, DiagKind);
+        unlock(FSet, FactMan, UnderlyingMutex.Cap, entry.loc(), &Handler,
+               DiagKind);
     }
   }
 
   void handleUnlock(FactSet &FSet, FactManager &FactMan,
                     const CapabilityExpr &Cp, SourceLocation UnlockLoc,
                     bool FullyRemove, ThreadSafetyHandler &Handler,
                     StringRef DiagKind) const override {
     assert(!Cp.negative() && "Managing object cannot be negative.");
     for (const auto &UnderlyingMutex : UnderlyingMutexes) {
-      CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
-
       // Remove/lock the underlying mutex if it exists/is still unlocked; warn
       // on double unlocking/locking if we're not destroying the scoped object.
       ThreadSafetyHandler *TSHandler = FullyRemove ? nullptr : &Handler;
-      if (UnderlyingMutex.getInt() == UCK_Acquired) {
-        unlock(FSet, FactMan, UnderCp, UnlockLoc, TSHandler, DiagKind);
+      if (UnderlyingMutex.Kind == UCK_Acquired) {
+        unlock(FSet, FactMan, UnderlyingMutex.Cap, UnlockLoc, TSHandler,
+               DiagKind);
       } else {
-        LockKind kind = UnderlyingMutex.getInt() == UCK_ReleasedShared
+        LockKind kind = UnderlyingMutex.Kind == UCK_ReleasedShared
                             ? LK_Shared
                             : LK_Exclusive;
-        lock(FSet, FactMan, UnderCp, kind, UnlockLoc, TSHandler, DiagKind);
+        lock(FSet, FactMan, UnderlyingMutex.Cap, kind, UnlockLoc, TSHandler,
+             DiagKind);
       }
     }
     if (FullyRemove)
       FSet.removeLock(FactMan, Cp);
   }
 
 private:
   void lock(FactSet &FSet, FactManager &FactMan, const CapabilityExpr &Cp,