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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
