[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 17 01:53:33 PDT 2019

hokein added inline comments.

Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:255
+  // ArrayRefs to the current line in the highlights.
+  ArrayRef<HighlightingToken> NewLine(Highlightings.data(),
+                                      Highlightings.data()),
jvikstrom wrote:
> hokein wrote:
> > IIUC, we are initializing an empty ArrayRef, if so, how about using `NewLine(Highlightings.data(), /*length*/0)`? `NewLine(Highlightings.data(), Highlightings.data())` is a bit weird, it took me a while to understand the purpose.
> I couldn't do that because the `ArrayRef(const T *data, size_t length)` and `ArrayRef(const T *begin, const T *end)` were ambiguous. Added a cast to cast 0 to a size_t which solved it.
you could also do it like `NewLine(Highlightings.data(), /*length*/0UL);`, which saves you a `static_cast<>`.

Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:281
+      )cpp"}},
+                {{5},
+                 {
so the empty lines are stored separately, which is not easy to figure out from the testing code snippet. I think we should make the empty lines more obvious in the code snippet (maybe use an `Empty` annotations?) , and explicitly verify the empty lines.

What do you think refining the test like below, just annotate the diff tokens? I find it is easy to spot the diff tokens.

struct Testcase {
   Code Before;
   Code After;

std::vector<TestCase> cases = {
             int a;
             int b; 
             int c;
            int a;
            $Empty[[]] // int b
            int $Variable[[C]];

oldHighlightings = getSemanticHighlightings(OldAST);
newHighlightings = getSemanticHighlightings(NewAST);
diffHighlightings = diff...;
// verify the diffHighlightings has the expected empty lines ("Empty" annotations).
// verify the diffHighlightings has the expected highlightings (the regular annotations);

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list