-Wthread-safety-analysis FR: automatic inference of annotations for local-scope lambdas

[pkasting]

With -Wthread-safety-analysis, accessing a guarded resource inside a lambda requires annotating the lambda similarly to any other function. See sample code at https://godbolt.org/z/7Pd1f3v5v; without the annotation on the lambda, compilation produces a warning.

This is problematic when the lambda comes from a macro. For example, in Chromium, we have an assert()-like macro called CHECK(), which currently preprocesses to an expression, but which we'd like to turn into a statement (in order to use C++20 [[unlikely]], which can only appear on a statement). A simplified implementation might look akin to:

#define CHECK(cond) [&] { if (!(cond)) [[unlikely]] CheckFailure(); }()

Unfortunately, if the condition being CHECKed is subject to thread safety analysis, this can produce cryptic compile failures. We could annotate this lambda with no_thread_safety_analysis, but that would hide any real thread-safety issues inside the condition. What we'd like is a partly- or fully-automatic way of making this work.

It seems like this may become intractable if the scope is "any lambda with any linkage", so perhaps if we say that only immediately-invoked lambdas are of concern, things are better? Then perhaps the requirements can be inferred either from the body of the function (what does it actually require?) or else from the invocation context (what resources are available?).

[nico]

(in order to use C++20 [[unlikely]]

We do PGO builds. If [[unlikely]] has a big effect on runtime perf, that's a bug with PGO. If putting [[unlikely]] in CHECK has a big perf impact, that's imho the bug.

(And without [[unlikely]], this isn't a problem for us AFAIU?)

[pkasting]

This is tangent to the upstream bug, since it's more about Chromium motivations, but -- We have used __builtin_expect() to mark CHECK() as "unlikely to fail" for a long time, so this isn't about trying to add an annotation in hopes it's beneficial, it's just about trying to replace the existing Clang-specific annotation with a standard-compliant one.

Of course, your question still stands: can we just eliminate the annotation entirely? While PGO builds mitigate the need for [[unlikely]], they don't eliminate it, for two reasons:

1. PGO is only as effective as its training data, and while we do our best, we know we're not comprehensive for all scenarios we care about (and in the limit will never be). In the case of a CHECK(), we know there should be no downside to [[unlikely]], so it's just a question of how much upside there is.
2. A lot of cases where performance matters are run on non-PGO-optimized builds, and we'd like the delta between the two to be as small as reasonable. (There is also code (like PartitionAlloc) that is not only perf-sensitive but potentially used by projects that don't do PGO, though that's sort of an edge case, albeit an important one.)