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