[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 18 14:08:00 PST 2018


lebedev.ri added a comment.

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55793/new/

https://reviews.llvm.org/D55793





More information about the cfe-commits mailing list