[clang] [NFC][analyzer] Replace BlockInCriticalSection MutexDescriptor with C… (PR #224230)

[Balázs Benics via cfe-commits]

================
@@ -380,37 +309,41 @@ void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
     return;
   }
 
-  if (std::optional<MutexDescriptor> LockDesc =
-          checkDescriptorMatch(Call, C, /*IsLock=*/true)) {
-    if (!std::holds_alternative<RAIIMutexDescriptor>(*LockDesc))
-      handleLock(*LockDesc, Call, C, C.getState());
+  const ThreadingCallDescription *Desc = lookupThreadingCall(Call);
+  if (!Desc)
     return;
-  }
-  if (std::optional<MutexDescriptor> UnlockDesc =
-          checkDescriptorMatch(Call, C, /*IsLock=*/false)) {
-    handleUnlock(*UnlockDesc, Call, C);
+
+  // RAII constructors are modeled in evalCall so they are not inlined.
+  if (isa<CXXConstructorCall>(Call))
+    return;
+
+  switch (Desc->Role) {
+  case Role::Lock:
+    handleLock(*Desc, Call, C, C.getState());
+    break;
+  case Role::Unlock:
+    handleUnlock(*Desc, Call, C);
+    break;
   }
 }
 
 bool BlockInCriticalSectionChecker::evalCall(const CallEvent &Call,
                                              CheckerContext &C) const {
-  if (std::optional<MutexDescriptor> LockDesc =
-          checkDescriptorMatch(Call, C, /*IsLock=*/true)) {
-    if (std::holds_alternative<RAIIMutexDescriptor>(*LockDesc)) {
-      ProgramStateRef State = C.getState();
-      // Escape the object under construction to model the side-effects of the
-      // constructor.
-      if (const auto *Ctor = dyn_cast<AnyCXXConstructorCall>(&Call)) {
-        const MemRegion *ObjRegion = Ctor->getCXXThisVal().getAsRegion();
-        State = State->invalidateRegions(ObjRegion, C.getCFGElementRef(),
-                                         C.blockCount(), C.getStackFrame(),
-                                         /*CausesPointerEscape=*/false);
-      }
-      handleLock(*LockDesc, Call, C, State);
-      return true;
-    }
+  const ThreadingCallDescription *Desc = lookupThreadingCall(Call);
+  if (!Desc || !isa<CXXConstructorCall>(Call))
+    return false;
----------------
steakhal wrote:

I have the impression that `isa<CXXConstructorCall>(Call)` is cheap and does not depend on the lookup. This suggests to me that we should check `isa<CXXConstructorCall>(Call)` (and an early return) before we would do the lookup to save the lookup.

https://github.com/llvm/llvm-project/pull/224230