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

2018-11-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE347669: [clangd] Put direct headers into srcs section. 
(authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54817?vs=175483=175486#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54817/new/

https://reviews.llvm.org/D54817

Files:
  clangd/Headers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: clangd/Headers.h
===
--- clangd/Headers.h
+++ clangd/Headers.h
@@ -48,6 +48,20 @@
 };
 llvm::raw_ostream <<(llvm::raw_ostream &, const Inclusion&);
 
+// Contains information about one file in the build grpah and its direct
+// dependencies. Doesn't own the strings it references (IncludeGraph is
+// self-contained).
+struct IncludeGraphNode {
+  // True if current file is a main file rather than a header.
+  bool IsTU;
+  llvm::StringRef URI;
+  FileDigest Digest;
+  std::vector DirectIncludes;
+};
+// FileURI and FileInclusions are references to keys of the map containing
+// them.
+using IncludeGraph = llvm::StringMap;
+
 // Information captured about the inclusion graph in a translation unit.
 // This includes detailed information about the direct #includes, and summary
 // information about all transitive includes.
Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -227,5 +227,17 @@
  Left.end.character == Right.start.character;
 }
 
+FileDigest digest(StringRef Content) {
+  return llvm::SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
+}
+
+Optional digestFile(const SourceManager , FileID FID) {
+  bool Invalid = false;
+  StringRef Content = SM.getBufferData(FID, );
+  if (Invalid)
+return None;
+  return digest(Content);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -16,13 +16,22 @@
 #include "Protocol.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/SHA1.h"
 
 namespace clang {
 class SourceManager;
 
 namespace clangd {
 
+// We tend to generate digests for source codes in a lot of different places.
+// This represents the type for those digests to prevent us hard coding details
+// of hashing function at every place that needs to store this information.
+using FileDigest = decltype(llvm::SHA1::hash({}));
+FileDigest digest(StringRef Content);
+Optional digestFile(const SourceManager , FileID FID);
+
 // Counts the number of UTF-16 code units needed to represent a string (LSP
 // specifies string lengths in UTF-16 code units).
 size_t lspLength(StringRef Code);
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -11,6 +11,7 @@
 #include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Logger.h"
+#include "SourceCode.h"
 #include "Threading.h"
 #include "Trace.h"
 #include "URI.h"
@@ -149,19 +150,6 @@
   QueueCV.notify_all();
 }
 
-static BackgroundIndex::FileDigest digest(StringRef Content) {
-  return SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
-}
-
-static Optional digestFile(const SourceManager ,
-FileID FID) {
-  bool Invalid = false;
-  StringRef Content = SM.getBufferData(FID, );
-  if (Invalid)
-return None;
-  return digest(Content);
-}
-
 // Resolves URI to file paths with cache.
 class URIToFileCache {
 public:
@@ -193,8 +181,7 @@
 };
 
 /// Given index results from a TU, only update files in \p FilesToUpdate.
-void BackgroundIndex::update(StringRef MainFile, SymbolSlab Symbols,
- RefSlab Refs,
+void BackgroundIndex::update(StringRef MainFile, IndexFileIn Index,
  const StringMap ,
  BackgroundIndexStorage *IndexStorage) {
   // Partition symbols/references into files.
@@ -204,7 +191,7 @@
   };
   StringMap Files;
   URIToFileCache URICache(MainFile);
-  for (const auto  : Symbols) {
+  for (const auto  : *Index.Symbols) {
 if (Sym.CanonicalDeclaration) {
   auto DeclPath = URICache.resolve(Sym.CanonicalDeclaration.FileURI);
   if (FilesToUpdate.count(DeclPath) != 0)
@@ -222,7 +209,7 @@
 }
   }
   DenseMap RefToIDs;
-  for (const auto  : Refs) {
+  for (const auto  : *Index.Refs) {
 for (const auto  : SymRefs.second) {
   auto 

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

2018-11-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 175483.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Address comments.
- Move digest and digestFile into SourceCode.h


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54817/new/

https://reviews.llvm.org/D54817

Files:
  clangd/Headers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -173,31 +173,44 @@
   UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
 }
 
-TEST(SerializationTest, HashTest) {
+TEST(SerializationTest, SrcsTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
-  std::string TestContent("TESTCONTENT");
-  auto Digest =
+  std::string TestContent("TestContent");
+  IncludeGraphNode IGN;
+  IGN.Digest =
   llvm::SHA1::hash({reinterpret_cast(TestContent.data()),
 TestContent.size()});
+  IGN.DirectIncludes = {"inc1", "inc2"};
+  IGN.URI = "URI";
+  IGN.IsTU = true;
+  IncludeGraph Sources;
+  Sources[IGN.URI] = IGN;
   // Write to binary format, and parse again.
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
-  Out.Digest = 
-  std::string Serialized = to_string(Out);
-
-  auto In2 = readIndexFile(Serialized);
-  ASSERT_TRUE(bool(In2)) << In.takeError();
-  ASSERT_EQ(In2->Digest, Digest);
-  ASSERT_TRUE(In2->Symbols);
-  ASSERT_TRUE(In2->Refs);
-
-  // Assert the YAML serializations match, for nice comparisons and diffs.
-  EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
-  UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
-  EXPECT_THAT(YAMLFromRefs(*In2->Refs),
-  UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  Out.Sources = 
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+ASSERT_TRUE(In->Sources);
+ASSERT_TRUE(In->Sources->count(IGN.URI));
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+auto IGNDeserialized = In->Sources->lookup(IGN.URI);
+EXPECT_EQ(IGNDeserialized.Digest, IGN.Digest);
+EXPECT_EQ(IGNDeserialized.DirectIncludes, IGN.DirectIncludes);
+EXPECT_EQ(IGNDeserialized.URI, IGN.URI);
+EXPECT_EQ(IGNDeserialized.IsTU, IGN.IsTU);
+  }
 }
 
 } // namespace
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -124,8 +124,6 @@
   )cpp";
   std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
   FS.Files[testPath("root/A.cc")] = A_CC;
-  auto Digest = llvm::SHA1::hash(
-  {reinterpret_cast(A_CC.data()), A_CC.size()});
 
   llvm::StringMap Storage;
   size_t CacheHits = 0;
@@ -160,7 +158,6 @@
   EXPECT_NE(ShardSource, nullptr);
   EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre());
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
-  EXPECT_EQ(*ShardSource->Digest, Digest);
 }
 
 } // namespace clangd
Index: clangd/index/Serialization.h
===
--- clangd/index/Serialization.h
+++ clangd/index/Serialization.h
@@ -24,6 +24,7 @@
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RIFF_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RIFF_H
+#include "Headers.h"
 #include "Index.h"
 #include "llvm/Support/Error.h"
 
@@ -37,11 +38,10 @@
 
 // Holds the contents of an index file that was read.
 struct IndexFileIn {
-  using FileDigest = std::array;
   llvm::Optional Symbols;
   llvm::Optional Refs;
-  // Digest of the source file that generated the contents.
-  llvm::Optional Digest;
+  // Keys are URIs of the source files.
+  llvm::Optional Sources;
 };
 // Parse an index file. The input must be a RIFF or YAML file.
 llvm::Expected readIndexFile(llvm::StringRef);
@@ -50,8 +50,8 @@
 struct IndexFileOut {
   const SymbolSlab *Symbols = nullptr;
   const RefSlab *Refs = nullptr;
-  // Digest of the source file that generated the contents.
-  const IndexFileIn::FileDigest *Digest = nullptr;
+  // Keys are URIs of the source files.
+  const IncludeGraph *Sources = nullptr;
   // TODO: Support serializing Dex posting lists.
   IndexFileFormat Format 

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

2018-11-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Only really significant thing is I think the name "build graph" is confusing, 
and we should specifically mention includes.
Rest is just nits to take or leave...




Comment at: clangd/Headers.h:52
+// Contains information about one file in the build grpah and its direct
+// dependencies.
+struct BuildGraphNode {

May be worth explicitly noting:
`Does not own the strings it references (BuildGraphInclusions is 
self-contained)`



Comment at: clangd/Headers.h:53
+// dependencies.
+struct BuildGraphNode {
+  bool IsTU;

I think we should be more specific and call this the "include graph".
Build graph can suggest other build-system related things (bazel and ninja have 
a very formal concept of a build graph, other systems have less formalized but 
may still use the term)



Comment at: clangd/Headers.h:54
+struct BuildGraphNode {
+  bool IsTU;
+  llvm::StringRef FileURI;

this is worth a comment (e.g. `A "main file" as opposed to a header.`)



Comment at: clangd/Headers.h:55
+  bool IsTU;
+  llvm::StringRef FileURI;
+  FileDigest Digest;

This is slightly ambiguous: URI of the file, or URI with the `file` scheme?
I think you mean the former.
I think `URI` is actually the best name for this field, even though it's taken 
by the class. As a member of a struct, there's not really any ambiguous cases.



Comment at: clangd/Headers.h:57
+  FileDigest Digest;
+  std::vector FileInclusions;
+};

nit: I don't think "File" actually adds anything here, the whole struct is 
scoped to a file.
I'd add a comment that this is only direct inclusions, if you choose not to 
mention it in the name.
(The comment seems more useful here than it does on the struct)



Comment at: clangd/SourceCode.h:26
 
+using FileDigest = std::array;
+

Probably worth a comment, especially since there are no functions here using it.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54817/new/

https://reviews.llvm.org/D54817



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


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

2018-11-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 175469.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Only keep the build graph structures and {de,}serialization logic.
- Rename a few structures, move to more relavant places.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54817/new/

https://reviews.llvm.org/D54817

Files:
  clangd/Headers.h
  clangd/SourceCode.h
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -173,31 +173,44 @@
   UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
 }
 
-TEST(SerializationTest, HashTest) {
+TEST(SerializationTest, SrcsTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
-  std::string TestContent("TESTCONTENT");
-  auto Digest =
+  std::string TestContent("TestContent");
+  BuildGraphNode BGN;
+  BGN.Digest =
   llvm::SHA1::hash({reinterpret_cast(TestContent.data()),
 TestContent.size()});
+  BGN.FileInclusions = {"inc1", "inc2"};
+  BGN.FileURI = "FileURI";
+  BGN.IsTU = true;
+  BuildGraphInclusions Sources;
+  Sources[BGN.FileURI] = BGN;
   // Write to binary format, and parse again.
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
-  Out.Digest = 
-  std::string Serialized = to_string(Out);
-
-  auto In2 = readIndexFile(Serialized);
-  ASSERT_TRUE(bool(In2)) << In.takeError();
-  ASSERT_EQ(In2->Digest, Digest);
-  ASSERT_TRUE(In2->Symbols);
-  ASSERT_TRUE(In2->Refs);
-
-  // Assert the YAML serializations match, for nice comparisons and diffs.
-  EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
-  UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
-  EXPECT_THAT(YAMLFromRefs(*In2->Refs),
-  UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  Out.Sources = 
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+ASSERT_TRUE(In->Sources);
+ASSERT_TRUE(In->Sources->count(BGN.FileURI));
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+auto BGNDeserialized = In->Sources->lookup(BGN.FileURI);
+EXPECT_EQ(BGNDeserialized.Digest, BGN.Digest);
+EXPECT_EQ(BGNDeserialized.FileInclusions, BGN.FileInclusions);
+EXPECT_EQ(BGNDeserialized.FileURI, BGN.FileURI);
+EXPECT_EQ(BGNDeserialized.IsTU, BGN.IsTU);
+  }
 }
 
 } // namespace
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -124,8 +124,6 @@
   )cpp";
   std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
   FS.Files[testPath("root/A.cc")] = A_CC;
-  auto Digest = llvm::SHA1::hash(
-  {reinterpret_cast(A_CC.data()), A_CC.size()});
 
   llvm::StringMap Storage;
   size_t CacheHits = 0;
@@ -160,7 +158,6 @@
   EXPECT_NE(ShardSource, nullptr);
   EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre());
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
-  EXPECT_EQ(*ShardSource->Digest, Digest);
 }
 
 } // namespace clangd
