This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Match AST and Index label for template Symbols
ClosedPublic

Authored by ilya-biryukov on Apr 10 2018, 3:50 AM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.Apr 10 2018, 3:50 AM
sammccall accepted this revision.Apr 11 2018, 5:26 AM

Nice, I'd been wondering about that...

clangd/index/SymbolCollector.cpp
36 ↗(On Diff #141821)

uber-nit: these three return statements are a bit confusing to me. Maybe omit them and if/elseif, so the default case falls through to the bottom.

331 ↗(On Diff #141821)

"an actual run" confused me here. Maybe "We use the primary template, as clang does during code completion"?

unittests/clangd/FileIndexTests.cpp
218 ↗(On Diff #141821)

If snippets are off, we'll get "vector", not "vector<>", right?

(Probably no need to test this explicitly, but I just want to be sure)

This revision is now accepted and ready to land.Apr 11 2018, 5:26 AM
ilya-biryukov marked 2 inline comments as done.
  • Also test plain text completion text
  • Clarify the comment
  • Simplify conditions in getTemplateOrThis
ilya-biryukov added inline comments.Apr 13 2018, 4:06 AM
unittests/clangd/FileIndexTests.cpp
218 ↗(On Diff #141821)

Yes, that's exactly the case.
Testing doesn't hurt too, added a test.

This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.