================
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;
----------------
francisco.lopes wrote:
> 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.
Correction: which all successful overloads with one parameter or more will be
providing.
http://reviews.llvm.org/D6880
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
