jvikstrom marked an inline comment as done.
jvikstrom added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:36
+ // must be deduplicated.
+ std::sort(Tokens.begin(), Tokens.end());
+ for (unsigned I = 1; I < Tokens.size(); ++I) {
----------------
hokein wrote:
> nit: we could write it like
>
> ```
> llvm::sort(Tokens, [](const HighlightingToken &Lhs, const HighlightingToken
> &Rhs) {
> return std::tie(Lhs.Kind, Lhs.R) < std::tie(Rhs.Kind, Rhs.R);
> });
> auto Last = std::unique(Tokens.begin(), Tokens.end());
> Tokens.erase(Last, Tokens.end());
> ```
About the comparator function though. I added an < operator because
https://reviews.llvm.org/D64475 also sorts the tokens. However, could I just
remove the sort in the main diffHighlightings method and just rely on the
highlightings to be sorted in the first place from this patch?
Instead we'd do the sorting like:
```
llvm::sort(Tokens, [](const HighlightingToken &Lhs, const HighlightingToken
&Rhs) {
return std::tie(Lhs.R,Lhs.Kind) < std::tie(Rhs.R,Rhs.Kind);
});
```
so we get it sorted by their position first and Kind second.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64634/new/
https://reviews.llvm.org/D64634
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
