aaron.ballman added inline comments.

================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:1644
     if (!LDat) {
-      Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(),
-                                           LK_Shared, Loc);
+      Analyzer->Handler.handleMutexNotHeld("negative capability", D, POK,
+                                           Cp.toString(), LK_Shared, Loc);
----------------
aaronpuchert wrote:
> aaron.ballman wrote:
> > It's a bit odd that we aren't using `DiagKind` as below, I assume that's 
> > because this is a negative test and the others are positive tests, but 
> > doesn't this introduce a terminology difference where a positive failure 
> > may call it a mutex and a negative failure may call it a negative 
> > capability? Should this be hooked in to `ClassifyDiagnostic()` (perhaps we 
> > need a `ClassifyNegativeDiagnostic()`?
> My thinking was that whatever the positive capability is called, we should 
> only talk about a "negative capability" instead of a "negative mutex" or a 
> "negative role". Also because not holding a capability is in some way its own 
> kind of capability.
I may still be confused or thinking of this differently, but I would assume 
that a negative mutex would be a mutex that's explicitly not held, which you 
may want to ensure on a function boundary to avoid deadlock. From that, I'd 
have guessed we would want the diagnostic to read `cannot call function 'bar' 
while mutex 'mu' is held` or `calling function 'bar' requires mutex 'mu' to not 
be held` because that's more clear than talking about negative capabilities 
(when the user is thinking in terms of mutexes which are or aren't held).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84603/new/

https://reviews.llvm.org/D84603

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to