<div dir="ltr"><div dir="ltr"><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 9, 2021 at 5:52 PM Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thank you for raising the concern! In general, I think these sorts of<br>
diagnostics involving macro expansions are difficult. Consider:<br>
<br>
#define MACRO(x) ({int foo[12] = {(int)x}; foo[0]; }) // In some<br>
third-party header<br>
MACRO(0); // In some user code<br>
<br>
In this case, the user has no control over the definition of MACRO and<br>
they have no way to silence the warning from the header, so they may<br>
not appreciate the diagnostic. However, something like:<br>
<br>
#define MACRO(x) x // In some third-party header<br>
MACRO(int foo[12] = {0}); // In some user code<br>
<br>
is something that the user has control over and may reasonably want to<br>
get diagnostics about. Clang-tidy is not supposed to diagnose things<br>
in header files unless the header file is listed in --header-filter<br>
(according to the public docs in<br>
<a href="https://clang.llvm.org/extra/clang-tidy/" rel="noreferrer" target="_blank">https://clang.llvm.org/extra/clang-tidy/</a>) but it seems that<br>
modernize-avoid-c-arrays is diagnosing the macro expansion location,<br>
even when the problematic code is within the macro definition itself<br>
(<a href="https://godbolt.org/z/Gze6Yo3Y4" rel="noreferrer" target="_blank">https://godbolt.org/z/Gze6Yo3Y4</a>) and that may be worth seeing if we<br>
can improve. I'd expect the diagnostic for MACRO1 but not MACRO2 in an<br>
ideal world.<br></blockquote><div><br></div><div>Thank you for the detailed response!</div><div><br></div><div>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.</div><div><br></div><div>  Dmitry</div></div></div>