[clangd] Replace StringRef in SymbolLocation with a char pointer.

[hokein]

Summary

This would save us 8 bytes per ref, and buy us ~50MB in total for llvm
index (from ~350MB to ~300 MB).

The char pointer must be null-terminated, and llvm::StringSaver
guarantees it.

[sammccall]

Nice savings!

My initial thought is that it seems weird to only do it for this one string - the readers look different, the serialization code has different logic etc for the different flavors of strings.
The struct certainly looks nicer to my eyes.

This may be the right tradeoff, but I'd like to play with/think about other options for API here. One is to define a class like NullTerminatedStringRef (with a better name) that wraps a char* only, but otherwise behaves as much like stringref as we can manage. Happy for you to try some of these out or to try them myself.

I think this is worth a bit of thought before committing because as shown in this patch, these APIs have quite a lot of uses!

[hokein]

Move forward the patch based on the offline discussion.

[MaskRay]

clangd/index/Index.h

84 ↗

(On Diff #172949)

The StringRef(const char*) constructor calls strlen. Do you believe this does not have noticeable performance issue?

(A minor point: tuple::operator< calls operator< of its components in a not very efficient way. The compiler does not know compare(a, b) < 0 implies compare(b, a) > 0 and strcmp may end up being called twice.)

[hokein]

update the patch.

[hokein]

clangd/index/Index.h

84 ↗

(On Diff #172949)

Thanks for the suggestions. The code here has an addition strlen cost. I updated the patch to avoid this additional cost and the potential double < calls of string.

[sammccall]

clangd/index/Index.h

85 ↗

(On Diff #173842)

nit: I think it's more idiomatic just to inline as

return !strcmp(L.FileURI, R.FileURI) && ...

93 ↗

(On Diff #173842)

tie(strcmp(L.FileURI, R.FileURI), ...) < tie(0, ...)?

306 ↗

(On Diff #173842)

assert (!S.data()[S.size()] && "Visited StringRef must be null-terminated")

clangd/index/Merge.cpp

118 ↗

(On Diff #173842)

Since this reads awkwardly anyway...

bool(*L.Definition.FileURI) == bool(*R.definition.FileURI)

is shorter and faster

[hokein]

Address review comments.

clangd/index/Index.h

93 ↗

(On Diff #173842)

tie doesn't support this usage, it expects lvalue arguments.

306 ↗

(On Diff #173842)

Does this assert actually work? The StringRef is constructed from the char * (which using strlen). This assert will not get triggered I think, S.data()[S.size()] is always \0.

clangd/index/Merge.cpp

118 ↗

(On Diff #173842)

this is neat!

[sammccall]

clangd/index/Index.h

306 ↗

(On Diff #173842)

The point is the callback takes a mutable reference, and can replace the stringref.
We're asserting that the *new* stringref is nullterminated (so we can convert it safely to a char*)

[hokein]

Add assert.

[hokein]

clangd/index/Index.h

306 ↗

(On Diff #173842)

Ah, that makes sense, I missed the fact that the CB mutates S.

[alexfh]

clang-tools-extra/trunk/clangd/index/Index.h

71

If the users of the SymbolLocation have a way to get a common "context", you could keep a list of filenames in this context and just store an index into this list (which could be even shorter). If the structure is used where no context is available, you could still keep a list of StringRefs somewhere (probably close to where the string data pointed by the StringRefs lives) and keep a StringRef * here. At a cost of one indirection you'd avoid calling strlen.

Not sure what trade-off is better for this particular use case though.

[hokein]

clang-tools-extra/trunk/clangd/index/Index.h

71

SymbolLocation is a public interface, and the underlying data is stored in arena (which is not visible to clients), so clients don't know any Context except SymbolLocation.

Using StringRef * is an interesting idea -- keeping a list of StringRefs needs extra space, the number should be relatively small (only the number of different files), so we will spend extra <num of files> * sizeof(StringRef) bytes (for llvm, ~128 KB memory) , compared with the raw pointer solution.

[alexfh]

clang-tools-extra/trunk/clangd/index/Index.h

71

Another option mentioned by Ilya offline is similar to MS's BSTR: keep the length along with the string data in the arena. Something like:

class InlineString {
  size_t Length;
  char Data[0];  // Open-ended array with a null-terminated string in it.
 public:
   operator StringRef() const { return StringRef(&Data[0], Length); }
   ...
};

And then keep const InlineString * here.

[sammccall]

clang-tools-extra/trunk/clangd/index/Index.h

71

Yeah, operator StringRef() doesn't really work well unfortunately (I tried a StringRefz which just wrapped a char pointer, in a bid to avoid accidentally using pointer semantics).

Conversion rules are too complicated and existing APIs are already maximally constrained, so you can't satisfy all of {functions that take StringRef only, functions that take Twine only, functions that overload for StringRef/char*/std::string, functions that *also* overload for Twine, ...}

clang-tools-extra/trunk/clangd/index/Index.h

struct Position {
     static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
   private:
     uint32_t Line : 20; // 0-based
     // Using UTF-16 code units.
     uint32_t Column : 12; // 0-based
   };
 
-  // The URI of the source file where a symbol occurs.
-  llvm::StringRef FileURI;
-
   /// The symbol range, using half-open range [Start, End).
   Position Start;
   Position End;
 
-  explicit operator bool() const { return !FileURI.empty(); }
+  explicit operator bool() const { return !StringRef(FileURI).empty(); }
+
+  // The URI of the source file where a symbol occurs.
+  // The string must be null-terminated.
+  //
+  // We avoid using llvm::StringRef here to save memory.
+  // WARNING: unless you know what you are doing, it is recommended to use it
+  // via llvm::StringRef.
+  const char *FileURI = "";
 };
 inline bool operator==(const SymbolLocation::Position &L,
                        const SymbolLocation::Position &R) {
   return std::make_tuple(L.line(), L.column()) ==
          std::make_tuple(R.line(), R.column());
 }
 inline bool operator<(const SymbolLocation::Position &L,
                       const SymbolLocation::Position &R) {
   return std::make_tuple(L.line(), L.column()) <
          std::make_tuple(R.line(), R.column());
 }
 inline bool operator==(const SymbolLocation &L, const SymbolLocation &R) {
-  return std::tie(L.FileURI, L.Start, L.End) ==
-         std::tie(R.FileURI, R.Start, R.End);
+  assert(L.FileURI && R.FileURI);
+  return !std::strcmp(L.FileURI, R.FileURI) &&
+         std::tie(L.Start, L.End) == std::tie(R.Start, R.End);
 }
 inline bool operator<(const SymbolLocation &L, const SymbolLocation &R) {
-  return std::tie(L.FileURI, L.Start, L.End) <
-         std::tie(R.FileURI, R.Start, R.End);
+  assert(L.FileURI && R.FileURI);
+  int Cmp = std::strcmp(L.FileURI, R.FileURI);
+  if (Cmp != 0)
+    return Cmp < 0;
+  return std::tie(L.Start, L.End) < std::tie(R.Start, R.End);
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
 
 // The class identifies a particular C++ symbol (class, function, method, etc).
 //
 // As USRs (Unified Symbol Resolution) could be large, especially for functions
 // with long type arguments, SymbolID is using truncated SHA1(USR) values to
 // guarantee the uniqueness of symbols while using a relatively small amount of

 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S);
 raw_ostream &operator<<(raw_ostream &, Symbol::SymbolFlag);
 
 // Invokes Callback with each StringRef& contained in the Symbol.
 // Useful for deduplicating backing strings.
 template <typename Callback> void visitStrings(Symbol &S, const Callback &CB) {
   CB(S.Name);
   CB(S.Scope);
-  CB(S.CanonicalDeclaration.FileURI);
-  CB(S.Definition.FileURI);
   CB(S.Signature);
   CB(S.CompletionSnippetSuffix);
   CB(S.Documentation);
   CB(S.ReturnType);
+  auto RawCharPointerCB = [&CB](const char *&P) {
+    llvm::StringRef S(P);
+    CB(S);
+    assert(!S.data()[S.size()] && "Visited StringRef must be null-terminated");
+    P = S.data();
+  };
+  RawCharPointerCB(S.CanonicalDeclaration.FileURI);
+  RawCharPointerCB(S.Definition.FileURI);
+
   for (auto &Include : S.IncludeHeaders)
     CB(Include.IncludeHeader);
 }
 
 // Computes query-independent quality score for a Symbol.
 // This currently falls in the range [1, ln(#indexed documents)].
 // FIXME: this should probably be split into symbol -> signals
 //        and signals -> score, so it can be reused for Sema completions.

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

 
 raw_ostream &operator<<(raw_ostream &OS, const Ref &R) {
   return OS << R.Location << ":" << R.Kind;
 }
 
 void RefSlab::Builder::insert(const SymbolID &ID, const Ref &S) {
   auto &M = Refs[ID];
   M.push_back(S);
-  M.back().Location.FileURI = UniqueStrings.save(M.back().Location.FileURI);
+  M.back().Location.FileURI =
+      UniqueStrings.save(M.back().Location.FileURI).data();
 }
 
 RefSlab RefSlab::Builder::build() && {
   // We can reuse the arena, as it only has unique strings and we need them all.
   // Reallocate refs on the arena to reduce waste and indirections when reading.
   std::vector<std::pair<SymbolID, ArrayRef<Ref>>> Result;
   Result.reserve(Refs.size());
   size_t NumRefs = 0;

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

 Symbol mergeSymbol(const Symbol &L, const Symbol &R) {
   assert(L.ID == R.ID);
   // We prefer information from TUs that saw the definition.
   // Classes: this is the def itself. Functions: hopefully the header decl.
   // If both did (or both didn't), continue to prefer L over R.
   bool PreferR = R.Definition && !L.Definition;
   // Merge include headers only if both have definitions or both have no
   // definition; otherwise, only accumulate references of common includes.
+  assert(L.Definition.FileURI && R.Definition.FileURI);
   bool MergeIncludes =
-      L.Definition.FileURI.empty() == R.Definition.FileURI.empty();
+      bool(*L.Definition.FileURI) == bool(*R.Definition.FileURI);
   Symbol S = PreferR ? R : L;        // The target symbol we're merging into.
   const Symbol &O = PreferR ? L : R; // The "other" less-preferred symbol.
 
   // For each optional field, fill it from O if missing in S.
   // (It might be missing in O too, but that's a no-op).
   if (!S.Definition)
     S.Definition = O.Definition;
   if (!S.CanonicalDeclaration)

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

void writeLocation(const SymbolLocation &Loc, const StringTableOut &Strings,
   for (const auto &Endpoint : {Loc.Start, Loc.End}) {
     writeVar(Endpoint.line(), OS);
     writeVar(Endpoint.column(), OS);
   }
 }
 
 SymbolLocation readLocation(Reader &Data, ArrayRef<StringRef> Strings) {
   SymbolLocation Loc;
-  Loc.FileURI = Data.consumeString(Strings);
+  Loc.FileURI = Data.consumeString(Strings).data();
   for (auto *Endpoint : {&Loc.Start, &Loc.End}) {
     Endpoint->setLine(Data.consumeVar());
     Endpoint->setColumn(Data.consumeVar());
   }
   return Loc;
 }
 
 void writeSymbol(const Symbol &Sym, const StringTableOut &Strings,
void writeRIFF(const IndexFileOut &Data, raw_ostream &OS) {
   for (const auto &Sym : *Data.Symbols) {
     Symbols.emplace_back(Sym);
     visitStrings(Symbols.back(), [&](StringRef &S) { Strings.intern(S); });
   }
   std::vector<std::pair<SymbolID, std::vector<Ref>>> Refs;
   if (Data.Refs) {
     for (const auto &Sym : *Data.Refs) {
       Refs.emplace_back(Sym);
-      for (auto &Ref : Refs.back().second)
-        Strings.intern(Ref.Location.FileURI);
+      for (auto &Ref : Refs.back().second) {
+        StringRef File = Ref.Location.FileURI;
+        Strings.intern(File);
+        Ref.Location.FileURI = File.data();
+      }
     }
   }
 
   std::string StringSection;
   {
     raw_string_ostream StringOS(StringSection);
     Strings.finalize(StringOS);
   }

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

Optional<SymbolLocation> getTokenLocation(SourceLocation TokLoc,
                                           const SymbolCollector::Options &Opts,
                                           const clang::LangOptions &LangOpts,
                                           std::string &FileURIStorage) {
   auto U = toURI(SM, SM.getFilename(TokLoc), Opts);
   if (!U)
     return None;
   FileURIStorage = std::move(*U);
   SymbolLocation Result;
-  Result.FileURI = FileURIStorage;
+  Result.FileURI = FileURIStorage.c_str();
   auto Range = getTokenRange(TokLoc, SM, LangOpts);
   Result.Start = Range.first;
   Result.End = Range.second;
 
   return std::move(Result);
 }
 
 // Checks whether \p ND is a definition of a TagDecl (class/struct/enum/union)
for (const auto &It : DeclRefs) {
           // FIXME: use the result to filter out references.
           shouldIndexFile(SM, FileID, Opts, &FilesToIndexCache);
           if (auto FileURI = GetURI(FileID)) {
             auto Range =
                 getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
             Ref R;
             R.Location.Start = Range.first;
             R.Location.End = Range.second;
-            R.Location.FileURI = *FileURI;
+            R.Location.FileURI = FileURI->c_str();
             R.Kind = toRefKind(LocAndRole.second);
             Refs.insert(*ID, R);
           }
         }
       }
     }
   }
 

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

 
 #include "Index.h"
 #include "Serialization.h"
 #include "Trace.h"
 #include "dex/Dex.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Allocator.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/StringSaver.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstdint>
 
 using namespace llvm;
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Symbol::IncludeHeaderWithReferences)
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Ref)
Position denormalize(IO &) {
     Position Pos;
     Pos.setLine(P.Line);
     Pos.setColumn(P.Column);
     return Pos;
   }
   YPosition P;
 };
 
+struct NormalizedFileURI {
+  NormalizedFileURI(IO &) {}
+  NormalizedFileURI(IO &, const char *FileURI) { URI = FileURI; }
+
+  const char *denormalize(IO &IO) {
+    assert(IO.getContext() &&
+           "Expecting an UniqueStringSaver to allocate data");
+    return static_cast<llvm::UniqueStringSaver *>(IO.getContext())
+        ->save(URI)
+        .data();
+  }
+
+  std::string URI;
+};
+
 template <> struct MappingTraits<SymbolLocation> {
   static void mapping(IO &IO, SymbolLocation &Value) {
-    IO.mapRequired("FileURI", Value.FileURI);
+    MappingNormalization<NormalizedFileURI, const char *> NFile(IO,
+                                                                Value.FileURI);
+    IO.mapRequired("FileURI", NFile->URI);
     MappingNormalization<NormalizedPosition, SymbolLocation::Position> NStart(
         IO, Value.Start);
     IO.mapRequired("Start", NStart->P);
     MappingNormalization<NormalizedPosition, SymbolLocation::Position> NEnd(
         IO, Value.End);
     IO.mapRequired("End", NEnd->P);
   }
 };
for (auto &Sym : *O.Refs) {
       Entry.Refs = Sym;
       Yout << Entry;
     }
 }
 
 Expected<IndexFileIn> readYAML(StringRef Data) {
   SymbolSlab::Builder Symbols;
   RefSlab::Builder Refs;
-  yaml::Input Yin(Data);
+  BumpPtrAllocator Arena; // store the underlying data of Position::FileURI.
+  UniqueStringSaver Strings(Arena);
+  yaml::Input Yin(Data, &Strings);
   do {
     VariantEntry Variant;
     Yin >> Variant;
     if (Yin.error())
       return errorCodeToError(Yin.error());
     if (Variant.Symbol)
       Symbols.insert(*Variant.Symbol);
     if (Variant.Refs)

clang-tools-extra/trunk/clangd/index/dex/Dex.cpp

 // trigrams and scopes.
 // FIXME(kbobyrev): Support more token types:
 // * Types
 // * Namespace proximity
 std::vector<Token> generateSearchTokens(const Symbol &Sym) {
   std::vector<Token> Result = generateIdentifierTrigrams(Sym.Name);
   Result.emplace_back(Token::Kind::Scope, Sym.Scope);
   // Skip token generation for symbols with unknown declaration location.
-  if (!Sym.CanonicalDeclaration.FileURI.empty())
+  if (!StringRef(Sym.CanonicalDeclaration.FileURI).empty())
     for (const auto &ProximityURI :
          generateProximityURIs(Sym.CanonicalDeclaration.FileURI))
       Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
   if (Sym.Flags & Symbol::IndexedForCodeCompletion)
     Result.emplace_back(RestrictedForCodeCompletion);
   return Result;
 }
 

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

 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "index/Background.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using testing::_;
 using testing::AllOf;
 using testing::Not;
 using testing::UnorderedElementsAre;
 
+using namespace llvm;
 namespace clang {
 namespace clangd {
 
 MATCHER_P(Named, N, "") { return arg.Name == N; }
-MATCHER(Declared, "") { return !arg.CanonicalDeclaration.FileURI.empty(); }
-MATCHER(Defined, "") { return !arg.Definition.FileURI.empty(); }
-
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+MATCHER(Declared, "") {
+  return !StringRef(arg.CanonicalDeclaration.FileURI).empty();
+}
+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)));
 }
 
 TEST(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
   // a.h yields different symbols when included by A.cc vs B.cc.

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

TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) {
   CDB.ExtraClangFlags = {SearchDirArg.c_str()};
   std::string BarHeader = testPath("sub/bar.h");
   FS.Files[BarHeader] = "";
 
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   auto BarURI = URI::createFile(BarHeader).toString();
   Symbol Sym = cls("ns::X");
-  Sym.CanonicalDeclaration.FileURI = BarURI;
+  Sym.CanonicalDeclaration.FileURI = BarURI.c_str();
   Sym.IncludeHeaders.emplace_back(BarURI, 1);
   // Shoten include path based on search dirctory and insert.
   auto Results = completions(Server,
                              R"cpp(
           int main() { ns::^ }
       )cpp",
                              {Sym});
   EXPECT_THAT(Results.Completions,
TEST(CompletionTest, NoIncludeInsertionWhenDeclFoundInFile) {
   MockCompilationDatabase CDB;
 
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   Symbol SymX = cls("ns::X");
   Symbol SymY = cls("ns::Y");
   std::string BarHeader = testPath("bar.h");
   auto BarURI = URI::createFile(BarHeader).toString();
-  SymX.CanonicalDeclaration.FileURI = BarURI;
-  SymY.CanonicalDeclaration.FileURI = BarURI;
+  SymX.CanonicalDeclaration.FileURI = BarURI.c_str();
+  SymY.CanonicalDeclaration.FileURI = BarURI.c_str();
   SymX.IncludeHeaders.emplace_back("<bar>", 1);
   SymY.IncludeHeaders.emplace_back("<bar>", 1);
   // Shoten include path based on search dirctory and insert.
   auto Results = completions(Server,
                              R"cpp(
           namespace ns {
             class X;
             class Y {}
TEST(CompletionTest, OverloadBundling) {
   // Non-member completions are bundled, including index+sema.
   Symbol NoArgsGFunc = func("GFuncC");
   EXPECT_THAT(
       completions(Context + "int y = GFunc^", {NoArgsGFunc}, Opts).Completions,
       UnorderedElementsAre(Labeled("GFuncC(…)"), Labeled("GFuncD(int)")));
 
   // Differences in header-to-insert suppress bundling.
   std::string DeclFile = URI::createFile(testPath("foo")).toString();
-  NoArgsGFunc.CanonicalDeclaration.FileURI = DeclFile;
+  NoArgsGFunc.CanonicalDeclaration.FileURI = DeclFile.c_str();
   NoArgsGFunc.IncludeHeaders.emplace_back("<foo>", 1);
   EXPECT_THAT(
       completions(Context + "int y = GFunc^", {NoArgsGFunc}, Opts).Completions,
       UnorderedElementsAre(AllOf(Named("GFuncC"), InsertInclude("<foo>")),
                            Labeled("GFuncC(int)"), Labeled("GFuncD(int)")));
 
   // Examine a bundled completion in detail.
   auto A =
TEST(CompletionTest, EnableSpeculativeIndexRequest) {
   // request after sema.
   auto Reqs3 = Requests.consumeRequests();
   ASSERT_EQ(Reqs3.size(), 2u);
 }
 
 TEST(CompletionTest, InsertTheMostPopularHeader) {
   std::string DeclFile = URI::createFile(testPath("foo")).toString();
   Symbol sym = func("Func");
-  sym.CanonicalDeclaration.FileURI = DeclFile;
+  sym.CanonicalDeclaration.FileURI = DeclFile.c_str();
   sym.IncludeHeaders.emplace_back("\"foo.h\"", 2);
   sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
 
   auto Results = completions("Fun^", {sym}).Completions;
   assert(!Results.empty());
   EXPECT_THAT(Results[0], AllOf(Named("Func"), InsertInclude("\"bar.h\"")));
   EXPECT_EQ(Results[0].Includes.size(), 2u);
 }
 
 TEST(CompletionTest, NoInsertIncludeIfOnePresent) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
 
   std::string FooHeader = testPath("foo.h");
   FS.Files[FooHeader] = "";
 
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   std::string DeclFile = URI::createFile(testPath("foo")).toString();
   Symbol sym = func("Func");
-  sym.CanonicalDeclaration.FileURI = DeclFile;
+  sym.CanonicalDeclaration.FileURI = DeclFile.c_str();
   sym.IncludeHeaders.emplace_back("\"foo.h\"", 2);
   sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
 
   EXPECT_THAT(
       completions(Server, "#include \"foo.h\"\nFun^", {sym}).Completions,
       UnorderedElementsAre(
           AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude()))));
 }

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

TEST(DexTest, ProximityPathsBoosting) {
   // FuzzyFind request comes from the file which is close to the root: expect
   // RootSymbol to come out.
   Req.ProximityPaths = {testPath("file.h")};
   EXPECT_THAT(match(I, Req), ElementsAre("root::abc"));
 }
 
 TEST(DexTests, Refs) {
   DenseMap<SymbolID, std::vector<Ref>> Refs;
-  auto AddRef = [&](const Symbol& Sym, StringRef Filename, RefKind Kind) {
+  auto AddRef = [&](const Symbol &Sym, const char *Filename, RefKind Kind) {
     auto& SymbolRefs = Refs[Sym.ID];
     SymbolRefs.emplace_back();
     SymbolRefs.back().Kind = Kind;
     SymbolRefs.back().Location.FileURI = Filename;
   };
   auto Foo = symbol("foo");
   auto Bar = symbol("bar");
   AddRef(Foo, "foo.h", RefKind::Declaration);

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

 
 using testing::_;
 using testing::AllOf;
 using testing::Contains;
 using testing::ElementsAre;
 using testing::IsEmpty;
 using testing::Pair;
 using testing::UnorderedElementsAre;
+using namespace llvm;
 
 MATCHER_P(RefRange, Range, "") {
   return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
                          arg.Location.End.line(), arg.Location.End.column()) ==
          std::make_tuple(Range.start.line, Range.start.character,
                          Range.end.line, Range.end.character);
 }
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
-MATCHER_P(DeclURI, U, "") { return arg.CanonicalDeclaration.FileURI == U; }
-MATCHER_P(DefURI, U, "") { return arg.Definition.FileURI == U; }
+MATCHER_P(FileURI, F, "") { return StringRef(arg.Location.FileURI) == F; }
+MATCHER_P(DeclURI, U, "") {
+  return StringRef(arg.CanonicalDeclaration.FileURI) == U;
+}
+MATCHER_P(DefURI, U, "") { return StringRef(arg.Definition.FileURI) == U; }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
 
-using namespace llvm;
 namespace clang {
 namespace clangd {
 namespace {
 testing::Matcher<const RefSlab &>
 RefsAre(std::vector<testing::Matcher<Ref>> Matchers) {
   return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers)));
 }
 
 Symbol symbol(StringRef ID) {
   Symbol Sym;
   Sym.ID = SymbolID(ID);
   Sym.Name = ID;
   return Sym;
 }
 
 std::unique_ptr<SymbolSlab> numSlab(int Begin, int End) {
   SymbolSlab::Builder Slab;
   for (int i = Begin; i <= End; i++)
     Slab.insert(symbol(std::to_string(i)));
   return llvm::make_unique<SymbolSlab>(std::move(Slab).build());
 }
 
-std::unique_ptr<RefSlab> refSlab(const SymbolID &ID, StringRef Path) {
+std::unique_ptr<RefSlab> refSlab(const SymbolID &ID, const char *Path) {
   RefSlab::Builder Slab;
   Ref R;
   R.Location.FileURI = Path;
   R.Kind = RefKind::Reference;
   Slab.insert(ID, R);
   return llvm::make_unique<RefSlab>(std::move(Slab).build());
 }
 

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

 
 MATCHER_P(Named, N, "") { return arg.Name == N; }
 MATCHER_P(RefRange, Range, "") {
   return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
                          arg.Location.End.line(), arg.Location.End.column()) ==
          std::make_tuple(Range.start.line, Range.start.character,
                          Range.end.line, Range.end.character);
 }
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+MATCHER_P(FileURI, F, "") { return StringRef(arg.Location.FileURI) == F; }
 
 TEST(SymbolLocation, Position) {
   using Position = SymbolLocation::Position;
   Position Pos;
 
   Pos.setLine(1);
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
TEST(MergeTest, Merge) {
   L.Signature = "()";                   // present in left only
   R.CompletionSnippetSuffix = "{$1:0}"; // present in right only
   R.Documentation = "--doc--";
   L.Origin = SymbolOrigin::Dynamic;
   R.Origin = SymbolOrigin::Static;
 
   Symbol M = mergeSymbol(L, R);
   EXPECT_EQ(M.Name, "Foo");
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h");
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:///left.h");
   EXPECT_EQ(M.References, 3u);
   EXPECT_EQ(M.Signature, "()");
   EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}");
   EXPECT_EQ(M.Documentation, "--doc--");
   EXPECT_EQ(M.Origin,
             SymbolOrigin::Dynamic | SymbolOrigin::Static | SymbolOrigin::Merge);
 }
 
 TEST(MergeTest, PreferSymbolWithDefn) {
   Symbol L, R;
 
   L.ID = R.ID = SymbolID("hello");
   L.CanonicalDeclaration.FileURI = "file:/left.h";
   R.CanonicalDeclaration.FileURI = "file:/right.h";
   L.Name = "left";
   R.Name = "right";
 
   Symbol M = mergeSymbol(L, R);
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h");
-  EXPECT_EQ(M.Definition.FileURI, "");
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/left.h");
+  EXPECT_EQ(StringRef(M.Definition.FileURI), "");
   EXPECT_EQ(M.Name, "left");
 
   R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored.
   M = mergeSymbol(L, R);
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h");
-  EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp");
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/right.h");
+  EXPECT_EQ(StringRef(M.Definition.FileURI), "file:/right.cpp");
   EXPECT_EQ(M.Name, "right");
 }
 
 TEST(MergeIndexTest, Refs) {
   FileIndex Dyn({"unittest"});
   FileIndex StaticIndex({"unittest"});
   MergedIndex Merge(&Dyn, &StaticIndex);
 

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

auto Sym1 = *ParsedYAML->Symbols->find(
       cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F35")));
   auto Sym2 = *ParsedYAML->Symbols->find(
       cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F36")));
 
   EXPECT_THAT(Sym1, QName("clang::Foo1"));
   EXPECT_EQ(Sym1.Signature, "");
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
-  EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
+  EXPECT_EQ(StringRef(Sym1.CanonicalDeclaration.FileURI), "file:///path/foo.h");
   EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
   EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
   EXPECT_THAT(Sym1.IncludeHeaders,
               UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u),
                                    IncludeHeaderWithRef("include2", 3u)));
 
   EXPECT_THAT(Sym2, QName("clang::Foo2"));
   EXPECT_EQ(Sym2.Signature, "-sig");
   EXPECT_EQ(Sym2.ReturnType, "");
-  EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
+  EXPECT_EQ(llvm::StringRef(Sym2.CanonicalDeclaration.FileURI),
+            "file:///path/bar.h");
   EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
 
   ASSERT_TRUE(bool(ParsedYAML->Refs));
   EXPECT_THAT(
       *ParsedYAML->Refs,
       UnorderedElementsAre(
           Pair(cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F35")),
                testing::SizeIs(1))));
   auto Ref1 = ParsedYAML->Refs->begin()->second.front();
   EXPECT_EQ(Ref1.Kind, RefKind::Reference);
-  EXPECT_EQ(Ref1.Location.FileURI, "file:///path/foo.cc");
+  EXPECT_EQ(StringRef(Ref1.Location.FileURI), "file:///path/foo.cc");
 }
 
 std::vector<std::string> YAMLFromSymbols(const SymbolSlab &Slab) {
   std::vector<std::string> Result;
   for (const auto &Sym : Slab)
     Result.push_back(toYAML(Sym));
   return Result;
 }

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

MATCHER_P(Labeled, Label, "") {
   return (arg.Name + arg.Signature).str() == Label;
 }
 MATCHER_P(ReturnType, D, "") { return arg.ReturnType == D; }
 MATCHER_P(Doc, D, "") { return arg.Documentation == D; }
 MATCHER_P(Snippet, S, "") {
   return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
-MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
-MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
+MATCHER_P(DeclURI, P, "") {
+  return StringRef(arg.CanonicalDeclaration.FileURI) == P;
+}
+MATCHER_P(DefURI, P, "") { return StringRef(arg.Definition.FileURI) == P; }
 MATCHER_P(IncludeHeader, P, "") {
   return (arg.IncludeHeaders.size() == 1) &&
          (arg.IncludeHeaders.begin()->IncludeHeader == P);
 }
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References,  "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 MATCHER_P(DeclRange, Pos, "") {