[PATCH] D24656: [clang-tidy] Add check readability-redundant-declaration

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Sorry for the delay. I'm on vacation, returning on Monday.

The patch looks good when the comments are fixed.



================
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:36
+  const bool DifferentHeaders =
+      (!SM.isInMainFile(D->getLocation()) &&
+       !SM.isWrittenInSameFile(Prev->getLocation(), D->getLocation()));
----------------
nit: No need for parentheses here.


================
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:41
+  if (const auto *VD = dyn_cast<VarDecl>(D)) {
+    if (VD && VD->getPreviousDecl()->getStorageClass() == SC_Extern &&
+        VD->getStorageClass() != SC_Extern)
----------------
`VD` is checked on the previous line, no need to repeat the check here.


================
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:61
+    auto Diag = diag(D->getLocation(), "redundant %0 declaration")
+                << cast<NamedDecl>(D);
+    if (!MultiVar && !DifferentHeaders)
----------------
Looks like it's better to make `D` `NamedDecl` and remove the cast here:
  
  Finder->addMatcher(namedDecl(...
  ...
  const auto *D = Result.Nodes.getNodeAs<NamedDecl>("Decl");


================
Comment at: docs/clang-tidy/checks/readability-redundant-declaration.rst:6
+
+Finds redundant variable declarations.
+
----------------
I think, these points should be covered more thoroughly:
  * kinds of declarations supported by the check (function declarations aren't mentioned);
  * how declarations in different files are treated;
  * limitations (e.g. declarations with multiple variables);
  * a few words on the motivation for this check.


Repository:
  rL LLVM

https://reviews.llvm.org/D24656