This is an archive of the discontinued LLVM Phabricator instance.

[clangd] More precise representation of symbol names/labels in the index.
ClosedPublic

Authored by sammccall on Jun 22 2018, 2:32 AM.

Details

Summary

Previously, the strings matched LSP completion pretty closely.
The completion label was a single string, for instance. This made
implementing completion itself easy but makes it hard to use the names
in other way, e.g. pretty-printed name in synthesized
documentation/hover.

It also limits our introspection into completion items, which can only
be as precise as the indexed symbols. This change is a prerequisite to
improvements to overload bundling which need to inspect e.g. signature
structure.

Event Timeline

sammccall created this revision.Jun 22 2018, 2:32 AM
ioeric accepted this revision.Jun 22 2018, 7:32 AM

lgtm

clangd/CodeCompletionStrings.cpp
122

Qualifier seems a bit ambiguous for this. The documentation for CK_TypedText says:

/// The piece of text that the user is expected to type to
/// match the code-completion string, typically a keyword or the name of a
/// declarator or macro.
CK_TypedText,

For example, it could also be the keyword in "typedef ^". Maybe just TypedText would be more straightforward?

unittests/clangd/CodeCompletionStringsTests.cpp
70

nit: I find getSignature a bit awkward here because we are not getting anything back. Maybe setSignature?

This revision is now accepted and ready to land.Jun 22 2018, 7:32 AM
sammccall marked an inline comment as done.Jun 22 2018, 9:08 AM
sammccall added inline comments.
clangd/CodeCompletionStrings.cpp
122

Changed to "RequiredQualifiers" and specified it more clearly in the method comment.

Also added an implementation comment here as it's a little subtle. We're not outputting the current chunk to Qualifiers, but rather moving all the text we recorded so far.

unittests/clangd/CodeCompletionStringsTests.cpp
70

Agreed. Changed to computeSignature.

(I'm not sure the current test structure with the inputs and outputs being fixture members is great, but I don't really want to shave that yak now)

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