[PATCH] D78890: [clang-tidy] Add check callee-namespace.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 28 11:17:29 PDT 2020
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
More information about the cfe-commits
mailing list