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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to