[PATCH] D78890: [clang-tidy] Add check callee-namespace.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 27 14:34:22 PDT 2020
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<DeclContext>(FuncDecl));
+ if (isa<NamespaceDecl>(NS) &&
+ cast<NamespaceDecl>(NS)->getName() == "__llvm_libc")
+ return;
----------------
```
const auto *NS = dyn_cast<NamespaceDecl>(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
More information about the cfe-commits
mailing list