cameron314 added inline comments.

================
Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365
                            cast<CXXMethodDecl>(FD)->getParent(), ObjectType,
-                           ObjectClassification, Args.slice(1), CandidateSet,
+                           ObjectClassification, FunctionArgs, CandidateSet,
                            SuppressUserConversions, PartialOverloading);
----------------
yvvan wrote:
> cameron314 wrote:
> > This breaks normal (non-static) method overload resolution, since the 
> > `this` argument is now passed to `AddMethodCandidate` which is not 
> > expecting it. It always thinks too many arguments are passed to methods 
> > with no parameters; most other calls to `AddMethodCandidate` in 
> > SemaOverload.cpp don't pass the implicit `this`.
> This code is correct. It is sliced at line 6361 - only in the case when the 
> args size is greater than 0.
Hmm, you're right, I didn't see that. That line was missing after a rebase on 
our side, my fault for not properly cross-referencing the diffs. Sorry for the 
false alarm.

But shouldn't the slice only be done now when `Args.size() > 0 && (!Args[0] || 
(FirstArgumentIsBase && isa<CXXMethodDecl>(FD) && 
!isa<CXXConstructorDecl>(FD)))`?


================
Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6396
       } else {
         AddTemplateOverloadCandidate(FunTmpl, F.getPair(),
                                      ExplicitTemplateArgs, Args,
----------------
yvvan wrote:
> cameron314 wrote:
> > The same slice that was added above needs to be done here for templated 
> > static methods that are accessed via a non-static object.
> It might be the case. You can add that :)
> I did not check template cases in details.
It is the case, I have parameter completion tests that fail without this :-)
I'll commit in a few days.


Repository:
  rL LLVM

https://reviews.llvm.org/D36390



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to