[PATCH] D78890: [clang-tidy] Add check callee-namespace.
PaulkaToast updated this revision to Diff 260805.
PaulkaToast marked 3 inline comments as done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78890/new/
https://reviews.llvm.org/D78890
Files:
clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.h
clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s llvmlibc-callee-namespace %t
+
+namespace __llvm_libc {
+namespace nested {
+void nested_func() {}
+} // namespace nested
+void libc_api_func() {}
+} // namespace __llvm_libc
+
+// Emulate a function from the public headers like string.h
+void libc_api_func() {}
+
+namespace __llvm_libc {
+void Test() {
+ // Allow calls with the fully qualified name.
+ __llvm_libc::libc_api_func();
+ __llvm_libc::nested::nested_func();
+ void (*qualifiedPtr)(void) = __llvm_libc::libc_api_func;
+ qualifiedPtr();
+
+ // Should not trigger on compiler provided function calls.
+ (void)__builtin_abs(-1);
+
+ // Bare calls are allowed as long as they resolve to the correct namespace.
+ libc_api_func();
+ nested::nested_func();
+ void (*barePtr)(void) = __llvm_libc::libc_api_func;
+ barePtr();
+
+ // Disallow calling into global namespace for implemented entrypoints.
+ ::libc_api_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
+ // CHECK-MESSAGES: :11:6: note: resolves to this declaration
+
+ // Disallow indirect references to functions in global namespace.
+ void (*badPtr)(void) = ::libc_api_func;
+ badPtr();
+ // CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
+ // CHECK-MESSAGES: :11:6: note: resolves to this declaration
+}
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - llvmlibc-callee-namespace
+
+llvmlibc-callee-namespace
+
+
+Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
+.. code-block:: c++
+
+namespace __llvm_libc {
+
+// Allow calls with the fully qualified name.
+__llvm_libc::strlen("hello");
+
+// Allow calls to compiler provided functions.
+(void)__builtin_abs(-1);
+
+// Bare calls are allowed as long as they resolve to the correct namespace.
+strlen("world");
+
+// Disallow calling into functions in the global namespace.
+::strlen("!");
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -188,6 +188,7 @@
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
`llvm-twine-local `_, "Yes"
+ `llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
``memcmp`` and similar derivatives on non-trivial types.
+- New :doc:`llvmlibc-callee-namespace
+ ` check.
+
+ Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
- New :doc:`llvmlibc-implementation-in-namespace
` check.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "CalleeNamespaceCheck.h"
#include "ImplementationInNamespaceCheck.h"
#include
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
PaulkaToast added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:52-53 + + diag(FuncDecl->getLocation(), + "currently resolves to: ", clang::DiagnosticIDs::Note); +} aaron.ballman wrote: > PaulkaToast wrote: > > aaron.ballman wrote: > > > This diagnostic seems a bit strange -- I would expect some text after the > > > colon. > > I was trying mimic the clang's previous definition diagnostic. e.g. : > > https://godbolt.org/z/V4tKr- > > Although the colon does seem to confusing so I removed it. > Ah, thank you for the explanation. I think I would word it `resolves to this > declaration` (or something along those lines) to be a bit less grammatically > ambiguous. When the diagnostic ends in "to", I assume there's a part of the > diagnostic missing and I wonder "to what?" I agree with this, thanks for the insight (: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78890/new/ https://reviews.llvm.org/D78890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8683f5de5352: [clang-tidy] Add check callee-namespace.
(authored by PaulkaToast).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78890/new/
https://reviews.llvm.org/D78890
Files:
clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.h
clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s llvmlibc-callee-namespace %t
+
+namespace __llvm_libc {
+namespace nested {
+void nested_func() {}
+} // namespace nested
+void libc_api_func() {}
+} // namespace __llvm_libc
+
+// Emulate a function from the public headers like string.h
+void libc_api_func() {}
+
+namespace __llvm_libc {
+void Test() {
+ // Allow calls with the fully qualified name.
+ __llvm_libc::libc_api_func();
+ __llvm_libc::nested::nested_func();
+ void (*qualifiedPtr)(void) = __llvm_libc::libc_api_func;
+ qualifiedPtr();
+
+ // Should not trigger on compiler provided function calls.
+ (void)__builtin_abs(-1);
+
+ // Bare calls are allowed as long as they resolve to the correct namespace.
+ libc_api_func();
+ nested::nested_func();
+ void (*barePtr)(void) = __llvm_libc::libc_api_func;
+ barePtr();
+
+ // Disallow calling into global namespace for implemented entrypoints.
+ ::libc_api_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
+ // CHECK-MESSAGES: :11:6: note: resolves to this declaration
+
+ // Disallow indirect references to functions in global namespace.
+ void (*badPtr)(void) = ::libc_api_func;
+ badPtr();
+ // CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
+ // CHECK-MESSAGES: :11:6: note: resolves to this declaration
+}
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - llvmlibc-callee-namespace
+
+llvmlibc-callee-namespace
+
+
+Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
+.. code-block:: c++
+
+namespace __llvm_libc {
+
+// Allow calls with the fully qualified name.
+__llvm_libc::strlen("hello");
+
+// Allow calls to compiler provided functions.
+(void)__builtin_abs(-1);
+
+// Bare calls are allowed as long as they resolve to the correct namespace.
+strlen("world");
+
+// Disallow calling into functions in the global namespace.
+::strlen("!");
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -188,6 +188,7 @@
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
`llvm-twine-local `_, "Yes"
+ `llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
``memcmp`` and similar derivatives on non-trivial types.
+- New :doc:`llvmlibc-callee-namespace
+ ` check.
+
+ Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
- New :doc:`llvmlibc-implementation-in-namespace
` check.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "CalleeNamespaceCheck.h"
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:52-53 + + diag(FuncDecl->getLocation(), + "currently resolves to: ", clang::DiagnosticIDs::Note); +} PaulkaToast wrote: > aaron.ballman wrote: > > This diagnostic seems a bit strange -- I would expect some text after the > > colon. > I was trying mimic the clang's previous definition diagnostic. e.g. : > https://godbolt.org/z/V4tKr- > Although the colon does seem to confusing so I removed it. Ah, thank you for the explanation. I think I would word it `resolves to this declaration` (or something along those lines) to be a bit less grammatically ambiguous. When the diagnostic ends in "to", I assume there's a part of the diagnostic missing and I wonder "to what?" Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:22-24 + if (Decl->getParent() && Decl->getParent()->isTranslationUnit()) +return Decl; + return getOutermostNamespace(Decl->getParent()); Maybe `const DeclContext *Parent = Decl->getParent();` and then use `Parent` in this function? Not critical, but it would simplify things a bit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78890/new/ https://reviews.llvm.org/D78890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
PaulkaToast added inline comments.
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:22-23
+const DeclContext *getOutermostNamespace(const DeclContext *Decl) {
+ if (Decl->isTranslationUnit())
+return Decl;
+ if (Decl->getParent() && Decl->getParent()->isTranslationUnit())
aaron.ballman wrote:
> Under what circumstances could the translation unit be passed in as the
> declaration? I think this code can just be removed.
Actually you are totally right. This case would never occur, thanks (:
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:30-31
+void CalleeNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(functionDecl().bind("func"))).bind("call-site"), this);
+}
aaron.ballman wrote:
> Do you also want to catch binding of function pointers? e.g.,
> ```
> float (*fp)(float) = sinf;
> ```
I hadn't considered function pointers, does seem like a good thing to catch
though. Modified the matches to catch this case as well. Thank you!
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:52-53
+
+ diag(FuncDecl->getLocation(),
+ "currently resolves to: ", clang::DiagnosticIDs::Note);
+}
aaron.ballman wrote:
> This diagnostic seems a bit strange -- I would expect some text after the
> colon.
I was trying mimic the clang's previous definition diagnostic. e.g. :
https://godbolt.org/z/V4tKr-
Although the colon does seem to confusing so I removed it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78890/new/
https://reviews.llvm.org/D78890
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
PaulkaToast updated this revision to Diff 260524.
PaulkaToast marked 8 inline comments as done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78890/new/
https://reviews.llvm.org/D78890
Files:
clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.h
clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s llvmlibc-callee-namespace %t
+
+namespace __llvm_libc {
+namespace nested {
+void nested_func() {}
+} // namespace nested
+void libc_api_func() {}
+} // namespace __llvm_libc
+
+// Emulate a function from the public headers like string.h
+void libc_api_func() {}
+
+namespace __llvm_libc {
+void Test() {
+ // Allow calls with the fully qualified name.
+ __llvm_libc::libc_api_func();
+ __llvm_libc::nested::nested_func();
+ void (*qualifiedPtr)(void) = __llvm_libc::libc_api_func;
+ qualifiedPtr();
+
+ // Should not trigger on compiler provided function calls.
+ (void)__builtin_abs(-1);
+
+ // Bare calls are allowed as long as they resolve to the correct namespace.
+ libc_api_func();
+ nested::nested_func();
+ void (*barePtr)(void) = __llvm_libc::libc_api_func;
+ barePtr();
+
+ // Disallow calling into global namespace for implemented entrypoints.
+ ::libc_api_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
+ // CHECK-MESSAGES: :11:6: note: currently resolves to
+
+ // Disallow indirect references to functions in global namespace.
+ void (*badPtr)(void) = ::libc_api_func;
+ badPtr();
+ // CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
+ // CHECK-MESSAGES: :11:6: note: currently resolves to
+}
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - llvmlibc-callee-namespace
+
+llvmlibc-callee-namespace
+
+
+Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
+.. code-block:: c++
+
+namespace __llvm_libc {
+
+// Allow calls with the fully qualified name.
+__llvm_libc::strlen("hello");
+
+// Allow calls to compiler provided functions.
+(void)__builtin_abs(-1);
+
+// Bare calls are allowed as long as they resolve to the correct namespace.
+strlen("world");
+
+// Disallow calling into functions in the global namespace.
+::strlen("!");
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -188,6 +188,7 @@
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
`llvm-twine-local `_, "Yes"
+ `llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
``memcmp`` and similar derivatives on non-trivial types.
+- New :doc:`llvmlibc-callee-namespace
+ ` check.
+
+ Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
- New :doc:`llvmlibc-implementation-in-namespace
` check.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "CalleeNamespaceCheck.h"
#include "ImplementationInNamespaceCheck.h"
#include "RestrictSystemLibcHeadersCheck.h"
@@
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
aaron.ballman added inline comments.
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:22-23
+const DeclContext *getOutermostNamespace(const DeclContext *Decl) {
+ if (Decl->isTranslationUnit())
+return Decl;
+ if (Decl->getParent() && Decl->getParent()->isTranslationUnit())
Under what circumstances could the translation unit be passed in as the
declaration? I think this code can just be removed.
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:30-31
+void CalleeNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(functionDecl().bind("func"))).bind("call-site"), this);
+}
Do you also want to catch binding of function pointers? e.g.,
```
float (*fp)(float) = sinf;
```
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:43-46
+ const DeclContext *NS = getOutermostNamespace(cast(FuncDecl));
+ if (isa(NS) &&
+ cast(NS)->getName() == "__llvm_libc")
+return;
```
const auto *NS = dyn_cast(getOutermostNamespace(FuncDecl));
if (NS && NS->getName() == "__llvm_libc")
return;
```
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:48-49
+
+ diag(CallsiteExpr->getBeginLoc(), "function %0 must call to internal "
+"implementation in `__llvm_libc`
namespace")
+ << FuncDecl;
How about: `%0 must resolve to a function declared within the '__llvm_libc'
namespace`
Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:52-53
+
+ diag(FuncDecl->getLocation(),
+ "currently resolves to: ", clang::DiagnosticIDs::Note);
+}
This diagnostic seems a bit strange -- I would expect some text after the colon.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78890/new/
https://reviews.llvm.org/D78890
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
PaulkaToast updated this revision to Diff 260218.
PaulkaToast marked 2 inline comments as done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78890/new/
https://reviews.llvm.org/D78890
Files:
clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.h
clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s llvmlibc-callee-namespace %t
+
+namespace __llvm_libc {
+namespace nested {
+void nested_func() {}
+} // namespace nested
+void libc_api_func() {}
+} // namespace __llvm_libc
+
+// Emulate a function from the public headers like string.h
+void libc_api_func() {}
+
+namespace __llvm_libc {
+void Test() {
+ // Allow calls with the fully qualified name.
+ __llvm_libc::libc_api_func();
+ __llvm_libc::nested::nested_func();
+
+ // Should not trigger on compiler provided function calls.
+ (void)__builtin_abs(-1);
+
+ // Bare calls are allowed as long as they resolve to the correct namespace.
+ libc_api_func();
+ nested::nested_func();
+
+ // Disallow calling into global namespace for implemented entrypoints.
+ ::libc_api_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'libc_api_func' must call to internal implementation in `__llvm_libc` namespace
+ // CHECK-MESSAGES: :11:6: note: currently resolves to:
+}
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - llvmlibc-callee-namespace
+
+llvmlibc-callee-namespace
+
+
+Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
+.. code-block:: c++
+
+namespace __llvm_libc {
+
+// Allow calls with the fully qualified name.
+__llvm_libc::strlen("hello");
+
+// Allow calls to compiler provided functions.
+(void)__builtin_abs(-1);
+
+// Bare calls are allowed as long as they resolve to the correct namespace.
+strlen("world");
+
+// Disallow calling into functions in the global namespace.
+::strlen("!");
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -188,6 +188,7 @@
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
`llvm-twine-local `_, "Yes"
+ `llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
``memcmp`` and similar derivatives on non-trivial types.
+- New :doc:`llvmlibc-callee-namespace
+ ` check.
+
+ Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+
- New :doc:`llvmlibc-implementation-in-namespace
` check.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "CalleeNamespaceCheck.h"
#include "ImplementationInNamespaceCheck.h"
#include "RestrictSystemLibcHeadersCheck.h"
@@ -19,6 +20,8 @@
class LLVMLibcModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"llvmlibc-callee-namespace");
CheckFactories.registerCheck(
"llvmlibc-implementation-in-namespace");
CheckFactories.registerCheck(
Index: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.h
===
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:119 + + Checks all calls resolve to functions within __llvm_libc namespace. + Please enclose __llvm_libc in double back-ticks. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst:6 + +Checks all calls resolve to functions within __llvm_libc namespace. + Please enclose __llvm_libc in double back-ticks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78890/new/ https://reviews.llvm.org/D78890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78890: [clang-tidy] Add check callee-namespace.
PaulkaToast created this revision.
PaulkaToast added reviewers: aaron.ballman, njames93.
PaulkaToast added projects: clang-tools-extra, libc-project.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.
This check will ensure that all calls to functions resolve to one inside the
`__llvm_libc` namespace.
This is done to ensure that if we include a public header then we don't
accidentally call into the a function within the global namespace.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78890
Files:
clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.h
clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s llvmlibc-callee-namespace %t
+
+namespace __llvm_libc {
+namespace nested {
+void nested_func() {}
+} // namespace nested
+void libc_api_func() {}
+} // namespace __llvm_libc
+
+// Emulate a function from the public headers like string.h
+void libc_api_func() {}
+
+namespace __llvm_libc {
+void Test() {
+ // Allow calls with the fully qualified name.
+ __llvm_libc::libc_api_func();
+ __llvm_libc::nested::nested_func();
+
+ // Should not trigger on compiler provided function calls.
+ (void)__builtin_abs(-1);
+
+ // Bare calls are allowed as long as they resolve to the correct namespace.
+ libc_api_func();
+ nested::nested_func();
+
+ // Disallow calling into global namespace for implemented entrypoints.
+ ::libc_api_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'libc_api_func' must call to internal implementation in `__llvm_libc` namespace
+ // CHECK-MESSAGES: :11:6: note: currently resolves to:
+}
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc-callee-namespace.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - llvmlibc-callee-namespace
+
+llvmlibc-callee-namespace
+
+
+Checks all calls resolve to functions within __llvm_libc namespace.
+
+.. code-block:: c++
+
+namespace __llvm_libc {
+
+// Allow calls with the fully qualified name.
+__llvm_libc::strlen("hello");
+
+// Allow calls to compiler provided functions.
+(void)__builtin_abs(-1);
+
+// Bare calls are allowed as long as they resolve to the correct namespace.
+strlen("world");
+
+// Disallow calling into functions in the global namespace.
+::strlen("!");
+
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -188,6 +188,7 @@
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
`llvm-twine-local `_, "Yes"
+ `llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
``memcmp`` and similar derivatives on non-trivial types.
+- New :doc:`llvmlibc-callee-namespace
+ ` check.
+
+ Checks all calls resolve to functions within __llvm_libc namespace.
+
- New :doc:`llvmlibc-implementation-in-namespace
` check.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "CalleeNamespaceCheck.h"
#include "ImplementationInNamespaceCheck.h"
#include "RestrictSystemLibcHeadersCheck.h"
@@ -19,6 +20,8 @@
class LLVMLibcModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories ) override {
+
