[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

88–89

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

88–89

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

89

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

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

96

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

303

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

clangd/index/Merge.cpp

119

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

96

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

303

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

119

this is neat!

[sammccall]

clangd/index/Index.h

303

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

303

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

[alexfh]

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

71 ↗

(On Diff #174013)

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 ↗

(On Diff #174013)

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 ↗

(On Diff #174013)

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 ↗

(On Diff #174013)

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, ...}

clangd/CodeComplete.cpp

if (C.IndexResult) {
       }
       Completion.Deprecated |= (C.IndexResult->Flags & Symbol::Deprecated);
     }
 
     // Turn absolute path into a literal string that can be #included.
     auto Inserted =
         [&](StringRef Header) -> Expected<std::pair<std::string, bool>> {
       auto ResolvedDeclaring =
-          toHeaderFile(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
+          toHeaderFile(C.IndexResult->CanonicalDeclaration.fileURI(), FileName);
       if (!ResolvedDeclaring)
         return ResolvedDeclaring.takeError();
       auto ResolvedInserted = toHeaderFile(Header, FileName);
       if (!ResolvedInserted)
         return ResolvedInserted.takeError();
       return std::make_pair(
           Includes.calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted),
           Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
     };
     bool ShouldInsert = C.headerToInsertIfAllowed().hasValue();
     // Calculate include paths and edits for all possible headers.
     for (const auto &Inc : C.RankedIncludeHeaders) {
       if (auto ToInclude = Inserted(Inc)) {
         CodeCompletion::IncludeCandidate Include;
         Include.Header = ToInclude->first;
         if (ToInclude->second && ShouldInsert)
           Include.Insertion = Includes.insert(ToInclude->first);
         Completion.Includes.push_back(std::move(Include));
       } else
         log("Failed to generate include insertion edits for adding header "
             "(FileURI='{0}', IncludeHeader='{1}') into {2}",
-            C.IndexResult->CanonicalDeclaration.FileURI, Inc, FileName);
+            C.IndexResult->CanonicalDeclaration.fileURI(), Inc, FileName);
     }
     // Prefer includes that do not need edits (i.e. already exist).
     std::stable_partition(Completion.Includes.begin(),
                           Completion.Includes.end(),
                           [](const CodeCompletion::IncludeCandidate &I) {
                             return !I.Insertion.hasValue();
                           });
   }

clangd/FindSymbols.cpp

getWorkspaceSymbols(StringRef Query, int Limit, const SymbolIndex *const Index,
   if (Limit)
     Req.Limit = Limit;
   TopN<ScoredSymbolInfo, ScoredSymbolGreater> Top(
       Req.Limit ? *Req.Limit : std::numeric_limits<size_t>::max());
   FuzzyMatcher Filter(Req.Query);
   Index->fuzzyFind(Req, [HintPath, &Top, &Filter](const Symbol &Sym) {
     // Prefer the definition over e.g. a function declaration in a header
     auto &CD = Sym.Definition ? Sym.Definition : Sym.CanonicalDeclaration;
-    auto Uri = URI::parse(CD.FileURI);
+    auto Uri = URI::parse(CD.fileURI());
     if (!Uri) {
       log("Workspace symbol: Could not parse URI '{0}' for symbol '{1}'.",
-          CD.FileURI, Sym.Name);
+          CD.fileURI(), Sym.Name);
       return;
     }
     auto Path = URI::resolve(*Uri, HintPath);
     if (!Path) {
       log("Workspace symbol: Could not resolve path for URI '{0}' for symbol "
           "'{1}'.",
           Uri->toString(), Sym.Name);
       return;

clangd/Quality.cpp

if (D->getLinkageInternal() < ExternalLinkage)
     return SymbolRelevanceSignals::FileScope;
   return SymbolRelevanceSignals::GlobalScope;
 }
 
 void SymbolRelevanceSignals::merge(const Symbol &IndexResult) {
   // FIXME: Index results always assumed to be at global scope. If Scope becomes
   // relevant to non-completion requests, we should recognize class members etc.
 
-  SymbolURI = IndexResult.CanonicalDeclaration.FileURI;
+  SymbolURI = IndexResult.CanonicalDeclaration.fileURI();
   SymbolScope = IndexResult.Scope;
   IsInstanceMember |= isInstanceMember(IndexResult.SymInfo);
 }
 
 void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
   if (SemaCCResult.Availability == CXAvailability_NotAvailable ||
       SemaCCResult.Availability == CXAvailability_NotAccessible)
     Forbidden = true;

clangd/XRefs.cpp

 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
 // FindSymbols.
 llvm::Optional<Location> toLSPLocation(const SymbolLocation &Loc,
                                        llvm::StringRef HintPath) {
   if (!Loc)
     return llvm::None;
-  auto Uri = URI::parse(Loc.FileURI);
+  auto Uri = URI::parse(Loc.fileURI());
   if (!Uri) {
-    log("Could not parse URI: {0}", Loc.FileURI);
+    log("Could not parse URI: {0}", Loc.fileURI());
     return llvm::None;
   }
   auto Path = URI::resolve(*Uri, HintPath);
   if (!Path) {
-    log("Could not resolve URI: {0}", Loc.FileURI);
+    log("Could not resolve URI: {0}", Loc.fileURI());
     return llvm::None;
   }
   Location LSPLoc;
   LSPLoc.uri = URIForFile(*Path);
   LSPLoc.range.start.line = Loc.Start.line();
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc.End.line();
   LSPLoc.range.end.character = Loc.End.column();

clangd/index/Index.h

struct Position {
 
     // Clients should use getters and setters to access these members.
     // FIXME: hide these members.
     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;
+  llvm::StringRef fileURI() const { return FileURI; }
+  void setFileURI(const char *F) { FileURI = F; }
 
   /// 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 !fileURI().empty(); }
+
+  // The URI of the source file where a symbol occurs.
+  // The string must be null-terminated.
+  //
+  // FIXME: make it to private, the main blocker: YAMLSerailization needs to
+  // access this member, and we don't want to introduce YAML dependency into
+  // this file (declaring a friend class).
+  const char *FileURI = nullptr;
 };
 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);
+  return std::make_tuple(L.fileURI(), L.Start, L.End) ==
+         std::make_tuple(R.fileURI(), 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);
+  return std::make_tuple(L.fileURI(), L.Start, L.End) <
+         std::make_tuple(R.fileURI(), 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 160-bits SHA1(USR) values to
 // guarantee the uniqueness of symbols while using a relatively small amount of
 // memory (vs storing USRs directly).
 //
 // SymbolID can be used as key in the symbol indexes to lookup the symbol.

 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);
   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.
 float quality(const Symbol &S);
 
 // An immutable symbol container that stores a set of symbols.

clangd/index/Index.cpp

if (Col > MaxColumn) {
     return;
   }
   Column = Col;
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) {
   if (!L)
     return OS << "(none)";
-  return OS << L.FileURI << "[" << L.Start.line() << ":" << L.Start.column()
+  return OS << L.fileURI() << "[" << L.Start.line() << ":" << L.Start.column()
             << "-" << L.End.line() << ":" << L.End.column() << ")";
 }
 
 SymbolID::SymbolID(StringRef USR)
     : HashValue(SHA1::hash(arrayRefFromStringRef(USR))) {}
 
 raw_ostream &operator<<(raw_ostream &OS, const SymbolID &ID) {
   return OS << toHex(ID.raw());
SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &ID) const {
   if (It != Symbols.end() && It->ID == ID)
     return It;
   return Symbols.end();
 }
 
 // Copy the underlying data of the symbol into the owned arena.
 static void own(Symbol &S, llvm::UniqueStringSaver &Strings) {
   visitStrings(S, [&](StringRef &V) { V = Strings.save(V); });
+  S.CanonicalDeclaration.setFileURI(
+      Strings.save(S.CanonicalDeclaration.fileURI()).data());
+  S.Definition.setFileURI(Strings.save(S.Definition.fileURI()).data());
 }
 
 void SymbolSlab::Builder::insert(const Symbol &S) {
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
   if (R.second) {
     Symbols.push_back(S);
     own(Symbols.back(), UniqueStrings);
   } else {

 
 llvm::raw_ostream &operator<<(llvm::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.setFileURI(
+      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;

clangd/index/Merge.cpp

void MergedIndex::refs(const RefsRequest &Req,
   // We consider the dynamic index authoritative and report all its refs,
   // and only report static index refs from other files.
   //
   // FIXME: The heuristic fails if the dynamic index contains a file, but all
   // refs were removed (we will report stale ones from the static index).
   // Ultimately we should explicit check which index has the file instead.
   llvm::StringSet<> DynamicIndexFileURIs;
   Dynamic->refs(Req, [&](const Ref &O) {
-    DynamicIndexFileURIs.insert(O.Location.FileURI);
+    DynamicIndexFileURIs.insert(O.Location.fileURI());
     Callback(O);
   });
   Static->refs(Req, [&](const Ref &O) {
-    if (!DynamicIndexFileURIs.count(O.Location.FileURI))
+    if (!DynamicIndexFileURIs.count(O.Location.fileURI()))
       Callback(O);
   });
 }
 
 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.
   bool MergeIncludes =
-      L.Definition.FileURI.empty() == R.Definition.FileURI.empty();
+      L.Definition.fileURI().empty() == R.Definition.fileURI().empty();
   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)

clangd/index/Serialization.cpp

 // SYMBOL ENCODING
 // Each field of clangd::Symbol is encoded in turn (see implementation).
 //  - StringRef fields encode as varint (index into the string table)
 //  - enums encode as the underlying type
 //  - most numbers encode as varint
 
 void writeLocation(const SymbolLocation &Loc, const StringTableOut &Strings,
                    raw_ostream &OS) {
-  writeVar(Strings.index(Loc.FileURI), OS);
+  writeVar(Strings.index(Loc.fileURI()), OS);
   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.setFileURI(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) {
   {
     raw_svector_ostream MetaOS(Meta);
     write32(Version, MetaOS);
   }
   RIFF.Chunks.push_back({riff::fourCC("meta"), Meta});
 
   StringTableOut Strings;
   std::vector<Symbol> Symbols;
+  auto InternFileURI = [&](SymbolLocation &Loc) {
+      llvm::StringRef File = Loc.fileURI();
+      Strings.intern(File);
+      Loc.setFileURI(File.data());
+  };
   for (const auto &Sym : *Data.Symbols) {
     Symbols.emplace_back(Sym);
     visitStrings(Symbols.back(), [&](StringRef &S) { Strings.intern(S); });
+    InternFileURI(Symbols.back().CanonicalDeclaration);
+    InternFileURI(Symbols.back().Definition);
   }
   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);
+        InternFileURI(Ref.Location);
     }
   }
 
   std::string StringSection;
   {
     raw_string_ostream StringOS(StringSection);
     Strings.finalize(StringOS);
   }

clangd/index/SymbolCollector.cpp

getTokenLocation(SourceLocation TokLoc, const SourceManager &SM,
                  const SymbolCollector::Options &Opts,
                  const clang::LangOptions &LangOpts,
                  std::string &FileURIStorage) {
   auto U = toURI(SM, SM.getFilename(TokLoc), Opts);
   if (!U)
     return llvm::None;
   FileURIStorage = std::move(*U);
   SymbolLocation Result;
-  Result.FileURI = FileURIStorage;
+  Result.setFileURI(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) {
         for (const auto &LocAndRole : It.second) {
           auto FileID = SM.getFileID(LocAndRole.first);
           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.setFileURI(FileURI->c_str());
             R.Kind = toRefKind(LocAndRole.second);
             Refs.insert(*ID, R);
           }
         }
       }
     }
   }
 

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>
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Symbol::IncludeHeaderWithReferences)
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Ref)
 
 namespace {
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;
-  llvm::yaml::Input Yin(Data);
+  llvm::BumpPtrAllocator Arena;
+  llvm::UniqueStringSaver Strings(Arena);
+  llvm::yaml::Input Yin(Data, &Strings);
   do {
     VariantEntry Variant;
     Yin >> Variant;
     if (Yin.error())
       return llvm::errorCodeToError(Yin.error());
     if (Variant.Symbol)
       Symbols.insert(*Variant.Symbol);
     if (Variant.Refs)

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 (!Sym.CanonicalDeclaration.fileURI().empty())
     for (const auto &ProximityURI :
-         generateProximityURIs(Sym.CanonicalDeclaration.FileURI))
+         generateProximityURIs(Sym.CanonicalDeclaration.fileURI()))
       Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
   if (Sym.Flags & Symbol::IndexedForCodeCompletion)
     Result.emplace_back(RestrictedForCodeCompletion);
   return Result;
 }
 
 // Constructs BOOST iterators for Path Proximities.
 std::unique_ptr<Iterator> createFileProximityIterator(

clangd/index/dex/dexp/Dexp.cpp

if (ID.getNumOccurrences()) {
       IDs.push_back(*SID);
     } else {
       IDs = getSymbolIDsFromIndex(Name, Index);
     }
     clang::clangd::RefsRequest RefRequest;
     RefRequest.IDs.insert(IDs.begin(), IDs.end());
     llvm::Regex RegexFilter(Filter);
     Index->refs(RefRequest, [&RegexFilter](const clang::clangd::Ref &R) {
-      auto U = clang::clangd::URI::parse(R.Location.FileURI);
+      auto U = clang::clangd::URI::parse(R.Location.fileURI());
       if (!U) {
         llvm::outs() << U.takeError();
         return;
       }
       if (RegexFilter.match(U->body()))
         llvm::outs() << R << "\n";
     });
   }

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.setFileURI(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.setFileURI(BarURI.c_str());
+  SymY.CanonicalDeclaration.setFileURI(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.setFileURI(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.setFileURI(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.setFileURI(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()))));
 }

unittests/clangd/DexTests.cpp

TEST(DexTest, SymbolIndexOptionsFilter) {
   Req.RestrictForCodeCompletion = false;
   EXPECT_THAT(match(I, Req), ElementsAre("Completion", "NoCompletion"));
   Req.RestrictForCodeCompletion = true;
   EXPECT_THAT(match(I, Req), ElementsAre("Completion"));
 }
 
 TEST(DexTest, ProximityPathsBoosting) {
   auto RootSymbol = symbol("root::abc");
-  RootSymbol.CanonicalDeclaration.FileURI = "unittest:///file.h";
+  RootSymbol.CanonicalDeclaration.setFileURI("unittest:///file.h");
   auto CloseSymbol = symbol("close::abc");
-  CloseSymbol.CanonicalDeclaration.FileURI = "unittest:///a/b/c/d/e/f/file.h";
+  CloseSymbol.CanonicalDeclaration.setFileURI("unittest:///a/b/c/d/e/f/file.h");
 
   std::vector<Symbol> Symbols{CloseSymbol, RootSymbol};
   Dex I(Symbols, RefSlab(), URISchemes);
 
   FuzzyFindRequest Req;
   Req.Query = "abc";
   // The best candidate can change depending on the proximity paths.
   Req.Limit = 1;
 
   // FuzzyFind request comes from the file which is far from the root: expect
   // CloseSymbol to come out.
   Req.ProximityPaths = {testPath("a/b/c/d/e/f/file.h")};
   EXPECT_THAT(match(I, Req), ElementsAre("close::abc"));
 
   // 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;
+    SymbolRefs.back().Location.setFileURI(Filename);
   };
   auto Foo = symbol("foo");
   auto Bar = symbol("bar");
   AddRef(Foo, "foo.h", RefKind::Declaration);
   AddRef(Foo, "reffoo.h", RefKind::Reference);
   AddRef(Bar, "bar.h", RefKind::Declaration);
 
   std::vector<std::string> Files;
   RefsRequest Req;
   Req.IDs.insert(Foo.ID);
   Req.Filter = RefKind::Declaration | RefKind::Definition;
   Dex(std::vector<Symbol>{Foo, Bar}, Refs, {}).refs(Req, [&](const Ref &R) {
-    Files.push_back(R.Location.FileURI);
+    Files.push_back(R.Location.fileURI());
   });
 
   EXPECT_THAT(Files, ElementsAre("foo.h"));
 }
 
 } // namespace
 } // namespace dex
 } // namespace clangd
 } // namespace clang

