[PATCH] D78454: [clangd] Highlight related control flow.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:628
+    return FD->getBody();
+  if (const auto *FD = N.get<FunctionDecl>())
+    return FD->getBody();
----------------
You are checking for FunctionDecl twice


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:654
+  // Types of control-flow statements we might highlight.
+  enum Target {
+    Break = 1,
----------------
What about "goto"? It's more difficult to figure out if it breaks out of the loop or just jumps inside it, but it might still be worth highlighting it, even unconditionally (inside a loop only, not function).


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:761
+  auto CaseBefore = std::prev(CaseAfter);
+  // ... rewind CaseBefore to the first in a `case A: case B: ...` sequence.
+  while (CaseBefore != Cases.begin() &&
----------------
That's a nice feature, but there's no test for it. Can you add it?


================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:119
 
+TEST(HighlightsTest, ControlFlow) {
+  const char *Tests[] = {
----------------
Can you add a test for macros? I think highlighting like ASSIGN_OR_RETURN(int x, foo()); would be great, but even if we don't support that, just having a test that we don't crash would be valuable, IMHO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78454