[clang-tidy] Don't replace typedefs in extern c scope (#69102)

Added IgnoreExternC option to modernize-use-using check. Fixes #35272
llvm · Dec 26, 2023 · 9dcc665 · 9dcc665
1 parent 1d76692
commit 9dcc665
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 7 deletions.
diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -11,26 +11,39 @@
 #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 {
 
+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(isInstantiated()),
-                                 hasParent(decl().bind(ParentDeclName)))
-                         .bind(TypedefName),
-                     this);
+  Finder->addMatcher(
+      typedefDecl(
+          unless(isInstantiated()),
+          optionally(hasAncestor(
+              linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
+          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.
@@ -70,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)
@@ -122,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
@@ -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/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -408,7 +408,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.
+  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

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
@@ -28,6 +28,14 @@ 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
@@ -37,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`
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
@@ -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
@@ -325,3 +325,20 @@ 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;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using InExternC = int;
+
+}
+
+extern "C++" {
+
+typedef int InExternCPP;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
+// CHECK-FIXES: using InExternCPP = int;
+
+}