vmiklos added a comment. > I think that name is not very descriptive for the user of clang-tidy. `pp` > should be `preprocessor` or some other constellation that makes it very clear > its about the preprocessor.
Done, renamed to readability-redundant-preprocessor. > you are in namespace `clang`, you can ellide `clang::` Done. > Maybe `SmallVector<Entry, 4>`? Would be better for performance. Done. > I think it would be better to have these methods defined inline, as the > splitting does not achieve its original goal (declaration in header, > definition in impl). Done. > The two functions are copied, please remove this duplicatoin and refactor it > to a general parametrized function. Done. > Please order the checks alphabetically in the release notes, and one empty > line at the end is enough. Done. > This needs more explanation, please add `.. code-block:: c++` sections for > the cases that demonstrate the issue. Done. > Please add a test where the redundancy comes from including other headers. If > the headers are nested this case might still occur, but its not safe to > diagnose/remove these checks as other include-places might not have the same > constellation. > > `ifdef` and `ifndef` are used for the include-guards and therefore > particularly necessary to test. Done. I've added a test to make sure we don't warn in headers, the code for this was already there, just had no coverage. (Exactly for the reason you mention, the possibility of include-guards generating false positives.) https://reviews.llvm.org/D54349 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits