[llvm] [clang] [clang-tools-extra] Fix #35272: Don't replace typedefs in extern c scope (PR #69102)

via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 10 12:03:39 PST 2023 

https://github.com/Da-Viper updated https://github.com/llvm/llvm-project/pull/69102

>From 21156656433fb8d2dc5a805d97cbd20fa916fff9 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 15 Oct 2023 11:39:42 +0100
Subject: [PATCH 1/9] Fix #35272: Don't replace typedefs in extern c scope

---
 .../clang-tidy/modernize/UseUsingCheck.cpp       | 16 ++++++++++++----
 .../clang-tidy/checkers/modernize/use-using.cpp  | 14 ++++++++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index e6293ed48bfddb..841ffb4c9bfe66 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -11,6 +11,12 @@
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
+namespace {
+
+AST_MATCHER(clang::LinkageSpecDecl, isExternCLinkage) {
+  return Node.getLanguage() == clang::LinkageSpecDecl::lang_c;
+}
+} // namespace
 
 namespace clang::tidy::modernize {
 
@@ -27,10 +33,12 @@ void UseUsingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated()),
-                                 hasParent(decl().bind(ParentDeclName)))
-                         .bind(TypedefName),
-                     this);
+  Finder->addMatcher(
+      typedefDecl(unless(anyOf(isInstantiated(), hasAncestor(linkageSpecDecl(
+                                                     isExternCLinkage())))),
+                  hasParent(decl().bind(ParentDeclName)))
+          .bind(TypedefName),
+      this);
 
   // This matcher is used to find tag declarations in source code within
   // typedefs. They appear in the AST just *prior* to the typedefs.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 422abee11a7196..0f8f14502d5ca3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -325,3 +325,17 @@ typedef bool (*ISSUE_65055_2)(int);
 typedef class ISSUE_67529_1 *ISSUE_67529;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: using ISSUE_67529 = class ISSUE_67529_1 *;
+
+// Some Header
+extern "C" {
+
+typedef int InExternC;
+}
+
+extern "C++" {
+
+typedef int InExternCPP;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using InExternCPP = int;
+
+}

>From 521dec9325285d1e1819a8bee1bd20eadb7c4158 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Mon, 16 Oct 2023 23:26:25 +0100
Subject: [PATCH 2/9] Add: Update docs with the new changes. Update
 ReleaseNotes.rst with the changes made

---
 clang-tools-extra/docs/ReleaseNotes.rst                  | 5 +++++
 .../docs/clang-tidy/checks/modernize/use-using.rst       | 9 +++++++++
 2 files changed, 14 insertions(+)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c1b926b296b055..af6b20369c9dcf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -285,6 +285,10 @@ Changes in existing checks
   <clang-tidy/checks/modernize/use-using>` check to fix function pointer and
   forward declared ``typedef`` correctly.
 
+- Improved :doc:`modernize-use-using
+  <clang-tidy/checks/modernize/use-using>` by ignoring ``typedef`` declaration in
+  ``extern "C"`` scope.
+
 - Improved :doc:`performance-faster-string-find
   <clang-tidy/checks/performance/faster-string-find>` check to properly escape
   single quotes.
@@ -325,6 +329,7 @@ Changes in existing checks
   identify calls to static member functions with out-of-class inline definitions.
 
 
+
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
index eeddaf8d8d65ab..048fc26617b7b7 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
@@ -28,6 +28,15 @@ After:
   using R_t = struct { int a; };
   using R_p = R_t*;
 
+The checker ignores `typedef` within `extern "C" { ... }` blocks.
+
+.. code-block:: c++
+
+  extern "C" {
+
+    typedef int InExternC; // Left intact.
+  }
+
 This check requires using C++11 or higher to run.
 
 Options

>From 3426e0a36606a7e3eeb38c0f436c25aa2fde2b36 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 22 Oct 2023 20:02:20 +0100
Subject: [PATCH 3/9] Update: commit with review requested changes

---
 clang-tools-extra/docs/ReleaseNotes.rst                     | 6 +-----
 .../docs/clang-tidy/checks/modernize/use-using.rst          | 2 --
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index af6b20369c9dcf..e3bc99ce162135 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -283,10 +283,7 @@ Changes in existing checks
 
 - Improved :doc:`modernize-use-using
   <clang-tidy/checks/modernize/use-using>` check to fix function pointer and
-  forward declared ``typedef`` correctly.
-
-- Improved :doc:`modernize-use-using
-  <clang-tidy/checks/modernize/use-using>` by ignoring ``typedef`` declaration in
+  forward declared ``typedef`` correctly. Ignore ``typedef`` declaration in
   ``extern "C"`` scope.
 
 - Improved :doc:`performance-faster-string-find
