Revert "[clangd] Refactor IncludeStructure: use File (unsigned) for most computations"

This reverts

- d1c6e54
- 7394d3b
- e507711
- 1bcd6b5

Author: kirillbobyrev

clang-tools-extra/clangd/CodeComplete.cpp

@@ -1379,16 +1379,14 @@ class CodeCompleteFlow {
       FileDistanceOptions ProxOpts{}; // Use defaults.
       const auto &SM = Recorder->CCSema->getSourceManager();
       llvm::StringMap<SourceParams> ProxSources;
-      auto MainFileID =
-          Includes.getOrCreateID(SM.getFileEntryForID(SM.getMainFileID()));
-      for (auto &HeaderIDAndDepth : Includes.includeDepth(MainFileID)) {
-        auto &Source =
-            ProxSources[Includes.getRealPath(HeaderIDAndDepth.getFirst())];
-        Source.Cost = HeaderIDAndDepth.getSecond() * ProxOpts.IncludeCost;
+      for (auto &Entry : Includes.includeDepth(
+               SM.getFileEntryForID(SM.getMainFileID())->getName())) {
+        auto &Source = ProxSources[Entry.getKey()];
+        Source.Cost = Entry.getValue() * ProxOpts.IncludeCost;
         // Symbols near our transitive includes are good, but only consider
         // things in the same directory or below it. Otherwise there can be
         // many false positives.
-        if (HeaderIDAndDepth.getSecond() > 0)
+        if (Entry.getValue() > 0)
           Source.MaxUpTraversals = 1;
       }
       FileProximity.emplace(ProxSources, ProxOpts);

clang-tools-extra/clangd/Headers.cpp

@@ -67,9 +67,8 @@ class RecordHeaders : public PPCallbacks {
         // Treat as if included from the main file.
         IncludingFileEntry = SM.getFileEntryForID(MainFID);
       }
-      auto IncludingID = Out->getOrCreateID(IncludingFileEntry),
-           IncludedID = Out->getOrCreateID(File);
-      Out->IncludeChildren[IncludingID].push_back(IncludedID);
+      Out->recordInclude(IncludingFileEntry->getName(), File->getName(),
+                         File->tryGetRealPathName());
     }
   }
 
@@ -155,49 +154,49 @@ collectIncludeStructureCallback(const SourceManager &SM,
   return std::make_unique<RecordHeaders>(SM, Out);
 }
 
