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