@@ -329,7 +326,6 @@ Changes in existing checks
   identify calls to static member functions with out-of-class inline definitions.
 
 
-
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
index 048fc26617b7b7..a056eaf826fda3 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
@@ -31,9 +31,7 @@ After:
 The checker ignores `typedef` within `extern "C" { ... }` blocks.
 
 .. code-block:: c++
-
   extern "C" {
-
     typedef int InExternC; // Left intact.
   }
 

>From c1d36bcc7b2afa874ddeaab15b51ce128d88bf4d Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 29 Oct 2023 11:56:16 +0000
Subject: [PATCH 4/9] Add: Update ReleaseNotes.rst with the changes made

---
 clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
index a056eaf826fda3..c47a022f2c9868 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
@@ -31,6 +31,7 @@ After:
 The checker ignores `typedef` within `extern "C" { ... }` blocks.
 
 .. code-block:: c++
+
   extern "C" {
     typedef int InExternC; // Left intact.
   }

>From 3770a1c974813be894af629efb1f6d433bfe2ea9 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 29 Oct 2023 12:02:15 +0000
Subject: [PATCH 5/9] Revert "Add: Update ReleaseNotes.rst with the changes
 made"

This reverts commit cf8aae9305dd689ae042199c8ab8e5adc3b69489.
---
 clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
index c47a022f2c9868..a056eaf826fda3 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
@@ -31,7 +31,6 @@ After:
 The checker ignores `typedef` within `extern "C" { ... }` blocks.
 
 .. code-block:: c++
-
   extern "C" {
     typedef int InExternC; // Left intact.
   }

>From fccb26b75fa0ab194a0af2d4971a418fdb19e1e7 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 29 Oct 2023 12:04:38 +0000
Subject: [PATCH 6/9] Add: Update ReleaseNotes.rst with the changes made

---
 clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
index a056eaf826fda3..99410821c0f05b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
@@ -32,6 +32,7 @@ The checker ignores `typedef` within `extern "C" { ... }` blocks.
 
 .. code-block:: c++
   extern "C" {
+
     typedef int InExternC; // Left intact.
   }
 

>From 9e55ba4a06e9ac46b6b8d535499376bc8a203bcf Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 29 Oct 2023 12:17:50 +0000
Subject: [PATCH 7/9] Fix: place the space in the correct place

---
 .../docs/clang-tidy/checks/modernize/use-using.rst              | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
index 99410821c0f05b..c47a022f2c9868 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
@@ -31,8 +31,8 @@ After:
 The checker ignores `typedef` within `extern "C" { ... }` blocks.
 
 .. code-block:: c++
