alexfh added a comment.
There's a problem with this patch. Consider the following code:
// In some remote header:
#define SOME_RANDOM_MACRO internal
// In a completely unrelated file that transitively includes the header:
namespace internal {
void f();
} // namespace internal
It makes the check think that every namespace named `internal` has something to
do with SOME_RANDOM_MACRO.
$ clang_tidy -checks=-*,llvm-* /tmp/q.cc --
1 warning generated.
/tmp/q.cc:7:2: warning: namespace 'SOME_RANDOM_MACRO' ends with a comment
that refers to an expansion of macro [llvm-namespace-comment]
} // namespace internal
^~~~~~~~~~~~~~~~~~~~~~
// namespace SOME_RANDOM_MACRO
/tmp/q.cc:5:11: note: namespace 'SOME_RANDOM_MACRO' starts here
namespace internal {
^
This is definitely wrong and it introduces tons of false positives in our code.
It seems to me that the check shouldn't look any further than what is actually
spelled in the namespace declaration. It shouldn't try to match macros to their
expansions or vice versa. I'll see whether I can implement this quickly,
otherwise I'll just revert this patch to unbreak the checker.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69855/new/
https://reviews.llvm.org/D69855
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
