[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-04-06 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdfec26b186d2: Thread safety analysis: Don't warn about managed locks on join points (authored by aaronpuchert). Repository: rG LLVM Github Monorep

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-04-06 Thread Delesley Hutchins via Phabricator via cfe-commits
delesley accepted this revision. delesley added a comment. This revision is now accepted and ready to land. I am convinced by your argument. I think it is reasonable to assume that if someone is using an RAII object, then the underlying object is responsible for managing double locks/unlocks, a

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-04-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Ping @aaron.ballman, @delesley. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98747/new/ https://reviews.llvm.org/D98747 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-03-25 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D98747#2648943 , @aaronpuchert wrote: > We could probably address all false negatives by introducing a `FactEntry` > for "managed locks that might be held", which we introduce on discrepancies > at join points. We assume

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-03-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Not replying in order, I hope that's not confusing. Let's start with the motivation. In D98747#2648381 , @delesley wrote: > Is there a compelling reason for this change, other than consistency/symmetry > concerns? In other

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-03-24 Thread Delesley Hutchins via Phabricator via cfe-commits
delesley added a comment. Thanks for roping me into the conversation, Aaron, and sorry about the delay. I have mixed feelings about this patch. With respect to the purpose of thread safety analysis, finding race conditions is obviously a major concern, because race conditions are hard to find

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-03-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I'd really like to hear from @delesley about these changes, specifically because of this bit: > the primary goal of the Thread safety analysis is not to find double locks > but race conditions. I believe the primary goal of TSA is to find threading-related issues

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-03-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98747/new/ https://reviews.llvm.org/D98747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-03-18 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 331567. aaronpuchert added a comment. Negative capabilities don't need to be considered as managed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98747/new/ https://reviews.llvm.org/D98747 Files: clang/

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

2021-03-16 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We already did so for scoped locks acquired in the constructor, this change extends the