This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Use the index-based API to do the header-source switch.
ClosedPublic

Authored by hokein on Sep 30 2019, 5:41 AM.

Event Timeline

hokein created this revision.Sep 30 2019, 5:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2019, 5:41 AM

thanks, mostly LG

clang-tools-extra/clangd/ClangdServer.cpp
453

could you add some comments explaining, why we first use file-only version(speed) and why try with an ast&index afterwards(making use of declarations)

clang-tools-extra/clangd/ClangdServer.h
195

i don't think it is a helper function anymore

clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
250

could we rather provide testPath("src/include") to make sure this also works on windows?

hokein updated this revision to Diff 222432.Sep 30 2019, 7:53 AM
hokein marked 4 inline comments as done.

address comments.

kadircet accepted this revision.Sep 30 2019, 7:58 AM

some nits, thanks!

clang-tools-extra/clangd/ClangdServer.cpp
456

AST, it works -> AST, but it only works

456

indent one more column

458

s/works for/supports

This revision is now accepted and ready to land.Sep 30 2019, 7:58 AM
hokein updated this revision to Diff 222574.Oct 1 2019, 3:12 AM
hokein marked 3 inline comments as done.

address comments.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 1 2019, 3:19 AM