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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:21
+void DuplicatedAccessSpecifiersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxRecordDecl(has(accessSpecDecl()))
----------------
You should only register this matcher in C++ mode.


================
Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:31
+
+  AccessSpecDecl const *lastAccessDecl = nullptr;
+  for (DeclContext::specific_decl_iterator<AccessSpecDecl>
----------------
Please switch to `const AccessSpecDecl *`. Also, that should be `LastAccessDecl` per the naming conventions.


================
Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:33
+  for (DeclContext::specific_decl_iterator<AccessSpecDecl>
+           NS(MatchedDecl->decls_begin()),
+       NSEnd(MatchedDecl->decls_end());
----------------
Why `NS` -- that seems like a strange naming choice.


================
Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:36
+       NS != NSEnd; ++NS) {
+    const auto *decl = *NS;
+
----------------
Eugene.Zelenko wrote:
> Type is not obvious here, so please don't use auto.
Also, `decl` doesn't match our naming conventions -- change to `ASDecl`?


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

https://reviews.llvm.org/D55793