https://github.com/gchatelet updated https://github.com/llvm/llvm-project/pull/66504
>From f1427a81c4a3425c1a574316fc26d2c74297b34b Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet <gchate...@google.com> Date: Fri, 15 Sep 2023 12:34:17 +0000 Subject: [PATCH 1/4] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules This is the implementation of step 3 of https://discourse.llvm.org/t/rfc-customizable-namespace-to-allow-testing-the-libc-when-the-system-libc-is-also-llvms-libc/73079. --- .../ImplementationInNamespaceCheck.cpp | 21 ++++++++----- .../llvmlibc/implementation-in-namespace.rst | 23 +++++++++----- .../llvmlibc/implementation-in-namespace.cpp | 31 +++++++++++++------ 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp index d05310f09ef773a..69a385f5be9807f 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp @@ -14,7 +14,9 @@ using namespace clang::ast_matchers; namespace clang::tidy::llvm_libc { -const static StringRef RequiredNamespace = "__llvm_libc"; +const static StringRef RequiredNamespaceStart = "__llvm_libc"; +const static StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE"; + void ImplementationInNamespaceCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( decl(hasParent(translationUnitDecl()), unless(linkageSpecDecl())) @@ -29,16 +31,19 @@ void ImplementationInNamespaceCheck::check( if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation())) return; - if (const auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) { - if (NS->getName() != RequiredNamespace) { - diag(NS->getLocation(), "'%0' needs to be the outermost namespace") - << RequiredNamespace; - } + if (auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) { + if (!Result.SourceManager->isMacroBodyExpansion(NS->getLocation())) + diag(NS->getLocation(), + "the outermost namespace should be the '%0' macro") + << RequiredNamespaceMacroName; + else if (!NS->getName().starts_with(RequiredNamespaceStart)) + diag(NS->getLocation(), "the outermost namespace should start with '%0'") + << RequiredNamespaceStart; return; } diag(MatchedDecl->getLocation(), - "declaration must be declared within the '%0' namespace") - << RequiredNamespace; + "declaration must be declared within a namespace starting with '%0'") + << RequiredNamespaceStart; } } // namespace clang::tidy::llvm_libc diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst index 33d6dc8ff125c84..47ea2b866a93404 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst @@ -8,21 +8,30 @@ correct namespace. .. code-block:: c++ - // Correct: implementation inside the correct namespace. - namespace __llvm_libc { + // Implementation inside the LIBC_NAMESPACE namespace. + // Correct if: + // - LIBC_NAMESPACE is a macro + // - LIBC_NAMESPACE expansion starts with `__llvm_libc` + namespace LIBC_NAMESPACE { void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {} - // Namespaces within __llvm_libc namespace are allowed. - namespace inner{ + // Namespaces within LIBC_NAMESPACE namespace are allowed. + namespace inner { int localVar = 0; } // Functions with C linkage are allowed. - extern "C" void str_fuzz(){} + extern "C" void str_fuzz() {} } - // Incorrect: implementation not in a namespace. + // Incorrect: implementation not in the LIBC_NAMESPACE namespace. void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {} - // Incorrect: outer most namespace is not correct. + // Incorrect: outer most namespace is not the LIBC_NAMESPACE macro. namespace something_else { void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {} } + + // Incorrect: outer most namespace expansion does not start with `__llvm_libc`. + #define LIBC_NAMESPACE custom_namespace + namespace LIBC_NAMESPACE { + void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {} + } diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp index e75556a623b655c..16c5f9ca1067ec5 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp @@ -3,18 +3,18 @@ #define MACRO_A "defining macros outside namespace is valid" class ClassB; -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the '__llvm_libc' namespace +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within a namespace starting with '__llvm_libc' struct StructC {}; -// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be declared within the '__llvm_libc' namespace +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be declared within a namespace starting with '__llvm_libc' char *VarD = MACRO_A; -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the '__llvm_libc' namespace +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within a namespace starting with '__llvm_libc' typedef int typeE; -// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be declared within the '__llvm_libc' namespace +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be declared within a namespace starting with '__llvm_libc' void funcF() {} -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be declared within the '__llvm_libc' namespace +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be declared within a namespace starting with '__llvm_libc' namespace namespaceG { -// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: '__llvm_libc' needs to be the outermost namespace +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE' macro namespace __llvm_libc{ namespace namespaceH { class ClassB; @@ -26,9 +26,20 @@ typedef int typeE; void funcF() {} } // namespace namespaceG -// Wrapped in correct namespace. -namespace __llvm_libc { -// Namespaces within __llvim_libc namespace allowed. +// Wrapped in macro namespace but with an incorrect name +#define LIBC_NAMESPACE custom_namespace +namespace LIBC_NAMESPACE { +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should start with '__llvm_libc' +namespace namespaceH { +class ClassB; +} // namespace namespaceH +} // namespace LIBC_NAMESPACE + + +// Wrapped in macro namespace with a valid name, LIBC_NAMESPACE starts with '__llvm_libc' +#undef LIBC_NAMESPACE +#define LIBC_NAMESPACE __llvm_libc_xyz +namespace LIBC_NAMESPACE { namespace namespaceI { class ClassB; } // namespace namespaceI @@ -37,4 +48,4 @@ char *VarD = MACRO_A; typedef int typeE; void funcF() {} extern "C" void extern_funcJ() {} -} // namespace __llvm_libc +} // namespace LIBC_NAMESPACE >From bae9be668c284678818e957272a2906fe12c0808 Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet <gchate...@google.com> Date: Mon, 18 Sep 2023 07:40:49 +0000 Subject: [PATCH 2/4] Address comment --- .../clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp index 69a385f5be9807f..7ac483571b0de6c 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp @@ -31,7 +31,7 @@ void ImplementationInNamespaceCheck::check( if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation())) return; - if (auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) { + if (const auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) { if (!Result.SourceManager->isMacroBodyExpansion(NS->getLocation())) diag(NS->getLocation(), "the outermost namespace should be the '%0' macro") >From 1de5e614826c136b4b107f0835ca37564aee7270 Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet <gchate...@google.com> Date: Mon, 18 Sep 2023 15:03:05 +0000 Subject: [PATCH 3/4] Add entry in release notes --- clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 19c977977f9044c..c541e17ef076a2b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -300,6 +300,11 @@ Changes in existing checks <clang-tidy/checks/readability/static-accessed-through-instance>` check to identify calls to static member functions with out-of-class inline definitions. +- Updated :doc:`llvmlibc-implementation-in-namespace + <clang-tidy/checks/llvmlibc/implementation-in-namespace>` to support + customizable namespace. This further allows for testing the libc when the + system-libc is also LLVM's libc. + Removed checks ^^^^^^^^^^^^^^ >From d0466e640002c9e371c8c258867c8287d431f8cb Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet <gchate...@google.com> Date: Thu, 21 Sep 2023 09:12:22 +0000 Subject: [PATCH 4/4] Address comments --- .../ImplementationInNamespaceCheck.cpp | 18 +++++++++--------- clang-tools-extra/docs/ReleaseNotes.rst | 10 +++++----- .../llvmlibc/implementation-in-namespace.cpp | 17 +++++++++++------ 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp index 7ac483571b0de6c..a953ff626a51ea9 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp @@ -18,32 +18,32 @@ const static StringRef RequiredNamespaceStart = "__llvm_libc"; const static StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE"; void ImplementationInNamespaceCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - decl(hasParent(translationUnitDecl()), unless(linkageSpecDecl())) - .bind("child_of_translation_unit"), - this); + Finder->addMatcher(decl(isExpansionInMainFile(), + hasDeclContext(translationUnitDecl()), + unless(linkageSpecDecl())) + .bind("child_of_translation_unit"), + this); } void ImplementationInNamespaceCheck::check( const MatchFinder::MatchResult &Result) { const auto *MatchedDecl = Result.Nodes.getNodeAs<Decl>("child_of_translation_unit"); - if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation())) - return; if (const auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) { if (!Result.SourceManager->isMacroBodyExpansion(NS->getLocation())) diag(NS->getLocation(), "the outermost namespace should be the '%0' macro") << RequiredNamespaceMacroName; - else if (!NS->getName().starts_with(RequiredNamespaceStart)) + else if (NS->isAnonymousNamespace() || + NS->getName().starts_with(RequiredNamespaceStart) == false) diag(NS->getLocation(), "the outermost namespace should start with '%0'") << RequiredNamespaceStart; return; } diag(MatchedDecl->getLocation(), - "declaration must be declared within a namespace starting with '%0'") - << RequiredNamespaceStart; + "declaration must be declared within the '%0' namespace") + << RequiredNamespaceMacroName; } } // namespace clang::tidy::llvm_libc diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c541e17ef076a2b..4793728bc56c6d4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -231,6 +231,11 @@ Changes in existing checks <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for ``inline`` namespaces in the same format as :program:`clang-format`. +- Improved :doc:`llvmlibc-implementation-in-namespace + <clang-tidy/checks/llvmlibc/implementation-in-namespace>` to support + customizable namespace. This further allows for testing the libc when the + system-libc is also LLVM's libc. + - Improved :doc:`misc-include-cleaner <clang-tidy/checks/misc/include-cleaner>` check by adding option `DeduplicateFindings` to output one finding per symbol occurrence. @@ -300,11 +305,6 @@ Changes in existing checks <clang-tidy/checks/readability/static-accessed-through-instance>` check to identify calls to static member functions with out-of-class inline definitions. -- Updated :doc:`llvmlibc-implementation-in-namespace - <clang-tidy/checks/llvmlibc/implementation-in-namespace>` to support - customizable namespace. This further allows for testing the libc when the - system-libc is also LLVM's libc. - Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp index 16c5f9ca1067ec5..3f5000f9cc5bbd1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp @@ -3,19 +3,24 @@ #define MACRO_A "defining macros outside namespace is valid" class ClassB; -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within a namespace starting with '__llvm_libc' +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the 'LIBC_NAMESPACE' namespace struct StructC {}; -// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be declared within a namespace starting with '__llvm_libc' +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be declared within the 'LIBC_NAMESPACE' namespace char *VarD = MACRO_A; -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within a namespace starting with '__llvm_libc' +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the 'LIBC_NAMESPACE' namespace typedef int typeE; -// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be declared within a namespace starting with '__llvm_libc' +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be declared within the 'LIBC_NAMESPACE' namespace void funcF() {} -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be declared within a namespace starting with '__llvm_libc' +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be declared within the 'LIBC_NAMESPACE' namespace + +namespace outer_most { +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE' macro + class A {}; +} namespace namespaceG { // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE' macro -namespace __llvm_libc{ +namespace __llvm_libc { namespace namespaceH { class ClassB; } // namespace namespaceH _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
