[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clangd/ClangdServer.cpp:418
+  } else {
+    auto U = URI::parse(HeaderUri);
+    if (!U)
----------------
Pull out a function to compute ToInclude from a uri/abspath?


================
Comment at: clangd/ClangdServer.cpp:425
+
+    auto FS = FSProvider.getTaggedFileSystem(File).Value;
+    tooling::CompileCommand CompileCommand =
----------------
Need high-level comments explaining what we're doing to determine the right include path.


================
Comment at: clangd/ClangdServer.cpp:433
+      Argv.push_back(S.c_str());
+    llvm::errs() << "~~~ Build dir:" << CompileCommand.Directory << "\n";
+    IgnoringDiagConsumer IgnoreDiags;
----------------
temporary logs?


================
Comment at: clangd/ClangdServer.cpp:445
+    CI->getFrontendOpts().DisableFree = false;
+    CI->getPreprocessorOpts().SingleFileParseMode = true;
+
----------------
explain why? (this has implications for direct vs transitive includes I think)


================
Comment at: clangd/ClangdServer.cpp:447
+
+    auto Clang = prepareCompilerInstance(
+        std::move(CI), /*Preamble=*/nullptr,
----------------
hmm, why are we actually going to run the compiler/preprocessor?
It looks like we just get HeaderMapping out - can we "just" call ApplyHeaderSearchOptions instead?


================
Comment at: clangd/ClangdServer.cpp:465
+    auto &HeaderSearchInfo = Clang->getPreprocessor().getHeaderSearchInfo();
+    std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics(
+        *Resolved, CompileCommand.Directory);
----------------
do we handle the case that the suggestion is already included?
(including the case where it's included by a different name)


================
Comment at: clangd/ClangdServer.cpp:468
+
+    llvm::errs() << "Suggested #include is: " << Suggested << "\n";
+    ToInclude = "\"" + Suggested + "\"";
----------------
leftover?


================
Comment at: clangd/ClangdServer.cpp:490
+    llvm::consumeError(Style.takeError());
+    // FIXME(ioeric): support fallback style in clangd server.
+    Style = format::getLLVMStyle();
----------------
maybe this fixme should just be to have more consistent style support in clangdserver?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640