[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 18 04:57:01 PDT 2019
hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:46
+ // removed.
+ for (unsigned I = 0; I < Tokens.size(); ++I) {
+ ArrayRef<HighlightingToken> TokRef(Tokens);
----------------
we don't care the Kind in `HighlightingToken` now, I think we could simplify the code by tweaking the deduplication logic above?
```
llvm::sort(Tokens, [](const HighlightingToken &L, const HighlightingToken &R) {
return L.R < R.R;
});
std::unique(Tokens.begin(), Tokens.end(), [](const HighlightingToken &L, const HighlightingToken &R) {
return L.R == R.R;
});
```
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:195
+ #define DEF_CLASS(T) class T {};
+ DEF_MULTIPLE(XYZ);
+ DEF_MULTIPLE(XYZW);
----------------
jvikstrom wrote:
> ilya-biryukov wrote:
> > Could you add a comment explaining that we choose to not highlight the conflicting tokens?
> There is a comment in SemanticHighlighting.cpp at line 43. Want me to add a comment in the test case as well?
The comment in SemanticHighlighting.cpp file is too far away, there is no harm to add comment here, indeed, it somehow improves the test code readability .
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64741/new/
https://reviews.llvm.org/D64741
More information about the cfe-commits
mailing list