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

sammccall added inline comments. Comment at: clang-tools-extra/trunk/clangd/index/Index.h:71 + // via llvm::StringRef. + const char *FileURI = ""; }; alexfh wrote: > hokein wrote: > > alexfh wrote: > > > If the users of the SymbolLocation have a way to get a c

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

alexfh added inline comments. Comment at: clang-tools-extra/trunk/clangd/index/Index.h:71 + // via llvm::StringRef. + const char *FileURI = ""; }; hokein wrote: > alexfh wrote: > > If the users of the SymbolLocation have a way to get a common "context", > > y

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

hokein added inline comments. Comment at: clang-tools-extra/trunk/clangd/index/Index.h:71 + // via llvm::StringRef. + const char *FileURI = ""; }; alexfh wrote: > If the users of the SymbolLocation have a way to get a common "context", you > could keep a list

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

alexfh added inline comments. Comment at: clang-tools-extra/trunk/clangd/index/Index.h:71 + // via llvm::StringRef. + const char *FileURI = ""; }; If the users of the SymbolLocation have a way to get a common "context", you could keep a list of filenames in t

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL346852: [clangd] Replace StringRef in SymbolLocation with a char pointer. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D534

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

hokein updated this revision to Diff 173861. hokein marked an inline comment as done. hokein added a comment. Add assert. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53427 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/Merge.cpp clangd/index/Serializ

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

hokein added inline comments. Comment at: clangd/index/Index.h:306 +llvm::StringRef S(P); +CB(S); +P = S.data(); sammccall wrote: > hokein wrote: > > sammccall wrote: > > > ```assert (!S.data()[S.size()] && "Visited StringRef must be > > > null-termi

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

sammccall added inline comments. Comment at: clangd/index/Index.h:306 +llvm::StringRef S(P); +CB(S); +P = S.data(); hokein wrote: > sammccall wrote: > > ```assert (!S.data()[S.size()] && "Visited StringRef must be > > null-terminated") > Does this as

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

hokein added inline comments. Comment at: clangd/index/Index.h:93 +return Cmp < 0; + return std::tie(L.Start, L.End) < std::tie(R.Start, R.End); } sammccall wrote: > tie(strcmp(L.FileURI, R.FileURI), ...) < tie(0, ...)? tie doesn't support this usage, it ex

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

hokein updated this revision to Diff 173856. hokein marked 3 inline comments as done. hokein added a comment. Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53427 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/Merge.cpp clangd/i

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

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

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

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(

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

hokein updated this revision to Diff 173842. hokein marked an inline comment as done. hokein added a comment. update the patch. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53427 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/Merge.cpp clangd/index/Se

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

MaskRay 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

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

hokein updated this revision to Diff 172949. hokein added a comment. Move forward the patch based on the offline discussion. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53427 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/Merge.cpp clangd/index/Seria

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

sammccall added a comment. 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 r

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

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. 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