ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM from my side (and a few NIT, but up to you whether to apply them)
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:46
+ NonConflicting.reserve(Tokens.size());
+ ArrayRef<HighlightingToken> TokRef(Tokens);
+ while (!TokRef.empty()) {
----------------
NIT: Using a for-loop here could provide a little more structure and limit the
scope of `TokRef`
```
for (ArrayRef<Token> Toks = Tokens; !Toks.empty(); ) {
// ...
Toks = Toks.drop_front(Conflicting.size());
}
```
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:60
+ // of the Tokens).
+ TokRef = ArrayRef<HighlightingToken>(Conflicting.end(), TokRef.end());
+ }
----------------
NIT: the following version seems more readable: `TokRef =
TokRef.drop_front(Conflicting.size())`;
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:332
+ #define DEF_CLASS(T) class T {};
+ DEF_MULTIPLE(XYZ);
+ DEF_MULTIPLE(XYZW);
----------------
jvikstrom wrote:
> ilya-biryukov wrote:
> > Unrelated to the change: do we plan to highlight macro names?
> > That seems very useful.
> Yes
Awesome, can't wait for it to land!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64741/new/
https://reviews.llvm.org/D64741
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