unittests/clangd/FileIndexTests.cpp

 using testing::UnorderedElementsAre;
 
 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(FileURI, F, "") { return arg.Location.fileURI() == F; }
+MATCHER_P(DeclURI, U, "") { return arg.CanonicalDeclaration.fileURI() == U; }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
 
 namespace clang {
 namespace clangd {
 namespace {
 testing::Matcher<const RefSlab &>
 RefsAre(std::vector<testing::Matcher<Ref>> Matchers) {
   return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers)));
 }
 
 Symbol symbol(llvm::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, llvm::StringRef Path) {
+std::unique_ptr<RefSlab> refSlab(const SymbolID &ID, const char* Path) {
   RefSlab::Builder Slab;
   Ref R;
-  R.Location.FileURI = Path;
+  R.Location.setFileURI(Path);
   R.Kind = RefKind::Reference;
   Slab.insert(ID, R);
   return llvm::make_unique<RefSlab>(std::move(Slab).build());
 }
 
 RefSlab getRefs(const SymbolIndex &I, SymbolID ID) {
   RefsRequest Req;
   Req.IDs = {ID};

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 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(MergeIndexTest, FuzzyFind) {
   EXPECT_THAT(match(MergedIndex(I.get(), J.get()), Req),
               UnorderedElementsAre("ns::A", "ns::B", "ns::C"));
 }
 
 TEST(MergeTest, Merge) {
   Symbol L, R;
   L.ID = R.ID = SymbolID("hello");
   L.Name = R.Name = "Foo";                           // same in both
-  L.CanonicalDeclaration.FileURI = "file:///left.h"; // differs
-  R.CanonicalDeclaration.FileURI = "file:///right.h";
+  L.CanonicalDeclaration.setFileURI("file:///left.h"); // differs
+  R.CanonicalDeclaration.setFileURI("file:///right.h");
   L.References = 1;
   R.References = 2;
   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(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.CanonicalDeclaration.setFileURI("file:/left.h");
+  R.CanonicalDeclaration.setFileURI("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(M.CanonicalDeclaration.fileURI(), "file:/left.h");
+  EXPECT_EQ(M.Definition.fileURI(), "");
   EXPECT_EQ(M.Name, "left");
 
-  R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored.
+  R.Definition.setFileURI("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(M.CanonicalDeclaration.fileURI(), "file:/right.h");
+  EXPECT_EQ(M.Definition.fileURI(), "file:/right.cpp");
   EXPECT_EQ(M.Name, "right");
 }
 
 TEST(MergeIndexTest, Refs) {
   FileIndex Dyn({"unittest"});
   FileIndex StaticIndex({"unittest"});
   MergedIndex Merge(&Dyn, &StaticIndex);
 
TEST(MergeTest, MergeIncludesOnDifferentDefinitions) {
 
   // Both have no definition.
   Symbol M = mergeSymbol(L, R);
   EXPECT_THAT(M.IncludeHeaders,
               UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
                                    IncludeHeaderWithRef("new", 1u)));
 
   // Only merge references of the same includes but do not merge new #includes.
-  L.Definition.FileURI = "file:/left.h";
+  L.Definition.setFileURI("file:/left.h");
   M = mergeSymbol(L, R);
   EXPECT_THAT(M.IncludeHeaders,
               UnorderedElementsAre(IncludeHeaderWithRef("common", 2u)));
 
   // Definitions are the same.
-  R.Definition.FileURI = "file:/right.h";
+  R.Definition.setFileURI("file:/right.h");
   M = mergeSymbol(L, R);
   EXPECT_THAT(M.IncludeHeaders,
               UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
                                    IncludeHeaderWithRef("new", 1u)));
 
   // Definitions are different.
-  R.Definition.FileURI = "file:/right.h";
+  R.Definition.setFileURI("file:/right.h");
   M = mergeSymbol(L, R);
   EXPECT_THAT(M.IncludeHeaders,
               UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
                                    IncludeHeaderWithRef("new", 1u)));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

unittests/clangd/SerializationTests.cpp

auto Sym1 = *ParsedYAML->Symbols->find(
       cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F352D1DF856")));
   auto Sym2 = *ParsedYAML->Symbols->find(
       cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F352D1DF858")));
 
   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(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(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(
                            "057557CEBF6E6B2DD437FBF60CC58F352D1DF856")),
                        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(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;
 }

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 arg.CanonicalDeclaration.fileURI() == P; }
+MATCHER_P(DefURI, P, "") { return 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, "") {