Index: clangd/index/Serialization.h
===
--- clangd/index/Serialization.h
+++ clangd/index/Serialization.h
@@ -24,6 +24,7 @@
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RIFF_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RIFF_H
+#include "Headers.h"
 #include "Index.h"
 #include "llvm/Support/Error.h"
 
@@ -37,11 +38,10 @@
 
 // Holds the contents of an index file that was read.
 struct IndexFileIn {
-  using FileDigest = std::array;
   llvm::Optional Symbols;
   llvm::Optional Refs;
-  // Digest of the source file that generated the contents.
-  llvm::Optional Digest;
+  // Keys are URIs of the source files.
+  llvm::Optional Sources;
 };
 // Parse an index file. The input must be a RIFF or YAML file.
 llvm::Expected readIndexFile(llvm::StringRef);
@@ -50,8 +50,8 @@
 struct IndexFileOut {
   const SymbolSlab *Symbols = nullptr;
   const RefSlab *Refs = nullptr;
-  // Digest of the source file that generated the contents.
-  const IndexFileIn::FileDigest *Digest = nullptr;
+  // Keys are URIs of the source files.
+  const BuildGraphInclusions *Sources = nullptr;

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

2018-11-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

To summarize offline discussion: I think we need to split this patch up, as the 
scope is growing. (In a really useful direction I think!)

The first patch should add the include graph data structure. (I think the right 
place for it is probably Headers.h, as we want to reuse it). It can also 
include the code to serialize/deserialize it in index files, which is in good 
shape.

Then we have at least 3 independent dependent patches :-)

