sammccall added a comment.
This looks sensible to me, but as I wrote this code we should maybe get a
second look (@kadircet?) that it makes sense to lift to clang::tooling.
Some positive experience: we've used it successfully in an internal clang-tidy
check.
================
Comment at: clang-tools-extra/clangd/unittests/HeadersTests.cpp:412
-TEST(StdlibTest, All) {
- auto VectorH = stdlib::Header::named("<vector>");
- EXPECT_TRUE(VectorH);
- EXPECT_EQ(llvm::to_string(*VectorH), "<vector>");
- EXPECT_FALSE(stdlib::Header::named("HeadersTests.cpp"));
-
- auto Vector = stdlib::Symbol::named("std::", "vector");
- EXPECT_TRUE(Vector);
- EXPECT_EQ(llvm::to_string(*Vector), "std::vector");
- EXPECT_FALSE(stdlib::Symbol::named("std::", "dongle"));
- EXPECT_FALSE(stdlib::Symbol::named("clang::", "ASTContext"));
-
- EXPECT_EQ(Vector->header(), *VectorH);
- EXPECT_THAT(Vector->headers(), ElementsAre(*VectorH));
-}
-
TEST(StdlibTest, Recognizer) {
auto TU = TestTU::withCode(R"cpp(
----------------
Recognizer test also needs to be moved
================
Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:1
+#include "clang/AST/Decl.h"
+#include "llvm/ADT/Optional.h"
----------------
This needs a copyright header and a file comment
================
Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:7
+namespace clang {
+namespace stdlib {
+
----------------
namespace tooling { namespace stdlib {
================
Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:13
+// Lightweight class, in fact just an index into a table.
+class Header {
+public:
----------------
what's the design around C vs C++?
e.g. are `std::printf` and `::printf` distinct symbols, are C `printf` and C++
`::printf` distinct symbols, similarly for headers.
(Fine if only C++ is implemented here, but the interface should probably say
one way or the other)
In clangd we had a handle on all the callers, but here we have to worry about
acquiring callers that e.g. can't easily provide language information.
================
Comment at: clang/lib/Tooling/Inclusions/StandardLibrary.cpp:110
+ return namespaceSymbols(Parent);
+ return NamespaceSymbols->lookup((D->getName() + "::").str());
+ }();
----------------
if D is `std::chrono` I think it just returns "chrono"?
(we should probably have a Recognizer test for this, sorry I left it out...)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119130/new/
https://reviews.llvm.org/D119130
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
