[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Alexander Kornienko via Phabricator via cfe-commits]

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