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 expects lvalue arguments. ================ Comment at: clangd/index/Index.h:306 + llvm::StringRef S(P); + CB(S); + P = S.data(); ---------------- sammccall wrote: > ```assert (!S.data()[S.size()] && "Visited StringRef must be null-terminated") 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`. ================ 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. ---------------- sammccall wrote: > Since this reads awkwardly anyway... > ```bool(*L.Definition.FileURI) == bool(*R.definition.FileURI)``` > is shorter and faster this is neat! 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
