[cfe-dev] [clang-tidy] Undesired diagnostics due to macro expansion from third-party headers

[Aaron Ballman via cfe-dev]

On Wed, Jun 9, 2021 at 11:18 AM Dmitry Polukhin via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> Hi All,
>
> I would like to know people's opinion about clang-tidy diagnostics due to macro expansion of the macro defined in a third-party header. It is a huge pain for clang-tidy deployment on a large codebase that actively uses third-party bits that might not use the same clang-tidy checks enabled. Inserting NOLINT annotation might not be a solution because in some cases it is important to keep third-party code unmodified at all. Just one example from a very popular gflags library. This library requires using macro in use code like this:
>
> DEFINE_bool(some_bool_flag,
>             "the default value should go here, not the description",
>             false);
>
> But the macro has an outdated version of compiler assert that uses [C-style arrays](https://github.com/gflags/gflags/blob/2e227c3daae2ea8899f49858a23f3d318ea39b57/src/gflags.h.in#L528). Therefore use code becomes incompatible with clang-tidy check modernize-avoid-c-array. Another example of problematic is googletest/googlemock with lots of macro that you cannot avoid.
>
> I created RFC diff https://reviews.llvm.org/D90835 but got very limited feedback about the proposed approach. So bringing it here to increase visibility of the problem. Please share what you think about the problem itself and how it should be addressed in general. Feedback about the diff is also very appreciated but first of all I would like to understand how you think the problem should be addressed in principle.

Thank you for raising the concern! In general, I think these sorts of
diagnostics involving macro expansions are difficult. Consider:

#define MACRO(x) ({int foo[12] = {(int)x}; foo[0]; }) // In some
third-party header
MACRO(0); // In some user code

In this case, the user has no control over the definition of MACRO and
they have no way to silence the warning from the header, so they may
not appreciate the diagnostic. However, something like:

#define MACRO(x) x // In some third-party header
MACRO(int foo[12] = {0}); // In some user code

is something that the user has control over and may reasonably want to
get diagnostics about. Clang-tidy is not supposed to diagnose things
in header files unless the header file is listed in --header-filter
(according to the public docs in
https://clang.llvm.org/extra/clang-tidy/) but it seems that
modernize-avoid-c-arrays is diagnosing the macro expansion location,
even when the problematic code is within the macro definition itself
(https://godbolt.org/z/Gze6Yo3Y4) and that may be worth seeing if we
can improve. I'd expect the diagnostic for MACRO1 but not MACRO2 in an
ideal world.

~Aaron

>
> Thanks,
> Dmitry
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev