[clangd] Use our own relation kind.

Summary:
Move the RelationKind from Serialization.h to Relation.h. This patch doesn't
introduce any breaking changes.

Reviewers: kadircet

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

Tags: #clang

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

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@375117 91177308-0d34-0410-b5e6-96231b3b80d8

19 files changed, 58 insertions, 117 deletions

diff --git a/clangd/XRefs.cpp b/clangd/XRefs.cpp
index f51891b8..43904cdd 100644
--- a/clangd/XRefs.cpp
+++ b/clangd/XRefs.cpp
@@ -18,6 +18,7 @@
 #include "URI.h"
 #include "index/Index.h"
 #include "index/Merge.h"
+#include "index/Relation.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolLocation.h"
 #include "clang/AST/ASTContext.h"
@@ -1107,7 +1108,7 @@ static void fillSubTypes(const SymbolID &ID,
                          const SymbolIndex *Index, int Levels, PathRef TUPath) {
   RelationsRequest Req;
   Req.Subjects.insert(ID);
-  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Req.Predicate = RelationKind::BaseOf;
   Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
     if (Optional<TypeHierarchyItem> ChildSym =
             symbolToTypeHierarchyItem(Object, Index, TUPath)) {
diff --git a/clangd/index/Index.h b/clangd/index/Index.h
index 016c0804..f579a754 100644
--- a/clangd/index/Index.h
+++ b/clangd/index/Index.h
@@ -75,7 +75,7 @@ struct RefsRequest {
 
 struct RelationsRequest {
   llvm::DenseSet<SymbolID> Subjects;
-  index::SymbolRole Predicate;
+  RelationKind Predicate;
   /// If set, limit the number of relations returned from the index.
   llvm::Optional<uint32_t> Limit;
 };
diff --git a/clangd/index/MemIndex.cpp b/clangd/index/MemIndex.cpp
index 8933f28f..1d46dbd6 100644
--- a/clangd/index/MemIndex.cpp
+++ b/clangd/index/MemIndex.cpp
@@ -92,7 +92,8 @@ void MemIndex::relations(
       Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max());
   for (const SymbolID &Subject : Req.Subjects) {
     LookupRequest LookupReq;
-    auto It = Relations.find(std::make_pair(Subject, Req.Predicate));
+    auto It = Relations.find(
+        std::make_pair(Subject, static_cast<uint8_t>(Req.Predicate)));
     if (It != Relations.end()) {
       for (const auto &Obj : It->second) {
         if (Remaining > 0) {
diff --git a/clangd/index/MemIndex.h b/clangd/index/MemIndex.h
index 527b0ebc..ad74f5a0 100644
--- a/clangd/index/MemIndex.h
+++ b/clangd/index/MemIndex.h
@@ -27,8 +27,9 @@ public:
     for (const std::pair<SymbolID, llvm::ArrayRef<Ref>> &R : Refs)
       this->Refs.try_emplace(R.first, R.second.begin(), R.second.end());
     for (const Relation &R : Relations)
-      this->Relations[std::make_pair(R.Subject, R.Predicate)].push_back(
-          R.Object);
+      this->Relations[std::make_pair(R.Subject,
+                                     static_cast<uint8_t>(R.Predicate))]
+          .push_back(R.Object);
   }
   // Symbols are owned by BackingData, Index takes ownership.
   template <typename SymbolRange, typename RefRange, typename RelationRange,
@@ -69,8 +70,9 @@ private:
   // A map from symbol ID to symbol refs, support query by IDs.
   llvm::DenseMap<SymbolID, llvm::ArrayRef<Ref>> Refs;
   // A map from (subject, predicate) pair to objects.
-  llvm::DenseMap<std::pair<SymbolID, index::SymbolRole>, std::vector<SymbolID>>
-      Relations;
+  static_assert(sizeof(RelationKind) == sizeof(uint8_t),
+                "RelationKind should be of same size as a uint8_t");
+  llvm::DenseMap<std::pair<SymbolID, uint8_t>, std::vector<SymbolID>> Relations;
   std::shared_ptr<void> KeepAlive; // poor man's move-only std::any
   // Size of memory retained by KeepAlive.
   size_t BackingDataSize = 0;
diff --git a/clangd/index/Relation.cpp b/clangd/index/Relation.cpp
index e46aa236..e28591bb 100644
--- a/clangd/index/Relation.cpp
+++ b/clangd/index/Relation.cpp
@@ -14,8 +14,7 @@ namespace clang {
 namespace clangd {
 
 llvm::iterator_range<RelationSlab::iterator>
-RelationSlab::lookup(const SymbolID &Subject,
-                     index::SymbolRole Predicate) const {
+RelationSlab::lookup(const SymbolID &Subject, RelationKind Predicate) const {
   auto IterPair = std::equal_range(Relations.begin(), Relations.end(),
                                    Relation{Subject, Predicate, SymbolID{}},
                                    [](const Relation &A, const Relation &B) {
diff --git a/clangd/index/Relation.h b/clangd/index/Relation.h
index 60675c4e..36300001 100644
--- a/clangd/index/Relation.h
+++ b/clangd/index/Relation.h
@@ -19,12 +19,16 @@
 namespace clang {
 namespace clangd {
 
+enum class RelationKind : uint8_t {
+  BaseOf,
+};
+
 /// Represents a relation between two symbols.
 /// For an example "A is a base class of B" may be represented
-/// as { Subject = A, Predicate = RelationBaseOf, Object = B }.
+/// as { Subject = A, Predicate = BaseOf, Object = B }.
 struct Relation {
   SymbolID Subject;
-  index::SymbolRole Predicate;
+  RelationKind Predicate;
   SymbolID Object;
 
   bool operator==(const Relation &Other) const {
@@ -59,7 +63,7 @@ public:
 
   /// Lookup all relations matching the given subject and predicate.
   llvm::iterator_range<iterator> lookup(const SymbolID &Subject,
-                                        index::SymbolRole Predicate) const;
+                                        RelationKind Predicate) const;
 
   /// RelationSlab::Builder is a mutable container that can 'freeze' to
   /// RelationSlab.
@@ -85,31 +89,4 @@ private:
 } // namespace clangd
 } // namespace clang
 
-namespace llvm {
-
-// Support index::SymbolRole as a DenseMap key for the purpose of looking up
-// relations.
-template <> struct DenseMapInfo<clang::index::SymbolRole> {
-  static inline clang::index::SymbolRole getEmptyKey() {
-    // Choose an enumerator that's not a relation.
-    return clang::index::SymbolRole::Declaration;
-  }
-
-  static inline clang::index::SymbolRole getTombstoneKey() {
-    // Choose another enumerator that's not a relation.
-    return clang::index::SymbolRole::Definition;
-  }
-
-  static unsigned getHashValue(const clang::index::SymbolRole &Key) {
-    return hash_value(Key);
-  }
-
-  static bool isEqual(const clang::index::SymbolRole &LHS,
-                      const clang::index::SymbolRole &RHS) {
-    return LHS == RHS;
-  }
-};
-
-} // namespace llvm
-
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
diff --git a/clangd/index/Serialization.cpp b/clangd/index/Serialization.cpp
index 90520fdc..219c896f 100644
--- a/clangd/index/Serialization.cpp
+++ b/clangd/index/Serialization.cpp
@@ -29,29 +29,6 @@ llvm::Error makeError(const llvm::Twine &Msg) {
   return llvm::make_error<llvm::StringError>(Msg,
                                              llvm::inconvertibleErrorCode());
 }
-} // namespace
-
-RelationKind symbolRoleToRelationKind(index::SymbolRole Role) {
-  // SymbolRole is used to record relations in the index.
-  // Only handle the relations we actually store currently.
-  // If we start storing more relations, this list can be expanded.
-  switch (Role) {
-  case index::SymbolRole::RelationBaseOf:
-    return RelationKind::BaseOf;
-  default:
-    llvm_unreachable("Unsupported symbol role");
-  }
-}
-
-index::SymbolRole relationKindToSymbolRole(RelationKind Kind) {
-  switch (Kind) {
-  case RelationKind::BaseOf:
-    return index::SymbolRole::RelationBaseOf;
-  }
-  llvm_unreachable("Invalid relation kind");
-}
-
-namespace {
 
 // IO PRIMITIVES
 // We use little-endian 32 bit ints, sometimes with variable-length encoding.
@@ -395,15 +372,13 @@ readRefs(Reader &Data, llvm::ArrayRef<llvm::StringRef> Strings) {
 
 void writeRelation(const Relation &R, llvm::raw_ostream &OS) {
   OS << R.Subject.raw();
-  RelationKind Kind = symbolRoleToRelationKind(R.Predicate);
-  OS.write(static_cast<uint8_t>(Kind));
+  OS.write(static_cast<uint8_t>(R.Predicate));
   OS << R.Object.raw();
 }
 
 Relation readRelation(Reader &Data) {
   SymbolID Subject = Data.consumeID();
-  index::SymbolRole Predicate =
-      relationKindToSymbolRole(static_cast<RelationKind>(Data.consume8()));
+  RelationKind Predicate = static_cast<RelationKind>(Data.consume8());
   SymbolID Object = Data.consumeID();
   return {Subject, Predicate, Object};
 }
diff --git a/clangd/index/Serialization.h b/clangd/index/Serialization.h
index 7ee6d5f6..47317c04 100644
--- a/clangd/index/Serialization.h
+++ b/clangd/index/Serialization.h
@@ -83,11 +83,6 @@ std::string toYAML(const Relation &);
 std::unique_ptr<SymbolIndex> loadIndex(llvm::StringRef Filename,
                                        bool UseDex = true);
 
-// Used for serializing SymbolRole as used in Relation.
-enum class RelationKind : uint8_t { BaseOf };
-RelationKind symbolRoleToRelationKind(index::SymbolRole);
-index::SymbolRole relationKindToSymbolRole(RelationKind);
-
 } // namespace clangd
 } // namespace clang
 
diff --git a/clangd/index/SymbolCollector.cpp b/clangd/index/SymbolCollector.cpp
index aca2626c..269a699b 100644
--- a/clangd/index/SymbolCollector.cpp
+++ b/clangd/index/SymbolCollector.cpp
@@ -445,8 +445,7 @@ void SymbolCollector::processRelations(
     //       in the index and find nothing, but that's a situation they
     //       probably need to handle for other reasons anyways.
     // We currently do (B) because it's simpler.
-    this->Relations.insert(
-        Relation{ID, index::SymbolRole::RelationBaseOf, *ObjectID});
+    this->Relations.insert(Relation{ID, RelationKind::BaseOf, *ObjectID});
   }
 }
 
diff --git a/clangd/index/YAMLSerialization.cpp b/clangd/index/YAMLSerialization.cpp
index 4e30abaa..8895d7a9 100644
--- a/clangd/index/YAMLSerialization.cpp
+++ b/clangd/index/YAMLSerialization.cpp
@@ -282,14 +282,11 @@ template <> struct MappingTraits<Ref> {
 
 struct NormalizedSymbolRole {
   NormalizedSymbolRole(IO &) {}
-  NormalizedSymbolRole(IO &IO, SymbolRole R) {
-    Kind = static_cast<uint8_t>(clang::clangd::symbolRoleToRelationKind(R));
+  NormalizedSymbolRole(IO &IO, RelationKind R) {
+    Kind = static_cast<uint8_t>(R);
   }
 
-  SymbolRole denormalize(IO &IO) {
-    return clang::clangd::relationKindToSymbolRole(
-        static_cast<RelationKind>(Kind));
-  }
+  RelationKind denormalize(IO &IO) { return static_cast<RelationKind>(Kind); }
 
   uint8_t Kind = 0;
 };
@@ -303,7 +300,7 @@ template <> struct MappingTraits<SymbolID> {
 
 template <> struct MappingTraits<Relation> {
   static void mapping(IO &IO, Relation &Relation) {
-    MappingNormalization<NormalizedSymbolRole, SymbolRole> NRole(
+    MappingNormalization<NormalizedSymbolRole, RelationKind> NRole(
         IO, Relation.Predicate);
     IO.mapRequired("Subject", Relation.Subject);
     IO.mapRequired("Predicate", NRole->Kind);
diff --git a/clangd/index/dex/Dex.cpp b/clangd/index/dex/Dex.cpp
index 996cd8b3..b39f17b5 100644
--- a/clangd/index/dex/Dex.cpp
+++ b/clangd/index/dex/Dex.cpp
@@ -271,7 +271,8 @@ void Dex::relations(
       Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max());
   for (const SymbolID &Subject : Req.Subjects) {
     LookupRequest LookupReq;
-    auto It = Relations.find(std::make_pair(Subject, Req.Predicate));
+    auto It = Relations.find(
+        std::make_pair(Subject, static_cast<uint8_t>(Req.Predicate)));
     if (It != Relations.end()) {
       for (const auto &Object : It->second) {
         if (Remaining > 0) {
diff --git a/clangd/index/dex/Dex.h b/clangd/index/dex/Dex.h
index de2994b0..9e7b9877 100644
--- a/clangd/index/dex/Dex.h
+++ b/clangd/index/dex/Dex.h
@@ -26,6 +26,7 @@
 #include "Trigram.h"
 #include "index/Index.h"
 #include "index/MemIndex.h"
+#include "index/Relation.h"
 #include "index/SymbolCollector.h"
 
 namespace clang {
@@ -49,8 +50,9 @@ public:
     for (auto &&Ref : Refs)
       this->Refs.try_emplace(Ref.first, Ref.second);
     for (auto &&Rel : Relations)
-      this->Relations[std::make_pair(Rel.Subject, Rel.Predicate)].push_back(
-          Rel.Object);
+      this->Relations[std::make_pair(Rel.Subject,
+                                     static_cast<uint8_t>(Rel.Predicate))]
+          .push_back(Rel.Object);
     buildIndex();
   }
   // Symbols and Refs are owned by BackingData, Index takes ownership.
@@ -106,8 +108,9 @@ private:
   llvm::DenseMap<Token, PostingList> InvertedIndex;
   dex::Corpus Corpus;
   llvm::DenseMap<SymbolID, llvm::ArrayRef<Ref>> Refs;
-  llvm::DenseMap<std::pair<SymbolID, index::SymbolRole>, std::vector<SymbolID>>
-      Relations;
+  static_assert(sizeof(RelationKind) == sizeof(uint8_t),
+                "RelationKind should be of same size as a uint8_t");
+  llvm::DenseMap<std::pair<SymbolID, uint8_t>, std::vector<SymbolID>> Relations;
   std::shared_ptr<void> KeepAlive; // poor man's move-only std::any
   // Size of memory retained by KeepAlive.
   size_t BackingDataSize = 0;
diff --git a/clangd/unittests/BackgroundIndexTests.cpp b/clangd/unittests/BackgroundIndexTests.cpp
index e1cbcfd0..c01910e4 100644
--- a/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clangd/unittests/BackgroundIndexTests.cpp
@@ -239,9 +239,8 @@ TEST_F(BackgroundIndexTest, ShardStorageTest) {
   // containing the definition of the subject (A_CC)
   SymbolID A = findSymbol(*ShardHeader->Symbols, "A_CC").ID;
   SymbolID B = findSymbol(*ShardSource->Symbols, "B_CC").ID;
-  EXPECT_THAT(
-      *ShardHeader->Relations,
-      UnorderedElementsAre(Relation{A, index::SymbolRole::RelationBaseOf, B}));
+  EXPECT_THAT(*ShardHeader->Relations,
+              UnorderedElementsAre(Relation{A, RelationKind::BaseOf, B}));
   // (and not in the file containing the definition of the object (B_CC)).
   EXPECT_EQ(ShardSource->Relations->size(), 0u);
 }
diff --git a/clangd/unittests/DexTests.cpp b/clangd/unittests/DexTests.cpp
index 290dc939..7e4fde9a 100644
--- a/clangd/unittests/DexTests.cpp
+++ b/clangd/unittests/DexTests.cpp
@@ -705,16 +705,15 @@ TEST(DexTests, Relations) {
 
   std::vector<Symbol> Symbols{Parent, Child1, Child2};
 
-  std::vector<Relation> Relations{
-      {Parent.ID, index::SymbolRole::RelationBaseOf, Child1.ID},
-      {Parent.ID, index::SymbolRole::RelationBaseOf, Child2.ID}};
+  std::vector<Relation> Relations{{Parent.ID, RelationKind::BaseOf, Child1.ID},
+                                  {Parent.ID, RelationKind::BaseOf, Child2.ID}};
 
   Dex I{Symbols, RefSlab(), Relations};
 
   std::vector<SymbolID> Results;
   RelationsRequest Req;
   Req.Subjects.insert(Parent.ID);
-  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Req.Predicate = RelationKind::BaseOf;
   I.relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
     Results.push_back(Object.ID);
   });
diff --git a/clangd/unittests/FileIndexTests.cpp b/clangd/unittests/FileIndexTests.cpp
index 61f6e60c..bc072178 100644
--- a/clangd/unittests/FileIndexTests.cpp
+++ b/clangd/unittests/FileIndexTests.cpp
@@ -364,7 +364,7 @@ TEST(FileIndexTest, Relations) {
   uint32_t Results = 0;
   RelationsRequest Req;
   Req.Subjects.insert(A);
-  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Req.Predicate = RelationKind::BaseOf;
   Index.relations(Req, [&](const SymbolID &, const Symbol &) { ++Results; });
   EXPECT_EQ(Results, 1u);
 }
diff --git a/clangd/unittests/IndexTests.cpp b/clangd/unittests/IndexTests.cpp
index d4017c22..9f0265d4 100644
--- a/clangd/unittests/IndexTests.cpp
+++ b/clangd/unittests/IndexTests.cpp
@@ -83,20 +83,15 @@ TEST(RelationSlab, Lookup) {
   SymbolID D{"D"};
 
   RelationSlab::Builder Builder;
-  Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, B});
-  Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, C});
-  Builder.insert(Relation{B, index::SymbolRole::RelationBaseOf, D});
-  Builder.insert(Relation{C, index::SymbolRole::RelationBaseOf, D});
-  Builder.insert(Relation{B, index::SymbolRole::RelationChildOf, A});
-  Builder.insert(Relation{C, index::SymbolRole::RelationChildOf, A});
-  Builder.insert(Relation{D, index::SymbolRole::RelationChildOf, B});
-  Builder.insert(Relation{D, index::SymbolRole::RelationChildOf, C});
+  Builder.insert(Relation{A, RelationKind::BaseOf, B});
+  Builder.insert(Relation{A, RelationKind::BaseOf, C});
+  Builder.insert(Relation{B, RelationKind::BaseOf, D});
+  Builder.insert(Relation{C, RelationKind::BaseOf, D});
 
   RelationSlab Slab = std::move(Builder).build();
-  EXPECT_THAT(
-      Slab.lookup(A, index::SymbolRole::RelationBaseOf),
-      UnorderedElementsAre(Relation{A, index::SymbolRole::RelationBaseOf, B},
-                           Relation{A, index::SymbolRole::RelationBaseOf, C}));
+  EXPECT_THAT(Slab.lookup(A, RelationKind::BaseOf),
+              UnorderedElementsAre(Relation{A, RelationKind::BaseOf, B},
+                                   Relation{A, RelationKind::BaseOf, C}));
 }
 
 TEST(RelationSlab, Duplicates) {
@@ -105,14 +100,13 @@ TEST(RelationSlab, Duplicates) {
   SymbolID C{"C"};
 
   RelationSlab::Builder Builder;
-  Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, B});
-  Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, C});
-  Builder.insert(Relation{A, index::SymbolRole::RelationBaseOf, B});
+  Builder.insert(Relation{A, RelationKind::BaseOf, B});
+  Builder.insert(Relation{A, RelationKind::BaseOf, C});
+  Builder.insert(Relation{A, RelationKind::BaseOf, B});
 
   RelationSlab Slab = std::move(Builder).build();
-  EXPECT_THAT(Slab, UnorderedElementsAre(
-                        Relation{A, index::SymbolRole::RelationBaseOf, B},
-                        Relation{A, index::SymbolRole::RelationBaseOf, C}));
+  EXPECT_THAT(Slab, UnorderedElementsAre(Relation{A, RelationKind::BaseOf, B},
+                                         Relation{A, RelationKind::BaseOf, C}));
 }
 
 TEST(SwapIndexTest, OldIndexRecycled) {
diff --git a/clangd/unittests/SerializationTests.cpp b/clangd/unittests/SerializationTests.cpp
index 4abb6f3e..89739f42 100644
--- a/clangd/unittests/SerializationTests.cpp
+++ b/clangd/unittests/SerializationTests.cpp
@@ -152,9 +152,9 @@ TEST(SerializationTest, YAMLConversions) {
   SymbolID Base = cantFail(SymbolID::fromStr("6481EE7AF2841756"));
   SymbolID Derived = cantFail(SymbolID::fromStr("6512AEC512EA3A2D"));
   ASSERT_TRUE(bool(ParsedYAML->Relations));
-  EXPECT_THAT(*ParsedYAML->Relations,
-              UnorderedElementsAre(
-                  Relation{Base, index::SymbolRole::RelationBaseOf, Derived}));
+  EXPECT_THAT(
+      *ParsedYAML->Relations,
+      UnorderedElementsAre(Relation{Base, RelationKind::BaseOf, Derived}));
 }
 
 std::vector<std::string> YAMLFromSymbols(const SymbolSlab &Slab) {
diff --git a/clangd/unittests/SymbolCollectorTests.cpp b/clangd/unittests/SymbolCollectorTests.cpp
index d0614ec4..af1729e3 100644
--- a/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clangd/unittests/SymbolCollectorTests.cpp
@@ -673,8 +673,7 @@ TEST_F(SymbolCollectorTest, Relations) {
   const Symbol &Base = findSymbol(Symbols, "Base");
   const Symbol &Derived = findSymbol(Symbols, "Derived");
   EXPECT_THAT(Relations,
-              Contains(Relation{Base.ID, index::SymbolRole::RelationBaseOf,
-                                Derived.ID}));
+              Contains(Relation{Base.ID, RelationKind::BaseOf, Derived.ID}));
 }
 
 TEST_F(SymbolCollectorTest, References) {
diff --git a/clangd/unittests/TypeHierarchyTests.cpp b/clangd/unittests/TypeHierarchyTests.cpp
index bc9a1c6f..252c421e 100644
--- a/clangd/unittests/TypeHierarchyTests.cpp
+++ b/clangd/unittests/TypeHierarchyTests.cpp
@@ -482,7 +482,7 @@ std::vector<SymbolID> collectSubtypes(SymbolID Subject, SymbolIndex *Index) {
   std::vector<SymbolID> Result;
   RelationsRequest Req;
   Req.Subjects.insert(Subject);
-  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Req.Predicate = RelationKind::BaseOf;
   Index->relations(Req,
                    [&Result](const SymbolID &Subject, const Symbol &Object) {
                      Result.push_back(Object.ID);