Fix #35272: Don't replace typedefs in extern c scope

[da-viper]

Fixes #35272

[llvmbot]

@llvm/pr-subscribers-clang-tidy

Author: None (Da-Viper)

Changes

Fixes #35272

---

Full diff: https://github.com/llvm/llvm-project/pull/69102.diff

2 Files Affected:

• (modified) clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp (+12-4)
• (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp (+14)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index e6293ed48bfddbb..841ffb4c9bfe66e 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 422abee11a71962..0f8f14502d5ca3c 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;
+
+}

[HerrCai0907]

please update release note and doc

[PiotrZSL]

I got mixed feelings about this. Simply we can use 'using' within extern "C", there is nothing wrong with this. using/typedef doesn't haven linkage, that just type alias. And extern "C" does not impact it. In oryginal issue I think someone mistook extern "C" into something where only "C" code should be put, and that's not true. All what extern "C" does it's using C style symbol mangling for functions inside.

The only reason to not use 'using' in extern 'C' would be just readability, to avoid some confusion.
As for this change, release notes & doc is needed.

[da-viper]

Ok, I do have a few questions. 1. Currently this check does not match typedefs in functions regardless of extern c.should this be the case. 2. where do I find the release notes.

…

On Sun, 15 Oct 2023, 13:36 Piotr Zegar, ***@***.***> wrote: I got mixed feelings about this. Simply we can use 'using' within extern "C", there is nothing wrong with this. using/typedef doesn't haven linkage, that just type alias. And extern "C" does not impact it. In oryginal issue I think someone mistook extern "C" into something where only "C" code should be put, and that's not true. All what extern "C" does it's using C style symbol mangling for functions inside. The only reason to not use 'using' in extern 'C' would be just readability, to avoid some confusion. As for this change, release notes & doc is needed. — Reply to this email directly, view it on GitHub <#69102 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AN2DXISRXSJCEEZ7OEU2DTLX7PKDTAVCNFSM6AAAAAA6A5MI3OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRTGM3TMMRXHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[PiotrZSL]

2. where do I find the release notes.

clang-tools-extra/docs/ReleaseNotes.rst

[PiotrZSL]

Loooks, good, minor nit in release notes.