ioeric added inline comments.
Herald added a project: clang.
================
Comment at: clangd/ClangdUnit.cpp:100
+ : File(File), ParsedCallback(ParsedCallback),
+ IWYUHandler(collectIWYUHeaderMaps(&CanonIncludes)) {}
----------------
Does this have to own the `IWYUHandler`? Could we create one when
`getCommentHandler` is called?
================
Comment at: clangd/ClangdUnit.cpp:125
+ CommentHandler *getCommentHandler() override { return IWYUHandler.get(); }
+
----------------
This looks like a memory leak? `release()`?
================
Comment at: clangd/ClangdUnit.cpp:338
+ // Do we really care about IWYU pragmas inside the file rather than the
+ // preamble?
----------------
I think so? A header file (with IWYU pragma) can also be the main file.
================
Comment at: unittests/clangd/CodeCompleteTests.cpp:2287
+TEST(CompletionTest, DynamicIndexIncludeInsertion) {
+ MockFSProvider FS;
----------------
nit: maybe put this close to test cases that involve preamble. e.g.
IndexSuppressesPreambleCompletions
================
Comment at: unittests/clangd/CodeCompleteTests.cpp:2301
+ )cpp";
+ constexpr const char *FileContent(R"cpp(
+ #include "foo_header.h"
----------------
nit: just use `std::string` for readability.
================
Comment at: unittests/clangd/FileIndexTests.cpp:207
+ M, "f",
+ "// IWYU pragma: private, include <the/good/header.h>\nclass string
{};");
----------------
Are we sure the comment will be included in the preamble if there is no
#include block?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57508/new/
https://reviews.llvm.org/D57508
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
