Page MenuHomePhabricator

[clangd] Highlight related control flow.
ClosedPublic

Authored by sammccall on Apr 19 2020, 5:20 AM.

Details

Summary

This means e.g. highlighting "return" will show other returns/throws
from the same function, highlighting a case will show all the
return/breaks etc.

This is a bit of an abuse of textDocument/highlight, but seems useful.

Diff Detail

Event Timeline

sammccall created this revision.Apr 19 2020, 5:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2020, 5:20 AM
adamcz added inline comments.Apr 20 2020, 3:37 AM
clang-tools-extra/clangd/XRefs.cpp
725

You are checking for FunctionDecl twice

751

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).

858

That's a nice feature, but there's no test for it. Can you add it?

clang-tools-extra/clangd/unittests/XRefsTests.cpp
119

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.

sammccall updated this revision to Diff 264734.May 18 2020, 3:19 PM
sammccall marked 6 inline comments as done.

Address comments, handle goto.

clang-tools-extra/clangd/XRefs.cpp
751

I think I worked out the right logic for where it jumps based on Bounds (which we now always compute).

Selecting the goto statement itself isn't handled here, it should be handled as a "regular" reference to the labeldecl. (But isn't yet).

clang-tools-extra/clangd/unittests/XRefsTests.cpp
119

Changed the sourcelocation handling to enable macros.
Now if the relevant keyword is expanded from a macro body, the macro name will he highlighted. If it's expanded from a macro arg, the arg will still be highlighted.
Added a test for both.

adamcz accepted this revision.May 19 2020, 4:37 AM
adamcz added inline comments.
clang-tools-extra/clangd/XRefs.cpp
923

nit: in a do {...} while(...) case, would we want to highlight both keywords?

(doesn't seem very important, feel free to ignore)

997

nit: weird line wrap here?

This revision is now accepted and ready to land.May 19 2020, 4:37 AM
sammccall marked 3 inline comments as done.May 19 2020, 8:35 AM
sammccall added inline comments.
clang-tools-extra/clangd/XRefs.cpp
923

Yeah, I just left a comment for this. You're right... but do-while is pretty rare and the code is fairly awkward. Let's see if anyone ever mentions it :-)

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.

Hi Sam,

It looks like this is causing a failure on the Windows PS4 buildbot: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32606

Please could you take a look? PS4 target disables RTTI, hence exceptions, by default so it is probably related to that.

Thanks
Russ