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

Aaron Puchert via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 30 16:32:08 PDT 2024


================
@@ -608,15 +606,31 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
   if (!AL.checkAtLeastNumArgs(S, 1))
     return false;
 
-  if (!isIntOrBool(AL.getArgAsExpr(0))) {
+  // The attribute's first argument defines the success value.
+  const Expr *SuccessArg = AL.getArgAsExpr(0);
+  if (!isa<CXXNullPtrLiteralExpr>(SuccessArg) &&
----------------
aaronpuchert wrote:

I can see the appeal of `nullptr` in the attribute, but it suggests you can put arbitrary pointers in there, and I don't think we want that. But I can live with it.

Though it seems strange that one would return a pointer, and `nullptr` is success. Maybe an error message? More typical is probably to have `nullptr` if the lock couldn't be acquired.

In any event, since we want to able to express both the null and non-null case, conversion to `bool` seems the better conceptual framework. How much we need bother users with that framework is another question, but it's probably not a bad idea to bring home the idea that we're not trying to track pointers.

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


More information about the cfe-commits mailing list