[PATCH] D38425: [clangd] Document highlights for clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Overall looks good, just a few minor comments.
Let me know if need someone to land this patch for you after you fix those.



================
Comment at: clangd/ClangdUnit.cpp:249
+    // Don't keep the same Macro info multiple times.
+    // This can happen when nodes in the AST are visited twice.
+    std::sort(MacroInfos.begin(), MacroInfos.end());
----------------
Mentioning AST in this comment is weird, macros have nothing to do with the AST. We should remove/rephrase the comment.
I'm not sure if multiple occurences of `MacroInfo` are even possible here, but we could leave the code as is anyway.


================
Comment at: clangd/Protocol.h:562
+
+///
+/// A document highlight is a range inside a text document which deserves
----------------
NIT: remove this empty comment line and all the others.


================
Comment at: clangd/Protocol.h:581
+                        const DocumentHighlight &RHS) {
+    return std::tie(LHS.range) < std::tie(RHS.range) &&
+           std::tie(LHS.kind) < std::tie(RHS.kind);
----------------
This comparison does not provide a total order.
Please use `std::tie(LHS.range, int(LHS.kind)) < std::tie(RHS.range, int(RHS.kind))` instead.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38425