[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr
Aaron Puchert via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 5 09:31:14 PDT 2018
aaronpuchert added a comment.
In https://reviews.llvm.org/D52888#1256395, @aaron.ballman wrote:
> In https://reviews.llvm.org/D52888#1255862, @aaronpuchert wrote:
>
> > Additional changes (including some non-tail recursion unfortunately) would allow the following to work:
> >
> > void foo16() {
> > if (cond ? mu.TryLock() : false)
> > mu.Unlock();
> > }
> >
> > void foo17() {
> > if (cond ? true : !mu.TryLock())
> > return;
> > mu.Unlock();
> > }
> >
> >
> > Worth the effort, or is that too exotic?
>
>
> `foo16()` looks like code I could plausibly see in the wild. Consider the case where the mutex is a pointer and you want to test it for null before calling `TryLock()` on it (`M ? M->TryLock() : false`). However, I don't have a good feeling for how much work it would be to support that case.
It's relatively short, but not exactly straightforward, because we have to check if both branches are "compatible". However, thinking about this again I think most programmers would write `foo16` as
void foo16() {
if (cond && mu.TryLock())
mu.Unlock();
}
which is functionally equivalent, and which we already support. So I'd propose to add support for this only when someone asks for it, and leave it as it is for now.
Repository:
rC Clang
https://reviews.llvm.org/D52888
More information about the cfe-commits
mailing list