- populate the transitive include graph structure from a PPCallbacks. Probably 
hook it up to the static index FrontendAction too, since that's simple. This 
can be tested directly.
- write the include graph appropriately in auto-index. This isn't quite 
trivial, as it needs to be partitioned by file.
- read shards in auto-index. This could be all-at-once, or a simple version 
that didn't follow #include edges first, and then modify to consume the include 
graph.

This is a bunch more work than originally envisioned, but having dependency 
graph info in the index enables/improves a bunch of features. Making this 
reusable across indexes seems worth it.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54817/new/

https://reviews.llvm.org/D54817



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


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

2018-11-27 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

structure and serialization stuff looks great!
still some questions around the auto-index logic and organization of the 
extraction code, let's chat offline. May make sense to split.




Comment at: clangd/index/Background.cpp:329
+llvm::StringMap
+IncludeStructureToSources(llvm::StringRef MainFile,
+  const IncludeStructure ,

nit: lowercase



Comment at: clangd/index/Background.cpp:341
+std::string FileURI = URI::create(Dependency).toString();
+auto Buf = FS->getBufferForFile(Dependency);
+if (!Buf)

It looks like you're re-reading the files here. this won't reuse in-memory 
copies of the file, and won't respect overridden contents outside the VFS 
(ugh). The SourceManager would be better I think.



Comment at: clangd/index/Background.cpp:351
+SF.Digest = std::move(Digest);
+for (const auto  : Includes.includeDepth(Dependency)) {
+  if (Include.getValue() != 1) // Skip transitive includes.

I don't think this is going to work:
 - you're outputting the non-transitive dependencies from the TU, but I think 
you want the *transitive* ones, which should then be partitioned by file (same 
as symbols). Otherwise the headers from the shard for `foo.h` aren't going to 
be right, are they?
 - I don't think the current IncludeStructure includes all the info you want 
for transitive includes. e.g. for include edges A->B where there is a shorter 
path from Main->B, we don't record anything.
 - I think we will want the include information for other forms of index too. I 
think this means the logic should be next to symbolcollector, or in Headers.h, 
where it can be shared



Comment at: clangd/index/Serialization.cpp:400
+  Entry->getValue() = std::move(SF);
+  Entry->getValue().FileURI = Entry->getKey();
+  for(auto  : Entry->getValue().DirectIncludes)

these string reassignments deserve a comment



Comment at: clangd/index/Serialization.h:41
   using FileDigest = std::array;
+  // Contains information about one source file that participates in current
+  // index file.

Does this belong here, vs in Index, or SymbolCollector, or so?



Comment at: clangd/index/Serialization.h:53
+  // Keys are URIs of the source files.
+  llvm::Optional> Sources;
 };

I think the StringMap could also use a typedef near SourceFile - that's where 
we could mention that FileURI and DirectIncludes point back at the keys.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54817/new/

https://reviews.llvm.org/D54817



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


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

2018-11-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 175412.
kadircet added a comment.

- Rebase to get changes in background index.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54817/new/

https://reviews.llvm.org/D54817

Files:
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -173,31 +173,44 @@
   UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
 }
 
