alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed.
If this kind of an error is not too frequent, it might make sense as a clang diagnostic, indeed. Having it in clang-tidy until then doesn't hurt, though. ================ Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:36 @@ +35,3 @@ + const SourceManager &SM = *Result.SourceManager; + if (SM.isInSystemHeader(VD->getLocation())) + return; ---------------- Don't check for system headers, clang-tidy takes care of this (and sometimes, warnings in system headers are interesting to the standard library maintainers). ================ Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:38 @@ +37,3 @@ + return; + if (!SM.isInMainFile(VD->getLocation()) && + !SM.isWrittenInSameFile(Prev->getLocation(), VD->getLocation())) ---------------- Don't check for the same file. It will prevent fixing headers. ================ Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:44 @@ +43,3 @@ + bool MultiVar = false; + for (const char *Code = SM.getCharacterData(VD->getLocStart()); + *Code && *Code != ';'; ++Code) { ---------------- There should be a better way of checking for multi variable declarations. ================ Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:58-64 @@ +57,9 @@ + } else { + SourceLocation EndLoc = Lexer::getLocForEndOfToken( + VD->getLocEnd(), 0, SM, Result.Context->getLangOpts()); + diag(VD->getLocation(), "redundant variable %0 declaration") + << VD->getName() + << FixItHint::CreateRemoval(SourceRange(VD->getLocStart(), EndLoc)); + diag(VD->getPreviousDecl()->getLocation(), "previously declared here", + DiagnosticIDs::Note); + } ---------------- No need to duplicate this code. Store the diagnostic builder and conditionally insert the fix: { auto Diag = diag(...); if (x) Diag << FixItHint::....; } diag(..., Note); https://reviews.llvm.org/D24656 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits