Skip to content

Conversation

@HerrCai0907
Copy link
Contributor

@HerrCai0907 HerrCai0907 commented Nov 27, 2024

Fixed: #97190

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clang-tidy labels Nov 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes
  • [ast-matcher] add exportDecl matcher
  • [clang-tidy][use-internal-linkage]fix false positives for ExportDecl

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

7 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp (+1-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst (+5)
  • (added) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp (+20)
  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+11)
  • (modified) clang/lib/ASTMatchers/ASTMatchersInternal.cpp (+1)
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index 71eb2d94cd4f26..0656238a0e6767 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -101,7 +101,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
                 // 3. template
                 isExplicitTemplateSpecialization(),
                 // 4. friend
-                hasAncestor(friendDecl()))));
+                hasAncestor(decl(anyOf(friendDecl(), exportDecl()))))));
   Finder->addMatcher(
       functionDecl(Common, hasBody(), unless(cxxMethodDecl()), unless(isMain()))
           .bind("fn"),
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f050391110385e..42b1e4e9fd1fe4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -237,7 +237,8 @@ Changes in existing checks
 - Improved :doc:`misc-use-internal-linkage
   <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static``
   keyword before type qualifiers such as ``const`` and ``volatile`` and fix
-  false positives for function declaration without body.
+  false positives for function declaration without body and fix false positives
+  for C++20 export declarations.
 
 - Improved :doc:`modernize-avoid-c-arrays
   <clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
index b8bbcc62706101..508b0cac09a912 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
@@ -29,6 +29,11 @@ Example:
   void fn3(); // without function body in all declaration, maybe external linkage
   void fn3();
 
+  // export declarations
+  export void fn4() {}
+  export namespace t { void fn5() {} }
+  export int v2;
+
 Options
 -------
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp
new file mode 100644
index 00000000000000..9b0d8cde429b6b
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy -std=c++20 %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
+
+module;
+
+export module test;
+
+export void single_export_fn() {}
+export int single_export_var;
+
+export {
+  void group_export_fn1() {}
+  void group_export_fn2() {}
+  int group_export_var1;
+  int group_export_var2;
+}
+
+export namespace aa {
+void namespace_export_fn() {}
+int namespace_export_var;
+} // namespace aa
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 356b091c13af4f..4dfbd03d7756f2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -926,6 +926,8 @@ AST Matchers
 - Ensure ``hasName`` matches template specializations across inline namespaces,
   making `matchesNodeFullSlow` and `matchesNodeFullFast` consistent.
 
+- Add ``exportDecl`` matches export declaration.
+
 clang-format
 ------------
 
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 4bcaa953a61af2..efad600a3c58cd 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1236,6 +1236,17 @@ AST_MATCHER_P(TemplateArgument, equalsIntegralValue,
 extern const internal::VariadicDynCastAllOfMatcher<Stmt,
        ObjCAutoreleasePoolStmt> autoreleasePoolStmt;
 
+/// Matches any export declaration.
+///
+/// Example matches following declarations.
+/// \code
+///   export void foo();
+///   export { void foo(); }
+///   export namespace { void foo(); }
+///   export int v;
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher<Decl, ExportDecl> exportDecl;
+
 /// Matches any value declaration.
 ///
 /// Example matches A, B, C and F
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 46dd44e6f2b24f..cdbdb65195409f 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -799,6 +799,7 @@ const internal::VariadicDynCastAllOfMatcher<TypeLoc, ElaboratedTypeLoc>
 
 const internal::VariadicDynCastAllOfMatcher<Stmt, UnaryExprOrTypeTraitExpr>
     unaryExprOrTypeTraitExpr;
+const internal::VariadicDynCastAllOfMatcher<Decl, ExportDecl> exportDecl;
 const internal::VariadicDynCastAllOfMatcher<Decl, ValueDecl> valueDecl;
 const internal::VariadicDynCastAllOfMatcher<Decl, CXXConstructorDecl>
     cxxConstructorDecl;

@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

Changes
  • [ast-matcher] add exportDecl matcher
  • [clang-tidy][use-internal-linkage]fix false positives for ExportDecl

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

7 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp (+1-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst (+5)
  • (added) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp (+20)
  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+11)
  • (modified) clang/lib/ASTMatchers/ASTMatchersInternal.cpp (+1)
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index 71eb2d94cd4f26..0656238a0e6767 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -101,7 +101,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
                 // 3. template
                 isExplicitTemplateSpecialization(),
                 // 4. friend
-                hasAncestor(friendDecl()))));
+                hasAncestor(decl(anyOf(friendDecl(), exportDecl()))))));
   Finder->addMatcher(
       functionDecl(Common, hasBody(), unless(cxxMethodDecl()), unless(isMain()))
           .bind("fn"),
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f050391110385e..42b1e4e9fd1fe4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -237,7 +237,8 @@ Changes in existing checks
 - Improved :doc:`misc-use-internal-linkage
   <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static``
   keyword before type qualifiers such as ``const`` and ``volatile`` and fix
-  false positives for function declaration without body.
+  false positives for function declaration without body and fix false positives
+  for C++20 export declarations.
 
 - Improved :doc:`modernize-avoid-c-arrays
   <clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
index b8bbcc62706101..508b0cac09a912 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
@@ -29,6 +29,11 @@ Example:
   void fn3(); // without function body in all declaration, maybe external linkage
   void fn3();
 
+  // export declarations
+  export void fn4() {}
+  export namespace t { void fn5() {} }
+  export int v2;
+
 Options
 -------
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp
new file mode 100644
index 00000000000000..9b0d8cde429b6b
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy -std=c++20 %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
+
+module;
+
+export module test;
+
+export void single_export_fn() {}
+export int single_export_var;
+
+export {
+  void group_export_fn1() {}
+  void group_export_fn2() {}
+  int group_export_var1;
+  int group_export_var2;
+}
+
+export namespace aa {
+void namespace_export_fn() {}
+int namespace_export_var;
+} // namespace aa
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 356b091c13af4f..4dfbd03d7756f2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -926,6 +926,8 @@ AST Matchers
 - Ensure ``hasName`` matches template specializations across inline namespaces,
   making `matchesNodeFullSlow` and `matchesNodeFullFast` consistent.
 
+- Add ``exportDecl`` matches export declaration.
+
 clang-format
 ------------
 
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 4bcaa953a61af2..efad600a3c58cd 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1236,6 +1236,17 @@ AST_MATCHER_P(TemplateArgument, equalsIntegralValue,
 extern const internal::VariadicDynCastAllOfMatcher<Stmt,
        ObjCAutoreleasePoolStmt> autoreleasePoolStmt;
 
+/// Matches any export declaration.
+///
+/// Example matches following declarations.
+/// \code
+///   export void foo();
+///   export { void foo(); }
+///   export namespace { void foo(); }
+///   export int v;
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher<Decl, ExportDecl> exportDecl;
+
 /// Matches any value declaration.
 ///
 /// Example matches A, B, C and F
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 46dd44e6f2b24f..cdbdb65195409f 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -799,6 +799,7 @@ const internal::VariadicDynCastAllOfMatcher<TypeLoc, ElaboratedTypeLoc>
 
 const internal::VariadicDynCastAllOfMatcher<Stmt, UnaryExprOrTypeTraitExpr>
     unaryExprOrTypeTraitExpr;
+const internal::VariadicDynCastAllOfMatcher<Decl, ExportDecl> exportDecl;
 const internal::VariadicDynCastAllOfMatcher<Decl, ValueDecl> valueDecl;
 const internal::VariadicDynCastAllOfMatcher<Decl, CXXConstructorDecl>
     cxxConstructorDecl;

@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-clang

Author: Congcong Cai (HerrCai0907)

Changes
  • [ast-matcher] add exportDecl matcher
  • [clang-tidy][use-internal-linkage]fix false positives for ExportDecl

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

7 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp (+1-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst (+5)
  • (added) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp (+20)
  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+11)
  • (modified) clang/lib/ASTMatchers/ASTMatchersInternal.cpp (+1)
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index 71eb2d94cd4f26..0656238a0e6767 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -101,7 +101,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
                 // 3. template
                 isExplicitTemplateSpecialization(),
                 // 4. friend
-                hasAncestor(friendDecl()))));
+                hasAncestor(decl(anyOf(friendDecl(), exportDecl()))))));
   Finder->addMatcher(
       functionDecl(Common, hasBody(), unless(cxxMethodDecl()), unless(isMain()))
           .bind("fn"),
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f050391110385e..42b1e4e9fd1fe4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -237,7 +237,8 @@ Changes in existing checks
 - Improved :doc:`misc-use-internal-linkage
   <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static``
   keyword before type qualifiers such as ``const`` and ``volatile`` and fix
-  false positives for function declaration without body.
+  false positives for function declaration without body and fix false positives
+  for C++20 export declarations.
 
 - Improved :doc:`modernize-avoid-c-arrays
   <clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
index b8bbcc62706101..508b0cac09a912 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
@@ -29,6 +29,11 @@ Example:
   void fn3(); // without function body in all declaration, maybe external linkage
   void fn3();
 
+  // export declarations
+  export void fn4() {}
+  export namespace t { void fn5() {} }
+  export int v2;
+
 Options
 -------
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp
new file mode 100644
index 00000000000000..9b0d8cde429b6b
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy -std=c++20 %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
+
+module;
+
+export module test;
+
+export void single_export_fn() {}
+export int single_export_var;
+
+export {
+  void group_export_fn1() {}
+  void group_export_fn2() {}
+  int group_export_var1;
+  int group_export_var2;
+}
+
+export namespace aa {
+void namespace_export_fn() {}
+int namespace_export_var;
+} // namespace aa
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 356b091c13af4f..4dfbd03d7756f2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -926,6 +926,8 @@ AST Matchers
 - Ensure ``hasName`` matches template specializations across inline namespaces,
   making `matchesNodeFullSlow` and `matchesNodeFullFast` consistent.
 
+- Add ``exportDecl`` matches export declaration.
+
 clang-format
 ------------
 
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 4bcaa953a61af2..efad600a3c58cd 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1236,6 +1236,17 @@ AST_MATCHER_P(TemplateArgument, equalsIntegralValue,
 extern const internal::VariadicDynCastAllOfMatcher<Stmt,
        ObjCAutoreleasePoolStmt> autoreleasePoolStmt;
 
+/// Matches any export declaration.
+///
+/// Example matches following declarations.
+/// \code
+///   export void foo();
+///   export { void foo(); }
+///   export namespace { void foo(); }
+///   export int v;
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher<Decl, ExportDecl> exportDecl;
+
 /// Matches any value declaration.
 ///
 /// Example matches A, B, C and F
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 46dd44e6f2b24f..cdbdb65195409f 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -799,6 +799,7 @@ const internal::VariadicDynCastAllOfMatcher<TypeLoc, ElaboratedTypeLoc>
 
 const internal::VariadicDynCastAllOfMatcher<Stmt, UnaryExprOrTypeTraitExpr>
     unaryExprOrTypeTraitExpr;
+const internal::VariadicDynCastAllOfMatcher<Decl, ExportDecl> exportDecl;
 const internal::VariadicDynCastAllOfMatcher<Decl, ValueDecl> valueDecl;
 const internal::VariadicDynCastAllOfMatcher<Decl, CXXConstructorDecl>
     cxxConstructorDecl;

@HerrCai0907 HerrCai0907 changed the title users/ccc/fix/module use internal linkage [clang-tidy][use-internal-linkage]fix false positives for ExportDecl Nov 27, 2024
Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation & Test for matcher is missing.
Except that everything looks ok.

@HerrCai0907 HerrCai0907 requested a review from PiotrZSL November 27, 2024 23:15
Copy link
Contributor

@5chmidti 5chmidti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Ensure ``hasName`` matches template specializations across inline namespaces,
making `matchesNodeFullSlow` and `matchesNodeFullFast` consistent.

- Add ``exportDecl`` matches export declaration.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a exportDecl matcher to match export declarations.

@HerrCai0907
Copy link
Contributor Author

Please add the matcher to https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Registry.cpp as well (clang-query).

i will do it separately to avoid this pr become bigger.

@5chmidti
Copy link
Contributor

Please add the matcher to https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Registry.cpp as well (clang-query).

i will do it separately to avoid this pr become bigger.

I think you only have to add it into that file for it to work, and IMO a single PR for this change would be better. But as long as it gets added, I'm fine with it

@HerrCai0907 HerrCai0907 merged commit 010317e into llvm:main Dec 2, 2024
9 checks passed
@HerrCai0907 HerrCai0907 deleted the users/ccc/fix/module-use-internal-linkage branch December 2, 2024 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category clang-tidy clang-tools-extra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

misc-use-internal-linkage warns on exported functions

4 participants