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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