-llvm::Optional<IncludeStructure::HeaderID>
-IncludeStructure::getID(const FileEntry *Entry) const {
-  auto It = NameToIndex.find(Entry->getName());
-  if (It == NameToIndex.end())
-    return llvm::None;
-  return It->second;
+void IncludeStructure::recordInclude(llvm::StringRef IncludingName,
+                                     llvm::StringRef IncludedName,
+                                     llvm::StringRef IncludedRealName) {
+  auto Child = fileIndex(IncludedName);
+  if (!IncludedRealName.empty() && RealPathNames[Child].empty())
+    RealPathNames[Child] = std::string(IncludedRealName);
+  auto Parent = fileIndex(IncludingName);
+  IncludeChildren[Parent].push_back(Child);
 }
 
-IncludeStructure::HeaderID
-IncludeStructure::getOrCreateID(const FileEntry *Entry) {
-  auto R = NameToIndex.try_emplace(
-      Entry->getName(),
-      static_cast<IncludeStructure::HeaderID>(RealPathNames.size()));
+unsigned IncludeStructure::fileIndex(llvm::StringRef Name) {
+  auto R = NameToIndex.try_emplace(Name, RealPathNames.size());
   if (R.second)
     RealPathNames.emplace_back();
-  IncludeStructure::HeaderID Result = R.first->getValue();
-  std::string &RealPathName = RealPathNames[static_cast<unsigned>(Result)];
-  if (RealPathName.empty())
-    RealPathName = Entry->tryGetRealPathName().str();
-  return Result;
+  return R.first->getValue();
 }
 
-llvm::DenseMap<IncludeStructure::HeaderID, unsigned>
-IncludeStructure::includeDepth(HeaderID Root) const {
+llvm::StringMap<unsigned>
+IncludeStructure::includeDepth(llvm::StringRef Root) const {
   // Include depth 0 is the main file only.
-  llvm::DenseMap<HeaderID, unsigned> Result;
-  assert(static_cast<unsigned>(Root) < RealPathNames.size());
+  llvm::StringMap<unsigned> Result;
   Result[Root] = 0;
-  std::vector<IncludeStructure::HeaderID> CurrentLevel;
-  CurrentLevel.push_back(Root);
-  llvm::DenseSet<IncludeStructure::HeaderID> Seen;
-  Seen.insert(Root);
+  std::vector<unsigned> CurrentLevel;
+  llvm::DenseSet<unsigned> Seen;
+  auto It = NameToIndex.find(Root);
+  if (It != NameToIndex.end()) {
+    CurrentLevel.push_back(It->second);
+    Seen.insert(It->second);
+  }
 
   // Each round of BFS traversal finds the next depth level.
-  std::vector<IncludeStructure::HeaderID> PreviousLevel;
+  std::vector<unsigned> PreviousLevel;
   for (unsigned Level = 1; !CurrentLevel.empty(); ++Level) {
     PreviousLevel.clear();
     PreviousLevel.swap(CurrentLevel);
     for (const auto &Parent : PreviousLevel) {
       for (const auto &Child : IncludeChildren.lookup(Parent)) {
         if (Seen.insert(Child).second) {
           CurrentLevel.push_back(Child);
-          Result[Child] = Level;
+          const auto &Name = RealPathNames[Child];
+          // Can't include files if we don't have their real path.
+          if (!Name.empty())
+            Result[Name] = Level;
         }
       }
     }

clang-tools-extra/clangd/Headers.h

@@ -12,15 +12,13 @@
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "index/Symbol.h"
-#include "support/Logger.h"
 #include "support/Path.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Tooling/Inclusions/HeaderIncludes.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
@@ -64,7 +62,7 @@ struct Inclusion {
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &);
 bool operator==(const Inclusion &LHS, const Inclusion &RHS);
 
-// Contains information about one file in the build graph and its direct
+// 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 {
@@ -114,54 +112,34 @@ operator|=(IncludeGraphNode::SourceFlag &A, IncludeGraphNode::SourceFlag B) {
 // in any non-preamble inclusions.
 class IncludeStructure {
 public:
-  // HeaderID identifies file in the include graph. It corresponds to a
-  // FileEntry rather than a FileID, but stays stable across preamble & main
-  // file builds.
-  enum class HeaderID : unsigned {};
-
-  llvm::Optional<HeaderID> getID(const FileEntry *Entry) const;
-  HeaderID getOrCreateID(const FileEntry *Entry);
-
-  StringRef getRealPath(HeaderID ID) const {
-    assert(static_cast<unsigned>(ID) <= RealPathNames.size());
-    return RealPathNames[static_cast<unsigned>(ID)];
-  }
+  std::vector<Inclusion> MainFileIncludes;
 
   // Return all transitively reachable files.
   llvm::ArrayRef<std::string> allHeaders() const { return RealPathNames; }
 
   // Return all transitively reachable files, and their minimum include depth.
   // All transitive includes (absolute paths), with their minimum include depth.
   // Root --> 0, #included file --> 1, etc.
-  // Root is the ID of the header being visited first.
-  // Usually it is getID(SM.getFileEntryForID(SM.getMainFileID())->getName()).
-  llvm::DenseMap<HeaderID, unsigned> includeDepth(HeaderID Root) const;
+  // Root is clang's name for a file, which may not be absolute.
+  // Usually it should be SM.getFileEntryForID(SM.getMainFileID())->getName().
+  llvm::StringMap<unsigned> includeDepth(llvm::StringRef Root) const;
 
-  // Maps HeaderID to the ids of the files included from it.
-  llvm::DenseMap<HeaderID, SmallVector<HeaderID>> IncludeChildren;
-
-  std::vector<Inclusion> MainFileIncludes;
-
-  std::string dump() {
-    std::string Result =
-        "RealPathNames: " +
-        llvm::join(RealPathNames.begin(), RealPathNames.end(), ", ");
-    Result += "; NameToIndex: ";
-    for (const auto &NameIndex : NameToIndex) {
-      Result += NameIndex.first().str() + ' ' +
-                std::to_string(static_cast<unsigned>(NameIndex.second)) + ", ";
-    }
-    return Result;
-  }
+  // This updates IncludeDepth(), but not MainFileIncludes.
+  void recordInclude(llvm::StringRef IncludingName,
+                     llvm::StringRef IncludedName,
+                     llvm::StringRef IncludedRealName);
 
 private:
-  std::vector<std::string> RealPathNames; // In HeaderID order.
-  // HeaderID maps the FileEntry::Name to the internal representation.
   // Identifying files in a way that persists from preamble build to subsequent
-  // builds is surprisingly hard. FileID is unavailable in
-  // InclusionDirective(), and RealPathName and UniqueID are not preserved in
-  // the preamble.
-  llvm::StringMap<HeaderID> NameToIndex;
+  // builds is surprisingly hard. FileID is unavailable in InclusionDirective(),
+  // and RealPathName and UniqueID are not preserved in the preamble.
+  // We use the FileEntry::Name, which is stable, interned into a "file index".
+  // The paths we want to expose are the RealPathName, so store those too.
+  std::vector<std::string> RealPathNames; // In file index order.
+  unsigned fileIndex(llvm::StringRef Name);
+  llvm::StringMap<unsigned> NameToIndex; // Values are file indexes.
+  // Maps a file's index to that of the files it includes.
+  llvm::DenseMap<unsigned, llvm::SmallVector<unsigned>> IncludeChildren;
 };
 
 /// Returns a PPCallback that visits all inclusions in the main file.
@@ -227,31 +205,4 @@ class IncludeInserter {
 } // namespace clangd
 } // namespace clang
 
-namespace llvm {
-
-// Support Tokens as DenseMap keys.
-template <> struct DenseMapInfo<clang::clangd::IncludeStructure::HeaderID> {
-  static inline clang::clangd::IncludeStructure::HeaderID getEmptyKey() {
-    return static_cast<clang::clangd::IncludeStructure::HeaderID>(
-        DenseMapInfo<unsigned>::getEmptyKey());
-  }
-
-  static inline clang::clangd::IncludeStructure::HeaderID getTombstoneKey() {
-    return static_cast<clang::clangd::IncludeStructure::HeaderID>(
-        DenseMapInfo<unsigned>::getTombstoneKey());
-  }
-
-  static unsigned
-  getHashValue(const clang::clangd::IncludeStructure::HeaderID &Tag) {
-    return hash_value(static_cast<unsigned>(Tag));
-  }
-
-  static bool isEqual(const clang::clangd::IncludeStructure::HeaderID &LHS,
-                      const clang::clangd::IncludeStructure::HeaderID &RHS) {
-    return LHS == RHS;
-  }
-};
-
-} // namespace llvm
-
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H