-TEST(SerializationTest, HashTest) {
+TEST(SerializationTest, SrcsTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
-  std::string TestContent("TESTCONTENT");
-  auto Digest =
+  std::string TestContent("TestContent");
+  IndexFileIn::SourceFile SF;
+  SF.Digest =
   llvm::SHA1::hash({reinterpret_cast(TestContent.data()),
 TestContent.size()});
+  SF.DirectIncludes = {"inc1", "inc2"};
+  SF.FileURI = "FileURI";
+  SF.IsTU = true;
+  llvm::StringMap Sources;
+  Sources[SF.FileURI] = SF;
   // Write to binary format, and parse again.
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
-  Out.Digest = 
-  std::string Serialized = to_string(Out);
-
-  auto In2 = readIndexFile(Serialized);
-  ASSERT_TRUE(bool(In2)) << In.takeError();
-  ASSERT_EQ(In2->Digest, Digest);
-  ASSERT_TRUE(In2->Symbols);
-  ASSERT_TRUE(In2->Refs);
-
-  // Assert the YAML serializations match, for nice comparisons and diffs.
-  EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
-  UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
-  EXPECT_THAT(YAMLFromRefs(*In2->Refs),
-  UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  Out.Sources = 
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+ASSERT_TRUE(In->Sources);
+ASSERT_TRUE(In->Sources->count(SF.FileURI));
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+auto SFDeserialized = In->Sources->lookup(SF.FileURI);
+EXPECT_EQ(SFDeserialized.Digest, SF.Digest);
+EXPECT_EQ(SFDeserialized.DirectIncludes, SF.DirectIncludes);
+EXPECT_EQ(SFDeserialized.FileURI, SF.FileURI);
+EXPECT_EQ(SFDeserialized.IsTU, SF.IsTU);
+  }
 }
 
 } // namespace
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -160,7 +160,51 @@
   EXPECT_NE(ShardSource, nullptr);
   EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre());
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
-  EXPECT_EQ(*ShardSource->Digest, Digest);
+  EXPECT_EQ(ShardSource->Sources->lookup("unittest:///root/A.cc").Digest,
+Digest);
+}
+
+TEST(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  {
+OverlayCDB CDB(/*Base=*/nullptr);
+BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+[&](llvm::StringRef) { return  });
+CDB.setCompileCommand(testPath("root"), Cmd);
+ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 3U); // A.cc, A.h, B.h
+  EXPECT_THAT(
+  ShardSource->Sources->lookup("unittest:///root/A.cc").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_TRUE(ShardHeader->Sources);
+  EXPECT_EQ(ShardHeader->Sources->size(), 2U); // A.h B.h
+  EXPECT_THAT(
+  ShardHeader->Sources->lookup("unittest:///root/A.h").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/B.h"));
 }
 
 } // namespace 

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

