[PATCH] D54817: [clangd] Put direct headers into srcs section.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Going to leave awkward comments suggesting we expand the scope a bit.
Full disclosure: file dependency information is something that's come up as useful in lots of contexts.



================
Comment at: clangd/index/Serialization.cpp:469
 
+  // There's no point in putting headers without digest of the source file.
+  // Because they will only be needed whenever we have an up-to-date index file.
----------------
This seems like unneccesary coupling between layers. On the contrary, I think it's going to be really useful to have file and dependency information in the index data and in the in-memory index.
(e.g. for #include completion, more accurately guessing compile commands for headers, deciding which CC file corresponds to a .h file and vice versa...)


================
Comment at: clangd/index/Serialization.h:46
+  // URIs of headers directly included in the source file.
+  llvm::Optional<std::vector<std::string>> DirectIncludes;
 };
----------------
This seems a little off to me conceptually: an index contains symbols that might be from many files. (Also true of Digest, which I missed).

I'd suggest a richer structure which can represent one or many files:
```
std::unordered_map</*uri*/string, SourceFile>; // keys are URIs
struct SourceFile {
  bool IsTU;
  StringRef URI; // points back into key
  FileDigest Digest;
  vector<StringRef> Headers; // points back into keys
  // maybe later: compile command
}
```
(though probably wrap the map into a class to ensure the string ownership doesn't get messed up)

For auto-index shards we could include full SourceFile info for the main file (with IsTU = true, and headers) and each of the headers would have a skeletal entry with IsTU = false and no headers stored.

But clangd-indexer could spit out an index that contains full information for all the transitive include of all indexed TUs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54817