[clangd] Add HadErrors field into shards

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D64133

llvm-svn: 365122

diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h
index 5a20ea2..33b6ff2 100644 (file)
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -62,8 +62,15 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &);
 // 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 = false;
+  enum class SourceFlag : uint8_t {
+    None = 0,
+    // Whether current file is a main file rather than a header.
+    IsTU = 1 << 0,
+    // Whether current file had any uncompilable errors during indexing.
+    HadErrors = 1 << 1,
+  };
+
+  SourceFlag Flags = SourceFlag::None;
   llvm::StringRef URI;
   FileDigest Digest{{0}};
   std::vector<llvm::StringRef> DirectIncludes;
@@ -74,6 +81,22 @@ struct IncludeGraphNode {
 // edges and multi edges.
 using IncludeGraph = llvm::StringMap<IncludeGraphNode>;
 
+inline IncludeGraphNode::SourceFlag operator|(IncludeGraphNode::SourceFlag A,
+                                              IncludeGraphNode::SourceFlag B) {
+  return static_cast<IncludeGraphNode::SourceFlag>(static_cast<uint8_t>(A) |
+                                                   static_cast<uint8_t>(B));
+}
+
+inline bool operator&(IncludeGraphNode::SourceFlag A,
+                      IncludeGraphNode::SourceFlag B) {
+  return static_cast<uint8_t>(A) & static_cast<uint8_t>(B);
+}
+
+inline IncludeGraphNode::SourceFlag &
+operator|=(IncludeGraphNode::SourceFlag &A, IncludeGraphNode::SourceFlag B) {
+  return A = A | B;
+}
+
 // 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.

diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp
index 88d78ca..be0717b 100644 (file)
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -297,7 +297,7 @@ void BackgroundIndex::update(llvm::StringRef MainFile, IndexFileIn Index,
     // we don't even know what absolute path they should fall in.
     // FIXME: Also store contents from other files whenever the current contents
     // for those files are missing or if they had errors before.
-    if (HadErrors && !IGN.IsTU)
+    if (HadErrors && !(IGN.Flags & IncludeGraphNode::SourceFlag::IsTU))
       continue;
     const auto AbsPath = URICache.resolve(IGN.URI);
     const auto DigestIt = DigestsSnapshot.find(AbsPath);
@@ -498,10 +498,13 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd,
 
   bool HadErrors = Clang->hasDiagnostics() &&
                    Clang->getDiagnostics().hasUncompilableErrorOccurred();
+  if (HadErrors) {
+    log("Failed to compile {0}, index may be incomplete", AbsolutePath);
+    for (auto &It : *Index.Sources)
+      It.second.Flags |= IncludeGraphNode::SourceFlag::HadErrors;
+  }
   update(AbsolutePath, std::move(Index), DigestsSnapshot, IndexStorage,
          HadErrors);
-  if (HadErrors)
-    log("Failed to compile {0}, index may be incomplete", AbsolutePath);
 
   if (BuildIndexPeriodMs > 0)
     SymbolsUpdatedSinceLastIndex = true;
@@ -581,7 +584,8 @@ BackgroundIndex::loadShard(const tooling::CompileCommand &Cmd,
       SI.AbsolutePath = CurDependency.Path;
       SI.Shard = std::move(Shard);
       SI.Digest = I.getValue().Digest;
-      SI.CountReferences = I.getValue().IsTU;
+      SI.CountReferences =
+          I.getValue().Flags & IncludeGraphNode::SourceFlag::IsTU;
       IntermediateSymbols.push_back(std::move(SI));
       // Check if the source needs re-indexing.
       // Get the digest, skip it if file doesn't exist.

diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp
index ae3ed6a..84b69e9 100644 (file)
--- a/clang-tools-extra/clangd/index/IndexAction.cpp
+++ b/clang-tools-extra/clangd/index/IndexAction.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "IndexAction.h"
+#include "Headers.h"
 #include "Logger.h"
 #include "index/Relation.h"
 #include "index/SymbolOrigin.h"
@@ -67,7 +68,8 @@ public:
     }
     if (auto Digest = digestFile(SM, FileID))
       Node.Digest = std::move(*Digest);
-    Node.IsTU = FileID == SM.getMainFileID();
+    if (FileID == SM.getMainFileID())
+      Node.Flags |= IncludeGraphNode::SourceFlag::IsTU;
     Node.URI = I->getKey();
   }
 

diff --git a/clang-tools-extra/clangd/index/Serialization.cpp b/clang-tools-extra/clangd/index/Serialization.cpp
index f154689..5b3c7cf 100644 (file)
--- a/clang-tools-extra/clangd/index/Serialization.cpp
+++ b/clang-tools-extra/clangd/index/Serialization.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "Serialization.h"
+#include "Headers.h"
 #include "Logger.h"
 #include "RIFF.h"
 #include "SymbolLocation.h"
@@ -278,7 +279,7 @@ SymbolLocation readLocation(Reader &Data,
 IncludeGraphNode readIncludeGraphNode(Reader &Data,
                                       llvm::ArrayRef<llvm::StringRef> Strings) {
   IncludeGraphNode IGN;
-  IGN.IsTU = Data.consume8();
+  IGN.Flags = static_cast<IncludeGraphNode::SourceFlag>(Data.consume8());
   IGN.URI = Data.consumeString(Strings);
   llvm::StringRef Digest = Data.consume(IGN.Digest.size());
   std::copy(Digest.bytes_begin(), Digest.bytes_end(), IGN.Digest.begin());
@@ -291,7 +292,7 @@ IncludeGraphNode readIncludeGraphNode(Reader &Data,
 void writeIncludeGraphNode(const IncludeGraphNode &IGN,
                            const StringTableOut &Strings,
                            llvm::raw_ostream &OS) {
-  OS.write(IGN.IsTU);
+  OS.write(static_cast<uint8_t>(IGN.Flags));
   writeVar(Strings.index(IGN.URI), OS);
   llvm::StringRef Hash(reinterpret_cast<const char *>(IGN.Digest.data()),
                        IGN.Digest.size());
@@ -443,7 +444,7 @@ readCompileCommand(Reader CmdReader, llvm::ArrayRef<llvm::StringRef> Strings) {
 // The current versioning scheme is simple - non-current versions are rejected.
 // If you make a breaking change, bump this version number to invalidate stored
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 10;
+constexpr static uint32_t Version = 11;
 
 llvm::Expected<IndexFileIn> readRIFF(llvm::StringRef Data) {
   auto RIFF = riff::readFile(Data);

diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index 9446aba..bad1a3c 100644 (file)
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -1,3 +1,4 @@
+#include "Headers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
@@ -31,9 +32,14 @@ RefsAre(std::vector<::testing::Matcher<Ref>> Matchers) {
 }
 // URI cannot be empty since it references keys in the IncludeGraph.
 MATCHER(EmptyIncludeNode, "") {
-  return !arg.IsTU && !arg.URI.empty() && arg.Digest == FileDigest{{0}} &&
-         arg.DirectIncludes.empty();
+  return arg.Flags == IncludeGraphNode::SourceFlag::None && !arg.URI.empty() &&
+         arg.Digest == FileDigest{{0}} && arg.DirectIncludes.empty();
 }
+
+MATCHER(HadErrors, "") {
+  return arg.Flags & IncludeGraphNode::SourceFlag::HadErrors;
+}
+
 MATCHER_P(NumReferences, N, "") { return arg.References == N; }
 
 class MemoryShardStorage : public BackgroundIndexStorage {
@@ -525,6 +531,11 @@ TEST_F(BackgroundIndexTest, UncompilableFiles) {
   EXPECT_THAT(Shard->Sources->keys(),
               UnorderedElementsAre("unittest:///A.cc", "unittest:///A.h",
                                    "unittest:///B.h"));
+
+  EXPECT_THAT(Shard->Sources->lookup("unittest:///A.cc"), HadErrors());
+  // FIXME: We should also persist headers while marking them with errors.
+  EXPECT_THAT(Shard->Sources->lookup("unittest:///A.h"), Not(HadErrors()));
+  EXPECT_THAT(Shard->Sources->lookup("unittest:///B.h"), Not(HadErrors()));
 }
 
 TEST_F(BackgroundIndexTest, CmdLineHash) {

diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp
index 7188a3f..7d7a3c1 100644 (file)
--- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Headers.h"
 #include "TestFS.h"
 #include "index/IndexAction.h"
 #include "clang/Tooling/Tooling.h"
@@ -25,7 +26,7 @@ using ::testing::UnorderedPointwise;
 
 std::string toUri(llvm::StringRef Path) { return URI::create(Path).toString(); }
 
-MATCHER(IsTU, "") { return arg.IsTU; }
+MATCHER(IsTU, "") { return arg.Flags & IncludeGraphNode::SourceFlag::IsTU; }
 
 MATCHER_P(HasDigest, Digest, "") { return arg.Digest == Digest; }
 

diff --git a/clang-tools-extra/clangd/unittests/SerializationTests.cpp b/clang-tools-extra/clangd/unittests/SerializationTests.cpp
index 19d48ea..b76ea82 100644 (file)
--- a/clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Headers.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -212,7 +213,8 @@ TEST(SerializationTest, SrcsTest) {
                         TestContent.size()});
   IGN.DirectIncludes = {"inc1", "inc2"};
   IGN.URI = "URI";
-  IGN.IsTU = true;
+  IGN.Flags |= IncludeGraphNode::SourceFlag::IsTU;
+  IGN.Flags |= IncludeGraphNode::SourceFlag::HadErrors;
   IncludeGraph Sources;
   Sources[IGN.URI] = IGN;
   // Write to binary format, and parse again.
@@ -237,7 +239,7 @@ TEST(SerializationTest, SrcsTest) {
     EXPECT_EQ(IGNDeserialized.Digest, IGN.Digest);
     EXPECT_EQ(IGNDeserialized.DirectIncludes, IGN.DirectIncludes);
     EXPECT_EQ(IGNDeserialized.URI, IGN.URI);
-    EXPECT_EQ(IGNDeserialized.IsTU, IGN.IsTU);
+    EXPECT_EQ(IGNDeserialized.Flags, IGN.Flags);
   }
 }