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

Francisco Lopes da Silva francisco.mailing.lists at oblita.com
Wed Jan 14 10:08:04 PST 2015

Comment at: lib/Sema/SemaOverload.cpp:5966-5967
@@ -5960,3 +5965,4 @@
   // list (8.3.5).
-  if (Args.size() > NumParams && !Proto->isVariadic()) {
+  if ((Args.size() + (PartialOverloading && Args.size())) > NumParams &&
+      !Proto->isVariadic()) {
     Candidate.Viable = false;
klimek wrote:
> Ok, sorry for the many questions, but I'm new to this part of the code myself :)
> This also changes the overload resolution, right? Is this an integral part of the patch, or could this also be split out? If it cannot be split out, can you explain why we now need this and which test cases apply to that?
- Yes, this kind of changes that involve the `PartialOverloading` flag should change the behavior for overloading resolution **solely when it's `true`**.

- Yes, it's an integral part of the patch. As can be seen the original codebase already have this kind of check where it's already supporting "partial" overloading. It can't be splitted out.

As can be checked in SemaOverload.cpp:

/// AddOverloadCandidate - Adds the given function to the set of
/// candidate functions, using the given function call arguments.  If
/// @p SuppressUserConversions, then don't allow user-defined
/// conversions via constructors or conversion operators.
/// \param PartialOverloading true if we are performing "partial" overloading
/// based on an incomplete set of function arguments. This feature is used by
/// code completion.

PartialOverloading is used in code completion.

The most basic situation to make sense of this code is when code completion is requested in calling context, just after a comma for an argument. Let's say the prototype of the function being called has just one parameter. So in this test `NumParams` would be `1`. As code completion was triggered just after the comma to insert a second argument that was still not inserted, `Args.size()` is still `1`. So `Args.size() > NumParams` is `false` but the test must still succeed to set `Candidate.FailureKind = ovl_fail_too_few_arguments` since the extra comma means a second argument follows.

Notice that even though the code is being correct at this moment for failing with `ovl_fail_too_few_arguments`, this relates with another part of the code where I do add all candidates that failed with this reason to the result set of viable candidates.

The reason I'm adding the candidates that fail because of "too many arguments" to the result set is because the user will be able to obtain the overloads of that function so it can be used for displaying the expected prototypes, and also it will be able to verify when the overloads are not matching because they won't be providing a "current argument", which all successful overloads with more that one argument will be providing.

Anyway, this last part is still open for discussion although I think a change regarding this coming afterwards won't harm either.



More information about the cfe-commits mailing list