2018-11-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 175097.
kadircet added a comment.

- rebased and changed URI convertion patterns, since it changed in 
https://reviews.llvm.org/rL347467


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54817

Files:
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -173,31 +173,44 @@
   UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
 }
 
-TEST(SerializationTest, HashTest) {
+TEST(SerializationTest, SrcsTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
-  std::string TestContent("TESTCONTENT");
-  auto Digest =
+  std::string TestContent("TestContent");
+  IndexFileIn::SourceFile SF;
+  SF.Digest =
   llvm::SHA1::hash({reinterpret_cast(TestContent.data()),
 TestContent.size()});
+  SF.DirectIncludes = {"inc1", "inc2"};
+  SF.FileURI = "FileURI";
+  SF.IsTU = true;
+  llvm::StringMap Sources;
+  Sources[SF.FileURI] = SF;
   // Write to binary format, and parse again.
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
-  Out.Digest = 
-  std::string Serialized = to_string(Out);
-
-  auto In2 = readIndexFile(Serialized);
-  ASSERT_TRUE(bool(In2)) << In.takeError();
-  ASSERT_EQ(In2->Digest, Digest);
-  ASSERT_TRUE(In2->Symbols);
-  ASSERT_TRUE(In2->Refs);
-
-  // Assert the YAML serializations match, for nice comparisons and diffs.
-  EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
-  UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
-  EXPECT_THAT(YAMLFromRefs(*In2->Refs),
-  UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  Out.Sources = 
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+ASSERT_TRUE(In->Sources);
+ASSERT_TRUE(In->Sources->count(SF.FileURI));
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+auto SFDeserialized = In->Sources->lookup(SF.FileURI);
+EXPECT_EQ(SFDeserialized.Digest, SF.Digest);
+EXPECT_EQ(SFDeserialized.DirectIncludes, SF.DirectIncludes);
+EXPECT_EQ(SFDeserialized.FileURI, SF.FileURI);
+EXPECT_EQ(SFDeserialized.IsTU, SF.IsTU);
+  }
 }
 
 } // namespace
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -158,7 +158,50 @@
   EXPECT_NE(ShardSource, nullptr);
   EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre());
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
-  EXPECT_EQ(*ShardSource->Digest, Digest);
+  EXPECT_EQ(ShardSource->Sources->lookup("unittest:///root/A.cc").Digest,
+Digest);
+}
+
+TEST(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  {
+BackgroundIndex Idx(Context::empty(), "", FS,
+[&](llvm::StringRef) { return  });
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 3U); // A.cc, A.h, B.h
+  EXPECT_THAT(
+  ShardSource->Sources->lookup("unittest:///root/A.cc").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_TRUE(ShardHeader->Sources);
+  EXPECT_EQ(ShardHeader->Sources->size(), 2U); // A.h B.h
+  EXPECT_THAT(
+  ShardHeader->Sources->lookup("unittest:///root/A.h").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/B.h"));
 }
 
 } // namespace clangd
Index: clangd/index/Serialization.h

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

2018-11-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 175087.
kadircet marked an inline comment as done.
kadircet added a comment.

- Change srcs to store multiple source file information rathet than one.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54817

Files:
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -173,31 +173,44 @@
   UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
 }
 
