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
