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

[Alexander Kornienko via cfe-commits]

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