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

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-04-08T17:17:12Z
New Revision: d8c948cfe4eff814a2be71e74cbd835e9be28865

URL: https://github.com/llvm/llvm-project/commit/d8c948cfe4eff814a2be71e74cbd835e9be28865
DIFF: https://github.com/llvm/llvm-project/commit/d8c948cfe4eff814a2be71e74cbd835e9be28865.diff

LOG: [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: https://github.com/llvm/llvm-project/issues/42068

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D146904

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/readability/redundant-declaration.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
index f21f0a785689f..7850a6f29995f 100644
--- 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
index ba041bca86c66..1ab7f4c79c7df 100644
--- 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
index 85ee9a52dfb7e..395839ec3f6f3 100644
--- 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;{{$}}