[PATCH] D43462: [clangd] Fixes for #include insertion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clangd/Headers.h:26
 ///
 /// \param Header is an absolute file path.
+/// \return A quoted "path" or <path>. This returns an empty string if:
----------------
File also needs to be absolute.
(May want to add asserts for this at the start of the impl - up to you)


================
Comment at: clangd/index/FileIndex.cpp:36
+  CollectorOpts.IndexMainFiles = false;
+  CollectorOpts.CollectIncludePath = true;
+  const auto *Includes = CanonicalIncludesForSystemHeaders();
----------------
if this is always true now, we could remove the option?


================
Comment at: clangd/index/FileIndex.cpp:37
+  CollectorOpts.CollectIncludePath = true;
+  const auto *Includes = CanonicalIncludesForSystemHeaders();
+  CollectorOpts.Includes = Includes;
----------------
This is not going to handle IWYU right?
It seems like at this point the right thing to do is totally hide the CanonicalIncludes inside SymbolCollector, which would init it (to system headers), write IWYU mappings using the PP, and consume the mappings (when emitting symbols).
Churn :(


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43462