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

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:39
+  bool MultiVar = false;
+  if (const auto *VD = dyn_cast<VarDecl>(D)) {
+    if (VD && VD->getPreviousDecl()->getStorageClass() == SC_Extern &&
----------------
danielmarjamaki wrote:
> I don't want to generate a fixit. because it could easily break the code. And it will probably depend on inclusion order.
Yep, not generating fixes for redundant declarations in multiple headers totally makes sense (at least, by default).


================
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:45
+    for (const auto Other : VD->getDeclContext()->decls()) {
+      if (Other != D && Other->getLocStart() == VD->getLocStart()) {
+        MultiVar = true;
----------------
danielmarjamaki wrote:
> I think this is better. But not perfect. Maybe there is still a better way.
I don't know whether there is a better way. I somewhat dislike iterating over all declarations here, but it only happens when we're about to generate a warning anyway. Good enough for the start.


================
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:60
+    auto Diag = diag(D->getLocation(), "redundant '%0' declaration")
+        << cast<NamedDecl>(D)->getName();
+    if (!MultiVar && !DifferentHeaders)
----------------
It should be possible to just use `D` here.


https://reviews.llvm.org/D24656