[RFC][PATCH] Implementation for completion in call context for C++

Hi,

I’ve improved completion in call context to work with C++'s templates,
member functions, et al.

Notice: an important aspect of this implementation is that I’ve took freedom
to change the previous behavior for completion in call context at all.

As can be seen, in lib/Sema/SemaCodeComplete.cpp this comment:

// When we’re code-completing for a call, we fall back to ordinary
// name code-completion whenever we can’t produce specific
// results. We may want to revisit this strategy in the future,
// e.g., by merging the two kinds of results.

was changed to this:

// When we’re code-completing for a call, we DON’T fall back to ordinary
// name code-completion whenever we can’t produce specific
// results. This will do “parameter completion” solely.
// I think “parameter completion” is not a good classification, since this
// is useful solely for providing hints and not code-completing.

The rationale behind this change is:

  • The former comment shows that completion in call context is still not
    in a definite state, open to discussion.
  • I firmly believe this new behavior is for good. In call context, 99% of
    the time, code authoring tools will be interested in information about the
    prototype and arguments solely, not in all possible kinds of expressions
    that can fit as an argument. This behavior turns the interface slower than
    necessary to have almost no benefit.
    So I believe that completion in call context just after ‘(’ and ‘,’ should
    work similarly as for member access: only provide what’s of interest, which
    parametric information (function prototypes, current argument, parameter
    types, etc.).

Despite this, I should be able revert to the former behavior in the change set
without much trouble, in case there’s no agreement on this, while still
providing the new set of completions.

The following is a video that covers the new behavior, so that’s easy to see
what got changed. It’s based on the test set that’s part of the patch:

https://vimeo.com/115990512

The change set can be browsed here:

https://github.com/oblitum/clang/tree/improved_argument_hints

The Subversion compatible patch:

https://gist.github.com/oblitum/d24d7d2f9c6ee5cdcea1

This got started for YouCompleteMe:

https://github.com/Valloric/YouCompleteMe/pull/1300

TODO:
Functors (classes that overload operator()), Constructors, …

(I’ve already obtained commit access)

Att,
Francisco Lopes

PS: I’m actively looking for a job, remote or relocation, any help very much
appreciated!

PING.

Notice that meanwhile I’ve just completed parts of what was previously TODO (constructors, functors, …).

http://reviews.llvm.org/D6880

Regards,

Francisco Lopes

Hey Francisco.

I can't review the code, just wanted to comment on the functionality as I
observe it from the video above. I think that the information about the call
signatures during code completion is extremely useful. But it should not be
put into the list of code completion results. Rather, I think it is additional
information that should be distinguished from the results. These results
should be a list of everything accessible at the call site, sorted by their
relevance and excluding everything that can be ruled out.

In KDevelop e.g. this looks like this:

http://imgur.com/odCbYrU
http://imgur.com/xxN2isv

At the top, you see the signature(s) viable for the call. Below you get the
list of code completion offers, and the "best matches" are obtained from
sorting these results such that the types match the viable call signatures.

The screenshots above are obtained from running our legacy C++ code model. We
are currently working on a clang-based code model and that already gives us
similar results, including the sorting. What we are missing so far is the
information to display on top for the possible viable call signatures. We can
probably workaround that somehow, but having a Clang(-c) API for that would be
nice.

If I'm not mistaking your video, your patch removes the list of results and
replaces it only by a list of matching types? This would be an unfortunate
regression, imo.

Bye

Thanks for the input Milian, you did made me think it further, since I’m not used to

authoring tools that provide that kind of simultaneous results. I think I’ll maintain the
current official behavior then, even though I believe this may turn the interface slower
for parameter completions that don’t look for the extra information about possible fitting

suggestions. These additional cases require extra AST trips and returns too
much information, so, performance wise, maybe a future split in the API about this may
be worth looking at.

Notice that for disambiguation between the general cases and the call signature
suggestions one can check whether a given completion provides a “current argument”
token, so it’s a call signature that fits, or check whether its cursor kind is CXCursor_NotImplemented,

which it still is. I think I’ll change that so that these particular completion cases I’m

providing support for can be clearly distinguished from the rest.

If I’m not mistaking your video, your patch removes the list of results and
replaces it only by a list of matching types? This would be an unfortunate
regression, imo.

What’s is being provided as result in truth, is at the top-left of the video when I set completopt += preview,
which means the entire signatures that fits. From that signature I’m picking, in the Vim plugin, the current
argument to provide suggestions in the popup.

For more detail you can check the tests that are provided with the patches.

Despite this, I’ll work to also provide the general fitting cases as explained before.

Regards,

Francisco Lopes

Hi Milian, I’ve done what I’ve planned. Now to distinguish the set of completions for overload candidates from completions for ordinary name completion I’ve added a new CXCursorKind named CXCursor_OverloadCandidate.

+Val for how we can make this fit into YCM / general feedback

Thanks all for the help.

  • thanks Milian for the short and eye opening comments
  • and Manuel in particular for the extensive review.

There are already some FIXMEs to be done, but they’re minor, the main part has landed in SVN revision 226670.

Regards,

Francisco Lopes