[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

> As the analysis grew more complex, I switched to the current system based on "facts". There are a number of facts that are potentially useful in static analysis, such as whether one expression aliases another, and most of them don't look at all like capabilities. IMHO, knowing whether an object is within scope falls into this class.

Agreed. Though currently scoped capabilities aren't (necessarily) added/removed from the fact set on construction/destruction, only when the constructor/destructor is annotated as `ACQUIRE()`/`RELEASE()`. I was thinking about changing this, but haven't looked into it yet. This would clearly separate the fact of holding a scoped “capability” from the actual capabilities it represents.

> The only real argument for treating scoped lockable objects as proxies, which can be "locked" and "unlocked", is that you can (in a future patch) reuse the existing acquire_capability and release_capability attributes to support releasing and then re-acquiring the proxy. It's a bit counter-intuitive, but the alternative is adding new attributes, which is also bad.

Scoped capabilities can already be released (before destruction) since your change rC159387 <https://reviews.llvm.org/rC159387>, and I added the possibility to reacquire them in rC340459 <https://reviews.llvm.org/rC340459>. I agree that it's technically an abuse of notation <https://en.wikipedia.org/wiki/Abuse_of_notation>, but one can wrap it's head around it after a while. It is not possible though to annotate functions outside of the scoped capability class as acquiring/releasing a scoped capability. Right now I don't see a reason to change that.

To be clear, I'm not a big fan of this change myself, I just wanted to see if it was feasible. My personal opinion, as I wrote in the bug report, is that scoped releasing of mutexes is taking RAII a step too far. I'm putting this on ice for now until we're reached a state where it looks a bit less crazy. I hope @pwnall can live with that, since Clang 8 will not come out soon anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52578