[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