[PATCH] D85635: [clangd] Compute the inactive code range for semantic highlighting.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

I think the code could be a bit clearer, but up to you. The tests are good so feel free to submit any version you're happy with.



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:242
+            });
+        NonConflicting.push_back(
+            {HighlightingKind::InactiveCode,
----------------
Hmm, the role of `NonConflicting` is pretty confusing here: it has an invariant preserved as we construct it, then we violate it by dumping a bunch of ranges into it, then try to restore it.
And the correctness of the restoring seems to rely on some pretty subtle facts (that the inactive region always fully contains and is sorted *before* the regions it overlaps).

What about:
```
// Merge token stream with "inactive line" markers.
vector WithInactiveLines;
auto It = NonConflicting.begin();
for (Range &R : Ranges) {
  for (int Line : R) {
    // Copy tokens before the inactive line
    for (; It != NonConflicting.end() && It->position.begin.line < Line)
      WithInactiveLines.push_back(std::move(*It));
    // Add a token for the inactive line itself.
    WithInactiveLines.push_back(createInactiveLineToken(MainCode, Line));
    // Skip any other tokens on the inactive line
    while (It != NonConflicting.end() && It->position.begin.line == Line)
      ++It;
  }
}
// Copy tokens after the last inactive line
for (; It != NonOverlapping.end(); ++It)
  WithInactiveLines.push_back(std::move(*It));
```

The only real assumption here is "a token is associated with a line", which is pretty fundamental anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85635/new/

https://reviews.llvm.org/D85635