[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

[Vasil Dimov via Phabricator via cfe-commits]

vasild added inline comments.


================
Comment at: clang/docs/ThreadSafetyAnalysis.rst:452-454
+and terminating or throwing if the capability is not held.  Presence of this
+annotation causes the analysis to assume the capability is held at the point of
+the call. See :ref:`mutexheader`, below, for example uses.
----------------
aaronpuchert wrote:
> That's an interesting point. We currently do assume that the capability is held even when an exception is thrown, although that's primarily because Clang's source-level CFG doesn't really model exception handling that well:
> 
> ```
> Mutex mu;
> bool a GUARDED_BY(mu);
> 
> void except() {
>   try {
>     mu.AssertHeld();
>   } catch (...) {
>     a = 0; // No warning.
>   }
> }
> ```
> 
> We can't warn there because `AssertHeld` might terminate in case the mutex isn't held.
> 
> Not sure if we need to clarify this here: we just assume that the capability is held on a regular return. The other case is basically UB for us.
Maybe don't mention "throwing", given the example above and `s/at the call/after the call/`.
It is similar to:

```
void f(char x, unsigned char y)
{
    assert(x >= 0);
    assert(y <= 127);
    if (x > y) { // it is ok to omit a warning about comparing signed and unsigned because AFTER the asserts we know we are good.
        ...
    }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87629