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.

Diff Detail

Repository
rL LLVM

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 ↗(On Diff #222396)

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 ↗(On Diff #222396)

i don't think it is a helper function anymore

clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
250 ↗(On Diff #222396)

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 ↗(On Diff #222432)

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

456 ↗(On Diff #222432)

indent one more column

458 ↗(On Diff #222432)

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