[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)

[Aaron Puchert via cfe-commits]

================
@@ -1359,17 +1361,18 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
                                        const Expr *Exp, const NamedDecl *D,
                                        const CFGBlock *PredBlock,
                                        const CFGBlock *CurrBlock,
-                                       Expr *BrE, bool Neg) {
-  // Find out which branch has the lock
-  bool branch = false;
-  if (const auto *BLE = dyn_cast_or_null<CXXBoolLiteralExpr>(BrE))
-    branch = BLE->getValue();
-  else if (const auto *ILE = dyn_cast_or_null<IntegerLiteral>(BrE))
-    branch = ILE->getValue().getBoolValue();
----------------
aaronpuchert wrote:

> My instinct is to explicitly disallow enumerator success expressions. [...] To my knowledge, enumerators were never actually supported in success expressions, so in some sense this will only break code that was already broken.

Agreed.

> Is it normally expected to keep incorrect behavior around for compatibility?

It depends. If the incorrect behavior is being relied upon, or the correction could produce hard-to-find issues like different behavior at runtime, then it could make sense to keep it. But here I find it unlikely that someone relies on this, as it does not properly work, and the additional error would at most make compilation fail and would be trivial to fix.
 
> How are breaking changes such as this typically handled?

I'd argue that the change isn't breaking anything, it's merely diagnosing a case that was already not appropriately handled.

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