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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clangd/index/Index.h:85
+  assert(L.FileURI && R.FileURI);
+  int Cmp = std::strcmp(L.FileURI, R.FileURI);
+  return Cmp == 0 && std::tie(L.Start, L.End) == std::tie(R.Start, R.End);
----------------
nit: I think it's more idiomatic just to inline as
```return !strcmp(L.FileURI, R.FileURI) && ...```


================
Comment at: clangd/index/Index.h:93
+    return Cmp < 0;
+  return std::tie(L.Start, L.End) < std::tie(R.Start, R.End);
 }
----------------
tie(strcmp(L.FileURI, R.FileURI), ...) < tie(0, ...)?


================
Comment at: clangd/index/Index.h:306
+    llvm::StringRef S(P);
+    CB(S);
+    P = S.data();
----------------
```assert (!S.data()[S.size()] && "Visited StringRef must be null-terminated")


================
Comment at: clangd/index/Merge.cpp:118
   bool MergeIncludes =
-      L.Definition.FileURI.empty() == R.Definition.FileURI.empty();
+      !std::strlen(L.Definition.FileURI) == !std::strlen(R.Definition.FileURI);
   Symbol S = PreferR ? R : L;        // The target symbol we're merging into.
----------------
Since this reads awkwardly anyway...
```bool(*L.Definition.FileURI) == bool(*R.definition.FileURI)```
is shorter and faster


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53427