-  extern "C" {
 
+  extern "C" {
     typedef int InExternC; // Left intact.
   }
 

>From d3185b530528dc084e22146ac8642af0ed735c22 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 26 Nov 2023 00:59:28 +0000
Subject: [PATCH 8/9] Add: Put the ignoreExternC behind optional flag.

Update the documentation with the new option `IgnoreExternC`
---
 .../clang-tidy/modernize/UseUsingCheck.cpp    | 21 ++++++++++++++-----
 .../clang-tidy/modernize/UseUsingCheck.h      |  1 +
 .../clang-tidy/checks/modernize/use-using.rst |  5 +++++
 .../modernize/use-using-ignore-extern-c.cpp   | 14 +++++++++++++
 .../checkers/modernize/use-using.cpp          |  3 +++
 5 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-using-ignore-extern-c.cpp

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index 841ffb4c9bfe66..f5fc3ad3fac68b 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -20,23 +20,28 @@ AST_MATCHER(clang::LinkageSpecDecl, isExternCLinkage) {
 
 namespace clang::tidy::modernize {
 
+static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+      IgnoreExternC(Options.get("IgnoreExternC", false)) {}
 
 void UseUsingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+  Options.store(Opts, "IgnoreExternC", IgnoreExternC);
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-      typedefDecl(unless(anyOf(isInstantiated(), hasAncestor(linkageSpecDecl(
-                                                     isExternCLinkage())))),
-                  hasParent(decl().bind(ParentDeclName)))
+      typedefDecl(
+          unless(isInstantiated()),
+          optionally(hasAncestor(
+              linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
+          hasParent(decl().bind(ParentDeclName)))
           .bind(TypedefName),
       this);
 
@@ -78,6 +83,11 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
   if (MatchedDecl->getLocation().isInvalid())
     return;
 
+  const auto *ExternCDecl =
+      Result.Nodes.getNodeAs<LinkageSpecDecl>(ExternCDeclName);
+  if (ExternCDecl && IgnoreExternC)
+    return;
+
   SourceLocation StartLoc = MatchedDecl->getBeginLoc();
 
   if (StartLoc.isMacroID() && IgnoreMacros)
@@ -130,7 +140,8 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
       Type = FirstTypedefName + Type.substr(FirstTypedefType.size() + 1);
   }
   if (!ReplaceRange.getEnd().isMacroID()) {
-    const SourceLocation::IntTy Offset = MatchedDecl->getFunctionType() ? 0 : Name.size();
+    const SourceLocation::IntTy Offset =
+        MatchedDecl->getFunctionType() ? 0 : Name.size();
     LastReplacementEnd = ReplaceRange.getEnd().getLocWithOffset(Offset);
   }
 
diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
index 5c741a92d01310..7054778d84a0c6 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -20,6 +20,7 @@ namespace clang::tidy::modernize {
 class UseUsingCheck : public ClangTidyCheck {
 
   const bool IgnoreMacros;
+  const bool IgnoreExternC;
   SourceLocation LastReplacementEnd;
   llvm::DenseMap<const Decl *, SourceRange> LastTagDeclRanges;
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
index c47a022f2c9868..32272a07994c22 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
@@ -45,3 +45,8 @@ Options
 
    If set to `true`, the check will not give warnings inside macros. Default
    is `true`.
+
+.. option:: IgnoreExternC
+
+   If set to `true`, the check will not give warning inside `extern "C"`scope.
+   Default is `false`
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using-ignore-extern-c.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using-ignore-extern-c.cpp
new file mode 100644
index 00000000000000..6a845a0bcc3509
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using-ignore-extern-c.cpp
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s modernize-use-using %t -- -config="{CheckOptions: {modernize-use-using.IgnoreExternC: true}}" -- -I %S/Input/use-using/
+
+// Some Header
+extern "C" {
+
+typedef int NewInt;
+}
+
+extern "C++" {
+
+typedef int InExternCPP;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using InExternCPP = int;
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 0f8f14502d5ca3..462bc984fd3ad5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -330,6 +330,9 @@ typedef class ISSUE_67529_1 *ISSUE_67529;
 extern "C" {
 
 typedef int InExternC;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using InExternC = int;
+
 }
 
 extern "C++" {

>From 1c9feeaf6630f22fef90dc8a8d3831007f7f152c Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 10 Dec 2023 20:03:06 +0000
Subject: [PATCH 9/9] Update: the documentation with the correct information

---
 clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9fedb52966d6fe..32730a07897d39 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -318,8 +318,8 @@ Changes in existing checks
 
 - Improved :doc:`modernize-use-using
   <clang-tidy/checks/modernize/use-using>` check to fix function pointer and
-  forward declared ``typedef`` correctly. Ignore ``typedef`` declaration in
-  ``extern "C"`` scope.
+  forward declared ``typedef`` correctly. Added option `IgnoreExternC` to ignore ``typedef``
+  declaration in ``extern "C"`` scope.
 
 - Improved :doc:`performance-faster-string-find
   <clang-tidy/checks/performance/faster-string-find>` check to properly escape

More information about the llvm-commits mailing list