[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Please upload all patches with full context (`-U9999`)
I'm guessing you'll need help committing this, in which case please specify `Author <e at ma.il>` to be used for `git commit --author="<>"`

Thanks, looks good to me then.
Does anyone else have any further thoughts here?

In D90944#2418864 <https://reviews.llvm.org/D90944#2418864>, @segoon wrote:

> In D90944#2418845 <https://reviews.llvm.org/D90944#2418845>, @lebedev.ri wrote:
>
>> In D90944#2418792 <https://reviews.llvm.org/D90944#2418792>, @segoon wrote:
>>
>>> - mark mt-unsafe decls and check for marks in exprs
>>
>> Eeeh.
>> I was thinking of either some smart matcher "match any function declaration
>> with name from these lists, and then match every call to said decl".
>
> I tried to utilize bind()+equalsBoundNode(), but it seems it's impossible to mark and use the mark in a single matcher.
>
>> But the current implementation, i'm not sure this approach is even legal for checks.
>
> The trick is stolen from abseil/UpgradeDurationConversionsCheck.h. If it's invalid here, then abseil should be fixed too.

The problem with that approach isn't so much that it stores state in check (i guess that is fine in general - see preprocessor handling callbacks),
but that

1. is it guaranteed that we'll match all the decl before first callinst to that decl?
2. and what's worse, this approach wouldn't actually solve the problem, because it still matches all the callinsts, it just caches "is bad callee" check somewhat..


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

https://reviews.llvm.org/D90944