================
Comment at: lib/Sema/SemaOverload.cpp:5860-5861
@@ -5857,3 +5859,4 @@
if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
- if (isa<CXXMethodDecl>(FD) && !cast<CXXMethodDecl>(FD)->isStatic())
+ if (isa<CXXMethodDecl>(FD) && !cast<CXXMethodDecl>(FD)->isStatic() &&
+ !isa<CXXConstructorDecl>(FD))
AddMethodCandidate(cast<CXXMethodDecl>(FD), F.getPair(),
----------------
francisco.lopes wrote:
> klimek wrote:
> > francisco.lopes wrote:
> > > klimek wrote:
> > > > Here and elsewhere: this looks like it actually changes overload
> > > > resolution, but I didn't see a description about that change in the
> > > > change comment - what am I missing?
> > > This change is not required in truth (originally I was using it in
> > > `CodeCompleteConstructor` too), I've left it for a good reason.
> > > `AddFunctionCandidates` looks like a function that was added much later
> > > than the ones it calls, and it's being called in few places in the
> > > codebase, none involving constructors, hence no issue has been raised
> > > before. If you check inside `AddMethodCandidate` there's an assertion
> > > telling it shouldn't be called for constructors, and that this is a job
> > > for `AddOverloadCandidate`. The way `AddFunctionCandidates` was before
> > > would do exactly that: if `Fns` contained a constructor,
> > > `AddMethodCandidate` would be called for them, with also the added risk
> > > of accessing `Args[0]` when `Args` is empty.
> > If it's not required, could you perhaps split up the CL into 2 changes:
> > 1. the code completion change
> > 2. the overload resolution change
> > That would make it much easier for me to review the change and make sure
> > each part has sufficient tests.
> What you have in mind for how do this the best way? I'm not sure how to
> proceed, I'm thinking of doing a rebase that would take this overload
> resolution change from the middle of the commit history and push it to the
> tip as a commit of its own.
Since clang upstream is SVN, all the git history will be lost anyway, so you
have multiple options how to do this:
a) rebase
b) create two branches, revert the parts that shouldn't go into that branch
manually with a tool like meld, send out two patches
a) is nice if the rebase works without conflicts, otherwise I've done b) before
without too many problems.
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
