[PATCH] D78890: [clang-tidy] Add check callee-namespace.

[Paula Toth via Phabricator via cfe-commits]

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