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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clangd/index/Index.h:84
 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(llvm::StringRef(L.FileURI), L.Start, L.End) ==
+         std::make_tuple(llvm::StringRef(R.FileURI), R.Start, R.End);
----------------
MaskRay wrote:
> 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.)
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.  


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53427