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

2018-11-27 Thread Victor Costan via Phabricator via cfe-commits
pwnall added a comment.

In D52578#1307145 , @aaronpuchert 
wrote:

> 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.


If it makes a difference, Chrome (loosely) tracks Clang trunk 
.
 We wouldn't wait for a stable release to adopt the new code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52578



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


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

2018-10-05 Thread Victor Costan via Phabricator via cfe-commits
pwnall accepted this revision.
pwnall added a comment.
This revision is now accepted and ready to land.

test/SemaCXX/warn-thread-safety-analysis.cpp LGTM -- this is the functionality 
that we need in Chrome to use thread safety annotations for AutoUnlock.

very non-authoritative opinion - I think the tests would be a bit easier to 
follow if Lock() would be the EXCLUSIVE_LOCK_FUNCTION and Unlock() would be the 
EXCLUSIVE_UNLOCK_FUNCTION.

aaronpuchert: Thank you for the patch! I'd be happy and grateful to have this 
functionality in Chrome.
delesley: Thank you for your review and guidance!


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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


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

2018-10-05 Thread Victor Costan via Phabricator via cfe-commits
pwnall added a comment.

Thank you for clarifying, Aaron!

I probably used Phabricator incorrectly. My intent was to state that the tests 
LGTM and express support for the functionality in this patch. Please definitely 
address all review comments before landing.


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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