llvm dev - Jan 2021 - [clang-tidy][RFC] checks concurrency-async-{blocking, fs, no-new-threads}

Hi,

Thank you for the comment!

David Chisnall:
 > It is possible that coroutine code in the wild isn't doing many deep
 > function calls as more general code, in which case it's quite
plausible
 > that a simple clang-tidy check would be sufficient.  I worry that a
 > deny-list approach is going to be far less maintainable than an
 > allow-list though: in POSIX there are a handful of system calls that can
 > block, but in win32 there are vastly more and higher-level APIs such as
 > Qt have a massive API surface.

Agreed. I don't want the solution that's hard to maintain too.

 > A list of C++ standard-library functions that are safe to call in
 > coroutines seems plausible to maintain.  I wonder if the right approach
 > is a [[clang::coroutine_safe]] attribute that we can put on libc++
 > functions, methods, and classes (if all methods are safe to call, or
 > with a [[clang::coroutine_unsafe]] variant for methods where the default
 > has been defined).  We could then have a checker that warns if you call
 > a function that isn't marked as coroutine-safe from a coroutine or
from
 > a function that is marked as coroutine-safe.

Hm, the attribute sounds interesting. And if it becomes widespread, it could be 
proposed for the next standard...

However, it would suffer from the same "WL somewhere, BL somewhere"
issue - it's
relatively simple to mark with [[clang::*]] for LLVM's code, it would be
harder
for glibc/boost, and it will be impossible for third-party libraries we may not 
commit to. The problem is almost the same as with noexcept or const - if the 
keyword is not set, you may not be certain whether it is intentional or the 
method really throws/changes *this.

Btw, for a full replacement of the PRs there should be at least the 2nd 
attribute for "no fs access". I expect there are many projects that
tolerate fs
access from coroutines as there is no fs thread pool. It's not obvious to me
what the solution should be here (if any).

Hi,

On 27.01.2021 21:45, Vasily Kulikov wrote:
> Hm, the attribute sounds interesting. And if it becomes widespread, it
could be
> proposed for the next standard...
While I agree that the current approach with naive blacklists suffer from
"you
have to explicitly mark all bad functions/types" issue in general, I think
it
still has some non-zero limited value. If you don't develop with WinAPI32, 
OpenGL, or Qt, but with Boost, POSIX and std (e.g. pretty many backend 
projects), the extra steps you have to do by yourself are rather simple (if 
any). Blacklists of widespread libraries can be compiled and added into upstream
clang-tidy by other volunteers.

The patches fit our environment in Yandex.Taxi, might fit others too. I'm
not
sure whether I'm the right person to implement a check with ideologically
better
architecture. If merge BL code, it would not complicate things for the better 
architecture or significantly reduce motivation for invention, it can be 
implemented on top of it. If/when a more precise check is implemented (with 
attributes, or some heuristics), BL tidy check can be deprecated with a 
suggestion to use the new better tool.

So, I suggest to split the problem into two:
1) merge current PRs
2) someday come to a better solution

What do you say?