m4tx marked 4 inline comments as done. m4tx added a comment. In D55793#1335274 <https://reviews.llvm.org/D55793#1335274>, @lebedev.ri wrote:
> In D55793#1335249 <https://reviews.llvm.org/D55793#1335249>, @m4tx wrote: > > > In D55793#1333661 <https://reviews.llvm.org/D55793#1333661>, @lebedev.ri > > wrote: > > > > > Please add tests with preprocessor (`#if ...`) that will show that it > > > ignores disabled code. e.g.: > > > > > > class ProbablyValid { > > > private: > > > int a; > > > #if defined(ZZ) > > > public: > > > int b; > > > #endif > > > private: > > > int c; > > > protected: > > > int d; > > > public: > > > int e; > > > }; > > > > > > > > > Is this actually possible? > > It seems that macros are ran through the preprocessor before one can > > fiddle with them in clang-tidy. > > In other words, `int b` is not at all present in the AST. > > > .. and by "ignores" i meant that it **will** be diagnosing this code, since > it did not know anything about the code within the preprocessor-disabled > section. > > > However, I added a code to detect macro expansions, so duplicated access > > specifiers are ignored if at least one of them comes from a macro. If there > > is a way to cover your case as well, please let me know, because even after > > looking at the code of other checks I haven't found out a solution for this. Ok, thanks for the clarification. I've added the test in the latest diff! ================ Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:51 + diag(ASDecl->getLocation(), "duplicated access specifier") + << MatchedDecl + << FixItHint::CreateRemoval(ASDecl->getSourceRange()); ---------------- aaron.ballman wrote: > There is no %0 in the diagnostic string, so I'm not certain what this > argument is trying to print out. Did you mean `ASDecl->getSourceRange()` for > the underlining? Sorry, this is another line I forgot to remove. Thanks for pointing this out! By the way, does the underlining work in clang-tidy's `diag()` function? I see it is used outside the project, but here only `FixItHint`s seem to generate underline in the generated messages. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55793/new/ https://reviews.llvm.org/D55793 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits