================
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(),
----------------
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.

================
Comment at: lib/Sema/SemaOverload.cpp:5968
@@ -5960,2 +5967,3 @@
   // list (8.3.5).
-  if (Args.size() > NumParams && !Proto->isVariadic()) {
+  if ((Args.size() + (PartialOverloading && Args.size())) > NumParams &&
+      !Proto->isVariadic()) {
----------------
klimek wrote:
> This is a little too clever for me :)
> a) comment on why we need to +1 when PartialOverloading
> b) pull it out of the if into a variable?
For me too! but I've just copied the behavior from some original place. 
Changing all references of such snippet would mean to enter refactoring of the 
original code. While applying these changes I paused for a moment to analyze 
what the **** this was doing. I think I got it for a moment and didn't care 
anymore and just replicated where it was necessary.

Do you think this is time for refactoring the changeset and the remaining 
references of this
code?

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

Reply via email to