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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to