[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/gchatelet closed https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -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 ) {
const auto *MatchedDecl =
Result.Nodes.getNodeAs("child_of_translation_unit");
- if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation()))
-return;
if (const auto *NS = dyn_cast(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() ||
gchatelet wrote:
Yeah anonymous namespaces are not allowed.
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
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
Date: Fri, 15 Sep 2023 12:34:17 +
Subject: [PATCH 1/6] [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(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+ if (auto *NS = dyn_cast(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
@@
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL resolved https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -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 ) {
const auto *MatchedDecl =
Result.Nodes.getNodeAs("child_of_translation_unit");
- if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation()))
-return;
if (const auto *NS = dyn_cast(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() ||
PiotrZSL wrote:
shouldn't we exclude anonymous namespaces at all ? or they not allowed ?
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -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(),
PiotrZSL wrote:
On a hand, we could try oposite:
```
Finder->addMatcher(translationUnitDecl(forEach(decl(isExpansionInMainFile(),
unless(linkageSpecDecl()))
.bind("child_of_translation_unit"))),
this);
```
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
gchatelet wrote: > As a single small change to make hardcoded namespaces configurable looks > fine. I added some comments related to overall issues in this check. Fell > free to fix them or ignore them. Thx for the review! I had to restructure the code a bit to accommodate for anonymous namespaces (they generate an implict UsingDirectiveDecl that was triggering the warning twice). Let me know what you think. https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -30,15 +32,18 @@ void ImplementationInNamespaceCheck::check( return; gchatelet wrote: Done https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -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'
gchatelet wrote:
done
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
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
Date: Fri, 15 Sep 2023 12:34:17 +
Subject: [PATCH 1/5] [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(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+ if (auto *NS = dyn_cast(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
@@
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -30,15 +32,18 @@ void ImplementationInNamespaceCheck::check(
return;
if (const auto *NS = dyn_cast(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+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(),
gchatelet wrote:
This is the case where we are not in any namespace but the message is
misleading. Thx for pointing this out. I'll fix it shortly.
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
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
Date: Fri, 15 Sep 2023 12:34:17 +
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(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+ if (auto *NS = dyn_cast(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
@@
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL approved this pull request. As a single small change to make hardcoded namespaces configurable looks fine. I added some comments related to overall issues in this check. Fell free to fix them or ignore them. https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -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()))
PiotrZSL wrote:
Maybe: hasParent -> hasDeclContext(translationUnitDecl())
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -30,15 +32,18 @@ void ImplementationInNamespaceCheck::check( return; PiotrZSL wrote: this could be replaced with `decl(isExpansionInMainFile())` https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -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'
PiotrZSL wrote:
add test:
```
namespace outer_most {
class A {}; // i think it wont provide warning 'declaration must be declared
within a namespace starting with ' to A class
}
```
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -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{
PiotrZSL wrote:
this looks to be invalid as its namespaceG::__llvm_libc and it's not "the
outermost namespace should be the 'LIBC_NAMESPACE'"
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -30,15 +32,18 @@ void ImplementationInNamespaceCheck::check(
return;
if (const auto *NS = dyn_cast(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+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(),
PiotrZSL wrote:
This looks to be triggered even if declaration is declared with
RequiredNamespaceStart.
I'm blind or i'm not see a check for a RequiredNamespaceStart .
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -30,15 +32,18 @@ void ImplementationInNamespaceCheck::check(
return;
if (const auto *NS = dyn_cast(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+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))
PiotrZSL wrote:
getName will assert if Namespace got not identifier. What about anonymous
namespaces ? Or declarations without name ?
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -300,6 +300,11 @@ Changes in existing checks ` check to identify calls to static member functions with out-of-class inline definitions. +- Updated :doc:`llvmlibc-implementation-in-namespace PiotrZSL wrote: Put changes in alphabetical order. use 'Improved' instead of 'Updated' (only because it would look nice). https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
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
Date: Fri, 15 Sep 2023 12:34:17 +
Subject: [PATCH 1/3] [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(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+ if (auto *NS = dyn_cast(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
@@
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
PiotrZSL wrote: > Where would that be? `clang-tools-extra/docs/ReleaseNotes.rst`? I can't find > what the current release notes look like for head. Yes, it's deployed here: https://clang.llvm.org/extra/ReleaseNotes.html https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
gchatelet wrote: > Should this change be reflected in the Release Notes? Where would that be? `clang-tools-extra/docs/ReleaseNotes.rst`? I can't find what the current release notes look like for head. https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
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
Date: Fri, 15 Sep 2023 12:34:17 +
Subject: [PATCH 1/2] [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(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+ if (auto *NS = dyn_cast(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
@@
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/carlosgalvezp commented: Should this change be reflected in the Release Notes? https://github.com/llvm/llvm-project/pull/66504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
@@ -29,16 +31,19 @@ void ImplementationInNamespaceCheck::check(
if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation()))
return;
- if (const auto *NS = dyn_cast(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+ if (auto *NS = dyn_cast(MatchedDecl)) {
EugeneZelenko wrote:
`const auto *`
https://github.com/llvm/llvm-project/pull/66504
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Changes
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.
--
Full diff: https://github.com/llvm/llvm-project/pull/66504.diff
3 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
(+13-8)
- (modified)
clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst
(+16-7)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp
(+21-10)
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 = quot;__llvm_libcquot;;
+const static StringRef RequiredNamespaceStart =
quot;__llvm_libcquot;;
+const static StringRef RequiredNamespaceMacroName =
quot;LIBC_NAMESPACEquot;;
+
void ImplementationInNamespaceCheck::registerMatchers(MatchFinder *Finder) {
Finder-gt;addMatcher(
decl(hasParent(translationUnitDecl()), unless(linkageSpecDecl()))
@@ -29,16 +31,19 @@ void ImplementationInNamespaceCheck::check(
if
(!Result.SourceManager-gt;isInMainFile(MatchedDecl-gt;getLocation()))
return;
- if (const auto *NS = dyn_castlt;NamespaceDeclgt;(MatchedDecl)) {
-if (NS-gt;getName() != RequiredNamespace) {
- diag(NS-gt;getLocation(), quot;#x27;%0#x27; needs to
be the outermost namespacequot;)
- lt;lt; RequiredNamespace;
-}
+ if (auto *NS = dyn_castlt;NamespaceDeclgt;(MatchedDecl)) {
+if
(!Result.SourceManager-gt;isMacroBodyExpansion(NS-gt;getLocation()))
+ diag(NS-gt;getLocation(),
+ quot;the outermost namespace should be the
#x27;%0#x27; macroquot;)
+ lt;lt; RequiredNamespaceMacroName;
+else if (!NS-gt;getName().starts_with(RequiredNamespaceStart))
+ diag(NS-gt;getLocation(), quot;the outermost namespace should
start with #x27;%0#x27;quot;)
+ lt;lt; RequiredNamespaceStart;
return;
}
diag(MatchedDecl-gt;getLocation(),
- quot;declaration must be declared within the
#x27;%0#x27; namespacequot;)
- lt;lt; RequiredNamespace;
+ quot;declaration must be declared within a namespace starting with
#x27;%0#x27;quot;)
+ lt;lt; 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 quot;Cquot; void str_fuzz(){}
+extern quot;Cquot; 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
---
[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
https://github.com/gchatelet created
https://github.com/llvm/llvm-project/pull/66504
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.
>From f1427a81c4a3425c1a574316fc26d2c74297b34b Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet
Date: Fri, 15 Sep 2023 12:34:17 +
Subject: [PATCH] [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(MatchedDecl)) {
-if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
-}
+ if (auto *NS = dyn_cast(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
---
