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

[Dmitry Polukhin via cfe-dev]

On Wed, Jun 9, 2021 at 5:52 PM Aaron Ballman <aaron at aaronballman.com> wrote:

> 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.
>

Thank you for the detailed response!

It is possible to distinguish locations from macro arguments and macro
bodies. It is not implemented in my diff but it is a straightforward
addition to report the issue for the second case (macro argument) and don't
report for the first case (macro body). But in general the exact point for
reporting issues is somewhat arbitrary i.e. it is the most convenient place
for the user to see what is wrong or about which declaration the report is.
This report point can come from the macro argument but the rest where the
issue really is could come from the macro body. In case
of modernize-avoid-c-arrays, the point of the report is the beginning of
the whole declaration. But the issue could be reported on the name of the
variable or the array type, they are also reasonable points for the report.
So there is no one solution that will work the best in all cases. But the
issue is important and I think clang-tidy needs to have some options to
hide the majority of the unwanted reports from macro expansions.
Implementing this logic in every check is not practical so I think it
should be common logic for all checks like NOLINT.

  Dmitry
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20210609/8412bcfc/attachment.html>