-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-tidy][use-internal-linkage]fix false positives for ExportDecl #117901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang-tidy][use-internal-linkage]fix false positives for ExportDecl #117901
Conversation
C++20 modules supports to declare external linkage by `ExportDecl`.
@llvm/pr-subscribers-clang-tidy Author: Congcong Cai (HerrCai0907) Changes
Full diff: https://github.com/llvm/llvm-project/pull/117901.diff 7 Files Affected:
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;
|
@llvm/pr-subscribers-clang-tools-extra Author: Congcong Cai (HerrCai0907) Changes
Full diff: https://github.com/llvm/llvm-project/pull/117901.diff 7 Files Affected:
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;
|
@llvm/pr-subscribers-clang Author: Congcong Cai (HerrCai0907) Changes
Full diff: https://github.com/llvm/llvm-project/pull/117901.diff 7 Files Affected:
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;
|
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the matcher to https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Registry.cpp as well (clang-query).
clang/docs/ReleaseNotes.rst
Outdated
@@ -926,6 +926,8 @@ AST Matchers | |||
- Ensure ``hasName`` matches template specializations across inline namespaces, | |||
making `matchesNodeFullSlow` and `matchesNodeFullFast` consistent. | |||
|
|||
- Add ``exportDecl`` matches export declaration. |
There was a problem hiding this comment.
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.
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 |
Fixed: #97190