-TEST(SerializationTest, HashTest) {
+TEST(SerializationTest, SrcsTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
-  std::string TestContent("TESTCONTENT");
-  auto Digest =
+  std::string TestContent("TestContent");
+  IndexFileIn::SourceFile SF;
+  SF.Digest =
   llvm::SHA1::hash({reinterpret_cast(TestContent.data()),
 TestContent.size()});
+  SF.DirectIncludes = {"inc1", "inc2"};
+  SF.FileURI = "FileURI";
+  SF.IsTU = true;
+  llvm::StringMap Sources;
+  Sources[SF.FileURI] = SF;
   // Write to binary format, and parse again.
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
-  Out.Digest = 
-  std::string Serialized = to_string(Out);
-
-  auto In2 = readIndexFile(Serialized);
-  ASSERT_TRUE(bool(In2)) << In.takeError();
-  ASSERT_EQ(In2->Digest, Digest);
-  ASSERT_TRUE(In2->Symbols);
-  ASSERT_TRUE(In2->Refs);
-
-  // Assert the YAML serializations match, for nice comparisons and diffs.
-  EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
-  UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
-  EXPECT_THAT(YAMLFromRefs(*In2->Refs),
-  UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  Out.Sources = 
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+ASSERT_TRUE(In->Sources);
+ASSERT_TRUE(In->Sources->count(SF.FileURI));
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+auto SFDeserialized = In->Sources->lookup(SF.FileURI);
+EXPECT_EQ(SFDeserialized.Digest, SF.Digest);
+EXPECT_EQ(SFDeserialized.DirectIncludes, SF.DirectIncludes);
+EXPECT_EQ(SFDeserialized.FileURI, SF.FileURI);
+EXPECT_EQ(SFDeserialized.IsTU, SF.IsTU);
+  }
 }
 
 } // namespace
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -158,7 +158,50 @@
   EXPECT_NE(ShardSource, nullptr);
   EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre());
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
-  EXPECT_EQ(*ShardSource->Digest, Digest);
+  EXPECT_EQ(ShardSource->Sources->lookup("unittest:///root/A.cc").Digest,
+Digest);
+}
+
+TEST(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  {
+BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+[&](llvm::StringRef) { return  });
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 3U); // A.cc, A.h, B.h
+  EXPECT_THAT(
+  ShardSource->Sources->lookup("unittest:///root/A.cc").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_TRUE(ShardHeader->Sources);
+  EXPECT_EQ(ShardHeader->Sources->size(), 2U); // A.h B.h
+  EXPECT_THAT(
+  ShardHeader->Sources->lookup("unittest:///root/A.h").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/B.h"));
 }
 
 } // namespace clangd
Index: 

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

2018-11-22 Thread 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> 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; // keys are URIs
struct SourceFile {
  bool IsTU;
  StringRef URI; // points back into key
  FileDigest Digest;
  vector 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



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


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

2018-11-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 175032.
kadircet added a comment.

- Use include depth to get includes for all files, rather than just main file.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54817

Files:
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -173,7 +173,7 @@
   UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
 }
 
