[clangd] Add HadErrors field into shards

[kadircet]

• Delete leftover header swap

[sammccall]

clang-tools-extra/clangd/Headers.h

65 ↗

(On Diff #207793)

nit: I'd tend to use a plain enum here to avoid the operator boilerplate, but up to you

[kadircet]

clang-tools-extra/clangd/Headers.h

65 ↗

(On Diff #207793)

I was more worried about incidental usages like:

IncludeGraphNode IGN;
if(IGN.IsTU) blabla;

[kadircet]

• Populate HadErrors in background index

clang-tools-extra/trunk/clangd/Headers.h

struct Inclusion {
   SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
 };
 llvm::raw_ostream &operator<<(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 = 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;
 };
 // FileURI and FileInclusions are references to keys of the map containing
 // them.
 // Important: The graph generated by those callbacks might contain cycles, self
 // 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.
 //
 // It should be built incrementally with collectIncludeStructureCallback().
 // When we build the preamble, we capture and store its include structure along
 // with the preamble data. When we use the preamble, we can copy its
 // IncludeStructure and use another collectIncludeStructureCallback() to fill

clang-tools-extra/trunk/clangd/index/Background.cpp

void BackgroundIndex::update(llvm::StringRef MainFile, IndexFileIn Index,
   for (const auto &IndexIt : *Index.Sources) {
     const auto &IGN = IndexIt.getValue();
     // In case of failures, we only store main file of TU. That way we can still
     // get symbols from headers if some other TU can compile them. Note that
     // sources do not contain any information regarding missing headers, since
     // 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);
     // File has different contents.
     if (DigestIt == DigestsSnapshot.end() || DigestIt->getValue() != IGN.Digest)
       Files.try_emplace(AbsPath).first->getValue().Digest = IGN.Digest;
   }
   // This map is used to figure out where to store relations.
log("Indexed {0} ({1} symbols, {2} refs, {3} files)",
       Inputs.CompileCommand.Filename, Index.Symbols->size(),
       Index.Refs->numRefs(), Index.Sources->size());
   SPAN_ATTACH(Tracer, "symbols", int(Index.Symbols->size()));
   SPAN_ATTACH(Tracer, "refs", int(Index.Refs->numRefs()));
   SPAN_ATTACH(Tracer, "sources", int(Index.Sources->size()));
 
   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;
   else
     reset(
         IndexedSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
 
   return llvm::Error::success();
for (const auto &I : *Shard->Sources) {
         continue;
 
       // We found source file info for current dependency.
       assert(I.getValue().Digest != FileDigest{{0}} && "Digest is empty?");
       ShardInfo SI;
       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.
       auto Buf = FS->getBufferForFile(CurDependency.Path);
       if (!Buf) {
         elog("Couldn't get buffer for file: {0}: {1}", CurDependency.Path,
              Buf.getError().message());
         continue;

clang-tools-extra/trunk/clangd/index/IndexAction.cpp

 //===--- IndexAction.cpp -----------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "IndexAction.h"
+#include "Headers.h"
 #include "Logger.h"
 #include "index/Relation.h"
 #include "index/SymbolOrigin.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/Tooling.h"
 #include <utility>
 
#ifndef NDEBUG
       auto Digest = digestFile(SM, FileID);
       assert(Digest && Node.Digest == *Digest &&
              "Same file, different digest?");
 #endif
       return;
     }
     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();
   }
 
   // Add edges from including files to includes.
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           llvm::StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           llvm::StringRef SearchPath,

clang-tools-extra/trunk/clangd/index/Serialization.cpp

 //===-- Serialization.cpp - Binary serialization of index data ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Serialization.h"
+#include "Headers.h"
 #include "Logger.h"
 #include "RIFF.h"
 #include "SymbolLocation.h"
 #include "SymbolOrigin.h"
 #include "Trace.h"
 #include "dex/Dex.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/StringRef.h"
for (auto *Endpoint : {&Loc.Start, &Loc.End}) {
     Endpoint->setColumn(Data.consumeVar());
   }
   return Loc;
 }
 
 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());
   IGN.DirectIncludes.resize(Data.consumeVar());
   for (llvm::StringRef &Include : IGN.DirectIncludes)
     Include = Data.consumeString(Strings);
   return IGN;
 }
 
 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());
   OS << Hash;
   writeVar(IGN.DirectIncludes.size(), OS);
   for (llvm::StringRef Include : IGN.DirectIncludes)
     writeVar(Strings.index(Include), OS);
 }

 //   - srcs: information related to include graph
 //   - stri: string table
 //   - symb: symbols
 //   - refs: references to symbols
 
 // 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);
   if (!RIFF)
     return RIFF.takeError();
   if (RIFF->Type != riff::fourCC("CdIx"))
     return makeError("wrong RIFF type");
   llvm::StringMap<llvm::StringRef> Chunks;

clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp

+#include "Headers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/Background.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
 #include "gmock/gmock.h"

 MATCHER(Defined, "") { return !StringRef(arg.Definition.FileURI).empty(); }
 MATCHER_P(FileURI, F, "") { return StringRef(arg.Location.FileURI) == F; }
 ::testing::Matcher<const RefSlab &>
 RefsAre(std::vector<::testing::Matcher<Ref>> Matchers) {
   return ElementsAre(::testing::Pair(_, UnorderedElementsAreArray(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 {
   mutable std::mutex StorageMu;
   llvm::StringMap<std::string> &Storage;
   size_t &CacheHits;
 
 public:
TEST_F(BackgroundIndexTest, UncompilableFiles) {
 
   // Make sure we only store the shard for main file.
   EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc")));
   auto Shard = MSS.loadShard(testPath("A.cc"));
   EXPECT_THAT(*Shard->Symbols, UnorderedElementsAre(Named("foo")));
   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) {
   MockFSProvider FS;
   llvm::StringMap<std::string> Storage;
   size_t CacheHits = 0;
   MemoryShardStorage MSS(Storage, CacheHits);
   OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{},

clang-tools-extra/trunk/clangd/unittests/IndexActionTests.cpp

 //===------ IndexActionTests.cpp  -------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "Headers.h"
 #include "TestFS.h"
 #include "index/IndexAction.h"
 #include "clang/Tooling/Tooling.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::Not;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 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; }
 
 MATCHER_P(HasName, Name, "") { return arg.Name == Name; }
 
 MATCHER(HasSameURI, "") {
   llvm::StringRef URI = ::testing::get<0>(arg);
   const std::string &Path = ::testing::get<1>(arg);

clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp

 //===-- SerializationTests.cpp - Binary and YAML serialization unit tests -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "Headers.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
TEST(SerializationTest, SrcsTest) {
 
   std::string TestContent("TestContent");
   IncludeGraphNode IGN;
   IGN.Digest =
       llvm::SHA1::hash({reinterpret_cast<const uint8_t *>(TestContent.data()),
                         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.
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
   Out.Sources = &Sources;
   {
     std::string Serialized = llvm::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);
+    EXPECT_EQ(IGNDeserialized.Flags, IGN.Flags);
   }
 }
 
 TEST(SerializationTest, CmdlTest) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
 
   tooling::CompileCommand Cmd;