[clangd] Construct SymbolSlab from YAML format.

[hokein]

Summary

This will be used together with D40548 for the global index source (experimental).

[sammccall]

Nice! just nits

clangd/index/SymbolFromYAML.cpp

32 ↗

(On Diff #126742)

what does NP stand for?

44 ↗

(On Diff #126742)

Nit: for readability, I suggest you encode as a hex string instead.

We don't yet have operator<< for SymbolID, but I think we should, and it should be consistent.
So preferably define operator<< for SymbolID, and use it here, rather than just defining everything here.

clangd/index/SymbolFromYAML.h

1 ↗

(On Diff #126742)

nit: SymbolFromYAML.h seems slightly off for the file, because you support both conversions (and also multiple symbols)

Consider just YAML.h?

8 ↗

(On Diff #126742)

File comment, describing what's going on here, use cases.

In particular, I'd note that the format is designed for simplicity but isn't a suitable/efficient store and isn't intended for real use by end-users.

23 ↗

(On Diff #126742)

We may have multiple slabs, e.g. if we want to dump the dynamic index.
If we dump each slab and concatenate the resulting strings, is the result valid YAML (containing all symbols)?
If so, then this interface is good, but it may be worth a comment // The YAML is safe to concatenate if you have multiple slabs.

unittests/clangd/SymbolCollectorTests.cpp

108

round-trip is good to test the serialization, but can you add a separate test case loading a simple symbol from an actual YAML string?

[hokein]

Address the review comments.

[hokein]

Thanks for the comments!

clangd/index/SymbolFromYAML.cpp

32 ↗

(On Diff #126742)

Ah, P is a typo here. N stands for Normalized. Renamed it to NormalizedXXX.

44 ↗

(On Diff #126742)

Good idea. Done.

clangd/index/SymbolFromYAML.h

1 ↗

(On Diff #126742)

YAML seems too general. Named it SymbolYAML indicating the conversion between Symbol and YAML.

[sammccall]

clangd/index/Index.h

17

This isn't needed anymore (if you remove the friend below)

57

I think this no longer needs to be a friend.

[hokein]

MappingTraits is not Symbol's friend.

clangd/CMakeLists.txt

add_clang_library(clangDaemon
   JSONExpr.cpp
   JSONRPCDispatcher.cpp
   Logger.cpp
   Protocol.cpp
   ProtocolHandlers.cpp
   Trace.cpp
   index/Index.cpp
   index/SymbolCollector.cpp
+  index/SymbolYAML.cpp
 
   LINK_LIBS
   clangAST
   clangBasic
   clangFormat
   clangFrontend
   clangIndex
   clangLex

clangd/index/Index.h

 //===--- Symbol.h -----------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H
 
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/YAMLTraits.h"
 
 #include <array>
 #include <string>
 
 namespace clang {
 namespace clangd {
 
 struct SymbolLocation {
public:
   bool operator==(const SymbolID& Sym) const {
     return HashValue == Sym.HashValue;
   }
 
 private:
   friend llvm::hash_code hash_value(const SymbolID &ID) {
     return hash_value(ArrayRef<uint8_t>(ID.HashValue));
   }
+  friend struct llvm::yaml::MappingTraits<class SymbolID>;
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+                                       const SymbolID &ID);
+  friend void operator>>(llvm::StringRef Str, SymbolID &ID);
 
   std::array<uint8_t, 20> HashValue;
 };
 
+// Write SymbolID into the given stream. SymbolID is encoded as a 40-bytes
+// hex string.
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SymbolID &ID);
+
+// Construct SymbolID from a hex string.
+// The HexStr is required to be a 40-bytes hex string, which is encoded from the
+// "<<" operator.
+void operator>>(llvm::StringRef HexStr, SymbolID &ID);
+
 // The class presents a C++ symbol, e.g. class, function.
 //
 // FIXME: instead of having own copy fields for each symbol, we can share
 // storage from SymbolSlab.
 struct Symbol {
   // The ID of the symbol.
   SymbolID ID;
   // The qualified name of the symbol, e.g. Foo::bar.

clangd/index/Index.cpp

 //===--- Index.cpp -----------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Index.h"
 
 #include "llvm/Support/SHA1.h"
+#include "llvm/ADT/StringExtras.h"
 
 namespace clang {
 namespace clangd {
 
-namespace {
-ArrayRef<uint8_t> toArrayRef(StringRef S) {
-  return {reinterpret_cast<const uint8_t *>(S.data()), S.size()};
+SymbolID::SymbolID(llvm::StringRef USR)
+    : HashValue(llvm::SHA1::hash(arrayRefFromStringRef(USR))) {}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SymbolID &ID) {
+  OS << toHex(llvm::toStringRef(ID.HashValue));
+  return OS;
 }
-} // namespace
 
-SymbolID::SymbolID(llvm::StringRef USR)
-    : HashValue(llvm::SHA1::hash(toArrayRef(USR))) {}
+void operator>>(llvm::StringRef Str, SymbolID &ID) {
+  std::string HexString = fromHex(Str);
+  assert(HexString.size() == 20);
+  std::copy(HexString.begin(), HexString.end(), ID.HashValue.begin());
+}
 
 SymbolSlab::const_iterator SymbolSlab::begin() const {
   return Symbols.begin();
 }
 
 SymbolSlab::const_iterator SymbolSlab::end() const {
   return Symbols.end();
 }

clangd/index/SymbolYAML.h

+//===--- SymbolYAML.h --------------------------------------------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// SymbolYAML provides facilities to convert Symbol to YAML, and vice versa.
+// The YAML format of Symbol is designed for simplicity and experiment, but
+// isn't a suitable/efficient store.
+//
+// This is for **experimental** only. Don't use it in the production code.
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_FROM_YAML_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_FROM_YAML_H
+
+#include "Index.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+// Read symbols from a YAML-format string.
+SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent);
+
+// Convert symbols to a YAML-format string.
+// The YAML result is safe to concatenate if you have multiple symbol slabs.
+std::string SymbolToYAML(const SymbolSlab& Symbols);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_FROM_YAML_H

clangd/index/SymbolYAML.cpp

+//===--- SymbolYAML.cpp ------------------------------------------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SymbolYAML.h"
+
+#include "Index.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/YAMLTraits.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Errc.h"
+
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(clang::clangd::Symbol)
+
+namespace llvm {
+namespace yaml {
+
+using clang::clangd::Symbol;
+using clang::clangd::SymbolID;
+using clang::clangd::SymbolLocation;
+using clang::index::SymbolInfo;
+using clang::index::SymbolLanguage;
+using clang::index::SymbolKind;
+
+// Helper to (de)serialize the SymbolID. We serialize it as a hex string.
+struct NormalizedSymbolID {
+  NormalizedSymbolID(IO &) {}
+  NormalizedSymbolID(IO &, const SymbolID& ID) {
+    llvm::raw_string_ostream OS(HexString);
+    OS << ID;
+  }
+
+  SymbolID denormalize(IO&) {
+    SymbolID ID;
+    HexString >> ID;
+    return ID;
+  }
+
+  std::string HexString;
+};
+
+template <> struct MappingTraits<SymbolLocation> {
+  static void mapping(IO &IO, SymbolLocation &Value) {
+    IO.mapRequired("StartOffset", Value.StartOffset);
+    IO.mapRequired("EndOffset", Value.EndOffset);
+    IO.mapRequired("FilePath", Value.FilePath);
+  }
+};
+
+template <> struct MappingTraits<SymbolInfo> {
+  static void mapping(IO &io, SymbolInfo &SymInfo) {
+    // FIXME: expose other fields?
+    io.mapRequired("Kind", SymInfo.Kind);
+    io.mapRequired("Lang", SymInfo.Lang);
+  }
+};
+
+template<> struct MappingTraits<Symbol> {
+  static void mapping(IO &IO, Symbol &Sym) {
+    MappingNormalization<NormalizedSymbolID, SymbolID> NSymbolID(
+        IO, Sym.ID);
+    IO.mapRequired("ID", NSymbolID->HexString);
+    IO.mapRequired("QualifiedName", Sym.QualifiedName);
+    IO.mapRequired("SymInfo", Sym.SymInfo);
+    IO.mapRequired("CanonicalDeclaration", Sym.CanonicalDeclaration);
+  }
+};
+
+template <> struct ScalarEnumerationTraits<SymbolLanguage> {
+  static void enumeration(IO &IO, SymbolLanguage &Value) {
+    IO.enumCase(Value, "C", SymbolLanguage::C);
+    IO.enumCase(Value, "Cpp", SymbolLanguage::CXX);
+    IO.enumCase(Value, "ObjC", SymbolLanguage::ObjC);
+    IO.enumCase(Value, "Swift", SymbolLanguage::Swift);
+  }
+};
+
+template <> struct ScalarEnumerationTraits<SymbolKind> {
+  static void enumeration(IO &IO, SymbolKind &Value) {
+#define DEFINE_ENUM(name) IO.enumCase(Value, #name, SymbolKind::name)
+
+    DEFINE_ENUM(Unknown);
+    DEFINE_ENUM(Function);
+    DEFINE_ENUM(Module);
+    DEFINE_ENUM(Namespace);
+    DEFINE_ENUM(NamespaceAlias);
+    DEFINE_ENUM(Macro);
+    DEFINE_ENUM(Enum);
+    DEFINE_ENUM(Struct);
+    DEFINE_ENUM(Class);
+    DEFINE_ENUM(Protocol);
+    DEFINE_ENUM(Extension);
+    DEFINE_ENUM(Union);
+    DEFINE_ENUM(TypeAlias);
+    DEFINE_ENUM(Function);
+    DEFINE_ENUM(Variable);
+    DEFINE_ENUM(Field);
+    DEFINE_ENUM(EnumConstant);
+    DEFINE_ENUM(InstanceMethod);
+    DEFINE_ENUM(ClassMethod);
+    DEFINE_ENUM(StaticMethod);
+    DEFINE_ENUM(InstanceProperty);
+    DEFINE_ENUM(ClassProperty);
+    DEFINE_ENUM(StaticProperty);
+    DEFINE_ENUM(Constructor);
+    DEFINE_ENUM(Destructor);
+    DEFINE_ENUM(ConversionFunction);
+    DEFINE_ENUM(Parameter);
+    DEFINE_ENUM(Using);
+
+#undef DEFINE_ENUM
+  }
+};
+
+} // namespace yaml
+} // namespace llvm
+
+namespace clang {
+namespace clangd {
+
+SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent) {
+  std::vector<Symbol> S;
+  llvm::yaml::Input Yin(YAMLContent);
+  Yin >> S;
+  SymbolSlab Syms;
+  for (auto& Sym : S)
+    Syms.insert(std::move(Sym));
+  return Syms;
+}
+
+std::string SymbolToYAML(const SymbolSlab& Symbols) {
+  std::string Str;
+  llvm::raw_string_ostream OS(Str);
+  llvm::yaml::Output Yout(OS);
+  for (auto &Pair : Symbols) {
+    Symbol MutableSymbol = Pair.second;
+    Yout<< MutableSymbol;
+  }
+  return OS.str();
+}
+
+} // namespace clangd
+} // namespace clang

unittests/clangd/SymbolCollectorTests.cpp

 //===-- SymbolCollectorTests.cpp  -------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "index/SymbolCollector.h"
+#include "index/SymbolYAML.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/STLExtras.h"
const std::string Main = R"(
     void ff() {} // ignore
     }
     void f1() {}
   )";
   runSymbolCollector(Header, Main);
   EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
                                             QName("f1"), QName("f2")));
 }
 
+TEST_F(SymbolCollectorTest, YAMLConversions) {
+  const std::string YAML1 = R"(
+---
+ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
+QualifiedName:   'clang::Foo1'
+SymInfo:
+  Kind:            Function
+  Lang:            Cpp
+CanonicalDeclaration:
+  StartOffset:     0
+  EndOffset:       1
+  FilePath:        /path/foo.h
+...
+)";
+  const std::string YAML2 = R"(
+---
+ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858
+QualifiedName:   'clang::Foo2'
+SymInfo:
+  Kind:            Function
+  Lang:            Cpp
+CanonicalDeclaration:
+  StartOffset:     10
+  EndOffset:       12
+  FilePath:        /path/foo.h
+...
+)";
+
+  auto Symbols1 = SymbolFromYAML(YAML1);
+  EXPECT_THAT(Symbols1,
+              UnorderedElementsAre(QName("clang::Foo1")));
+  auto Symbols2 = SymbolFromYAML(YAML2);
+  EXPECT_THAT(Symbols2,
+              UnorderedElementsAre(QName("clang::Foo2")));
+
+  std::string ConcatenatedYAML =
+      SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
+  auto ConcatenatedSymbols = SymbolFromYAML(ConcatenatedYAML);
+  EXPECT_THAT(ConcatenatedSymbols,
+              UnorderedElementsAre(QName("clang::Foo1"),
+                                   QName("clang::Foo2")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang