[clang] [clang][analyzer] Improve BlockInCriticalSectionsChecker (PR #80029)

[via cfe-commits]

Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.com>,
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/80029 at github.com>


================
@@ -20,48 +20,178 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+
+#include <iterator>
+#include <utility>
+#include <variant>
 
 using namespace clang;
 using namespace ento;
 
 namespace {
+
+struct CritSectionMarker {
+  const Expr *LockExpr{};
+  const MemRegion *LockReg{};
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+    ID.Add(LockExpr);
+    ID.Add(LockReg);
+  }
+
+  [[nodiscard]] constexpr bool
+  operator==(const CritSectionMarker &Other) const noexcept {
+    return LockExpr == Other.LockExpr && LockReg == Other.LockReg;
+  }
+  [[nodiscard]] constexpr bool
+  operator!=(const CritSectionMarker &Other) const noexcept {
+    return !(*this == Other);
+  }
+};
+
+class CallDescriptionBasedMatcher {
+  CallDescription LockFn;
+  CallDescription UnlockFn;
+
+public:
+  CallDescriptionBasedMatcher(CallDescription &&LockFn,
+                              CallDescription &&UnlockFn)
+      : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+  [[nodiscard]] bool matches(const CallEvent &Call, bool IsLock) const {
+    if (IsLock) {
+      return LockFn.matches(Call);
+    }
----------------
NagyDonat wrote:

```suggestion
    if (IsLock)
      return LockFn.matches(Call);
```
Bikeshedding: the coding standard says that braces shouldn't be used on simple single-statement bodies: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

Unorthodox but (IMO) elegant alternative idea: replace the whole body of the function with
```
return (isLock ? LockFn : UnlockFn).matches(Call);
```

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