-TEST(SerializationTest, HashTest) {
+TEST(SerializationTest, SrcsTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
@@ -185,19 +185,39 @@
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
   Out.Digest = 
-  std::string Serialized = to_string(Out);
-
-  auto In2 = readIndexFile(Serialized);
-  ASSERT_TRUE(bool(In2)) << In.takeError();
-  ASSERT_EQ(In2->Digest, Digest);
-  ASSERT_TRUE(In2->Symbols);
-  ASSERT_TRUE(In2->Refs);
-
-  // Assert the YAML serializations match, for nice comparisons and diffs.
-  EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
-  UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
-  EXPECT_THAT(YAMLFromRefs(*In2->Refs),
-  UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_EQ(*In->Digest, Digest);
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  }
+
+  std::vector DirectIncludes = {"inc1", "inc2"};
+  Out.DirectIncludes = 
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_EQ(*In->Digest, Digest);
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+EXPECT_THAT(*In->DirectIncludes, UnorderedElementsAreArray(DirectIncludes));
+
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  }
 }
 
 } // namespace
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -161,5 +161,41 @@
   EXPECT_EQ(*ShardSource->Digest, Digest);
 }
 
+TEST(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  {
+BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+[&](llvm::StringRef) { return  });
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_THAT(*ShardSource->DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_THAT(*ShardHeader->DirectIncludes,
+  UnorderedElementsAre("unittest:///root/B.h"));
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/Serialization.h
===
--- clangd/index/Serialization.h
+++ clangd/index/Serialization.h
@@ -42,6 +42,8 @@
   llvm::Optional Refs;
   // Digest of the source file that generated the contents.
   llvm::Optional Digest;
+  // URIs of headers directly included in the source file.
+  llvm::Optional> DirectIncludes;
 };
 // Parse an index file. The input must be a RIFF or YAML file.
 llvm::Expected readIndexFile(llvm::StringRef);
@@ -52,6 +54,8 @@
   const RefSlab *Refs = nullptr;
   // Digest of the source file that generated the contents.
   const 

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

2018-11-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.

Currently, there's no way of knowing about header files
using compilation database, since it doesn't contain header files as entries.

Using this information, restoring from cache using compile commands becomes
possible instead of doing directory traversal. Also, we can issue indexing
actions for out-of-date headers even if source files depending on them haven't
changed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54817

Files:
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/Serialization.cpp
  clangd/index/Serialization.h
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/SerializationTests.cpp

Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -173,7 +173,7 @@
   UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
 }
 
-TEST(SerializationTest, HashTest) {
+TEST(SerializationTest, SrcsTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
@@ -185,19 +185,39 @@
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
   Out.Digest = 
-  std::string Serialized = to_string(Out);
-
-  auto In2 = readIndexFile(Serialized);
-  ASSERT_TRUE(bool(In2)) << In.takeError();
-  ASSERT_EQ(In2->Digest, Digest);
-  ASSERT_TRUE(In2->Symbols);
-  ASSERT_TRUE(In2->Refs);
-
-  // Assert the YAML serializations match, for nice comparisons and diffs.
-  EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
-  UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
-  EXPECT_THAT(YAMLFromRefs(*In2->Refs),
-  UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_EQ(*In->Digest, Digest);
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  }
+
+  std::vector DirectIncludes = {"inc1", "inc2"};
+  Out.DirectIncludes = 
+  {
+std::string Serialized = to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_EQ(*In->Digest, Digest);
+ASSERT_TRUE(In->Symbols);
+ASSERT_TRUE(In->Refs);
+EXPECT_THAT(*In->DirectIncludes, UnorderedElementsAreArray(DirectIncludes));
+
+// Assert the YAML serializations match, for nice comparisons and diffs.
+EXPECT_THAT(YAMLFromSymbols(*In->Symbols),
+UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+EXPECT_THAT(YAMLFromRefs(*In->Refs),
+UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
+  }
 }
 
 } // namespace
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -161,5 +161,37 @@
   EXPECT_EQ(*ShardSource->Digest, Digest);
 }
 
+TEST(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  {
+BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+[&](llvm::StringRef) { return  });
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_THAT(*ShardSource->DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/Serialization.h
===
--- clangd/index/Serialization.h
+++ clangd/index/Serialization.h
@@ -42,6 +42,8 @@
   llvm::Optional Refs;
   // Digest of the source file that generated the contents.
   llvm::Optional Digest;
+  // URIs of headers directly included in the source file.
+  llvm::Optional> DirectIncludes;
 };
 // Parse an index file. The input