This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers
ClosedPublic

Authored by george.burgess.iv on Apr 6 2021, 2:29 PM.

Details

Summary

As of 2a3498e24f97d, we ignore parameter name mismatches for functions in the std:: namespace, since those aren't standardized. It seems reasonable to extend this to all functions which are declared in system headers, since this lint can be a bit noisy otherwise (https://bugs.chromium.org/p/chromium/issues/detail?id=1191507).

I'm on the fence about whether this behavior should be governed by an option. I tended toward no in the current patch, since std:: isn't, but I'm happy to add a CheckSystemHeaderDecls flag or similar if that seems better.

Diff Detail

Event Timeline

george.burgess.iv requested review of this revision.Apr 6 2021, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2021, 2:29 PM
Eugene.Zelenko retitled this revision from bugprone-argument-comment: ignore name mismatches for decls from system headers to [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers.Apr 6 2021, 3:15 PM

Why --header-filter command line option or HeaderFilterRegex configuration file option could not solve this problem?

Thanks for the comment!

Why --header-filter command line option or HeaderFilterRegex configuration file option could not solve this problem?

AFAICT, --header-filter & HeaderFilterRegex exist to filter diagnostics that occur in matching headers. These diagnostics are influenced by the contents of system headers, but are ultimately emitted inside of source files that have interesting lints, e.g.,

[ub] /l [ 19ms ] ~> cat test.cpp
#include <string.h>
void foo(void *a, const void *b, size_t n) {
  memcpy(/*blah=*/a, b, n);
}
[ub] /l [ 2ms ] ~> clang-tidy --checks='bugprone-argument-comment' test.cpp --header-filter='dont-match-anything' --
1 warning generated.
/l/test.cpp:3:10: warning: argument name 'blah' in comment does not match parameter name '__dest' [bugprone-argument-comment]
  memcpy(/*blah=*/a, b, n);
         ^
/usr/include/string.h:43:39: note: '__dest' declared here
extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
                                      ^
[ub] /l [ 25ms ] ~> clang-tidy --checks='bugprone-argument-comment' test.cpp --config 'HeaderFilterRegex: "dont-match-anything"' --
1 warning generated.
/l/test.cpp:3:10: warning: argument name 'blah' in comment does not match parameter name '__dest' [bugprone-argument-comment]
  memcpy(/*blah=*/a, b, n);
         ^
/usr/include/string.h:43:39: note: '__dest' declared here
extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
                                      ^
[ub] /l [ 22ms ] ~>

So I don't think those help here.

Why --header-filter command line option or HeaderFilterRegex configuration file option could not solve this problem?

That's for suppressing warnings emitted in header files. The issue this is solving is warnings being emitted at call sites where the callee is in system header files.

aaron.ballman accepted this revision.Apr 23 2021, 11:41 AM

LGTM! I don't think we need to add an option for this until a user requests one, but I don't feel strongly either way.

This revision is now accepted and ready to land.Apr 23 2021, 11:41 AM

...This entirely dropped off my radar. Will try to land it now; thanks, all!