[clang-tidy] Fix extern fixes in readability-redundant-declaration

Currently check does not look into LinkageSpecDecl when removing redundant variable re-declarations. This were leaving code in non-compiling state. Fix patch fixes this and adds removal also of 'extern C'. Fixes: #42068 Reviewed By: carlosgalvezp Differential Revision: https://reviews.llvm.org/D146904
llvm · Apr 8, 2023 · d8c948c · d8c948c
1 parent 88622aa
commit d8c948c
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 5 deletions.
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -35,7 +35,8 @@ void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
                       functionDecl(unless(anyOf(
                           isDefinition(), isDefaulted(),
                           doesDeclarationForceExternallyVisibleDefinition(),
-                          hasAncestor(friendDecl()))))))
+                          hasAncestor(friendDecl()))))),
+                optionally(hasParent(linkageSpecDecl().bind("extern"))))
           .bind("Decl"),
       this);
 }
@@ -78,9 +79,17 @@ void RedundantDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
       D->getSourceRange().getEnd(), 0, SM, Result.Context->getLangOpts());
   {
     auto Diag = diag(D->getLocation(), "redundant %0 declaration") << D;
-    if (!MultiVar && !DifferentHeaders)
-      Diag << FixItHint::CreateRemoval(
-          SourceRange(D->getSourceRange().getBegin(), EndLoc));
+    if (!MultiVar && !DifferentHeaders) {
+      SourceLocation BeginLoc;
+      if (const auto *Extern =
+              Result.Nodes.getNodeAs<LinkageSpecDecl>("extern");
+          Extern && !Extern->hasBraces())
+        BeginLoc = Extern->getExternLoc();
+      else
+        BeginLoc = D->getSourceRange().getBegin();
+
+      Diag << FixItHint::CreateRemoval(SourceRange(BeginLoc, EndLoc));
+    }
   }
   diag(Prev->getLocation(), "previously declared here", DiagnosticIDs::Note);
 }

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -275,6 +275,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/misleading-indentation>` check when warning would
   be unnecessarily emitted for template dependent ``if constexpr``.
 
+- Fixed incorrect fixes in :doc:`readability-redundant-declaration
+  <clang-tidy/checks/readability/redundant-declaration>` check when linkage
+  (like ``extern "C"``) is explicitly specified.
+
 - Improved :doc:`readability-static-accessed-through-instance
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   support unscoped enumerations through instances and fixed usage of anonymous
@@ -298,7 +302,7 @@ Changes in existing checks
   ``DISABLED_`` in the test suite name.
 
 - Fixed an issue in :doc:`modernize-concat-nested-namespaces
-  <clang-tidy/checks/modernize/concat-nested-namespaces>` when using macro between 
+  <clang-tidy/checks/modernize/concat-nested-namespaces>` when using macro between
   namespace declarations could result incorrect fix.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move

diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp
@@ -120,3 +120,9 @@ extern inline void g(); // extern g
 // CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' declaration
 // CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
 #endif
+
+// PR42068
+extern "C" int externX;
+int dummyBeforeBegin;extern "C" int externX;int dummyAfterEnd;
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant 'externX' declaration [readability-redundant-declaration]
+// CHECK-FIXES: {{^}}int dummyBeforeBegin;int dummyAfterEnd;{{$}}