r207796 - When sorting overload candidates, sort arity mismatches in ascending

Kaelyn Takata rikka at google.com
Thu May 1 16:20:02 PDT 2014


On Thu, May 1, 2014 at 4:01 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, May 1, 2014 at 3:54 PM, Kaelyn Takata <rikka at google.com> wrote:
> >
> >
> >
> > On Thu, May 1, 2014 at 3:26 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>
> >> On Thu, May 1, 2014 at 2:15 PM, Kaelyn Takata <rikka at google.com> wrote:
> >> > Author: rikka
> >> > Date: Thu May  1 16:15:24 2014
> >> > New Revision: 207796
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=207796&view=rev
> >> > Log:
> >> > When sorting overload candidates, sort arity mismatches in ascending
> >> > order by the number of missing or extra parameters. This is useful if
> >> > there are more than a few overload candidates with arity mismatches,
> >> > particularly in the presence of -fshow-overloads=best.
> >> >
> >> > Modified:
> >> >     cfe/trunk/lib/Sema/SemaOverload.cpp
> >> >     cfe/trunk/test/Misc/error-limit-multiple-notes.cpp
> >> >
> >> > Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=207796&r1=207795&r2=207796&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> >> > +++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu May  1 16:15:24 2014
> >> > @@ -9229,7 +9229,10 @@ static unsigned RankDeductionFailure(con
> >> >
> >> >  struct CompareOverloadCandidatesForDisplay {
> >> >    Sema &S;
> >> > -  CompareOverloadCandidatesForDisplay(Sema &S) : S(S) {}
> >> > +  size_t NumArgs;
> >> > +
> >> > +  CompareOverloadCandidatesForDisplay(Sema &S, size_t nArgs)
> >> > +      : S(S), NumArgs(nArgs) {}
> >> >
> >> >    bool operator()(const OverloadCandidate *L,
> >> >                    const OverloadCandidate *R) {
> >> > @@ -9254,8 +9257,18 @@ struct CompareOverloadCandidatesForDispl
> >> >      if (!L->Viable) {
> >> >        // 1. Arity mismatches come after other candidates.
> >> >        if (L->FailureKind == ovl_fail_too_many_arguments ||
> >> > -          L->FailureKind == ovl_fail_too_few_arguments)
> >> > +          L->FailureKind == ovl_fail_too_few_arguments) {
> >> > +        if (R->FailureKind == ovl_fail_too_many_arguments ||
> >> > +            R->FailureKind == ovl_fail_too_few_arguments) {
> >> > +          int LDist = abs(L->Function->getNumParams() - NumArgs);
> >> > +          int RDist = abs(R->Function->getNumParams() - NumArgs);
> >> > +          if (LDist == RDist)
> >> > +            return L->FailureKind == ovl_fail_too_many_arguments &&
> >> > +                   R->FailureKind == ovl_fail_too_few_arguments;
> >> > +          return LDist < RDist;
> >> > +        }
> >> >          return false;
> >> > +      }
> >> >        if (R->FailureKind == ovl_fail_too_many_arguments ||
> >> >            R->FailureKind == ovl_fail_too_few_arguments)
> >> >          return true;
> >> > @@ -9442,7 +9455,7 @@ void OverloadCandidateSet::NoteCandidate
> >> >    }
> >> >
> >> >    std::sort(Cands.begin(), Cands.end(),
> >> > -            CompareOverloadCandidatesForDisplay(S));
> >> > +            CompareOverloadCandidatesForDisplay(S, Args.size()));
> >> >
> >> >    bool ReportedAmbiguousConversions = false;
> >> >
> >> >
> >> > Modified: cfe/trunk/test/Misc/error-limit-multiple-notes.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/error-limit-multiple-notes.cpp?rev=207796&r1=207795&r2=207796&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- cfe/trunk/test/Misc/error-limit-multiple-notes.cpp (original)
> >> > +++ cfe/trunk/test/Misc/error-limit-multiple-notes.cpp Thu May  1
> >> > 16:15:24 2014
> >> > @@ -4,20 +4,22 @@
> >> >  void foo(int);
> >> >  void foo(double);
> >> >  void foo(int, int);
> >> > +void foo(int, int, int, int);
> >> >
> >> >  int main()
> >> >  {
> >> > -    foo();
> >> > +    foo(1, 2, 3);
> >> >  }
> >> >
> >> >  // error and note suppressed by error-limit
> >> >  struct s1{};
> >> >  struct s1{};
> >> >
> >> > -// CHECK: 10:5: error: no matching function for call to 'foo'
> >> > -// CHECK: 6:6: note: candidate function not viable: requires 2
> >> > arguments, but 0 were provided
> >> > -// CHECK: 5:6: note: candidate function not viable: requires 1
> >> > argument, but 0 were provided
> >> > -// CHECK: 4:6: note: candidate function not viable: requires 1
> >> > argument, but 0 were provided
> >> > +// CHECK: 11:5: error: no matching function for call to 'foo'
> >> > +// CHECK: 6:6: note: candidate function not viable: requires 2
> >> > arguments, but 3 were provided
> >> > +// CHECK: 7:6: note: candidate function not viable: requires 4
> >> > arguments, but 3 were provided
> >> > +// CHECK: 5:6: note: candidate function not viable: requires 1
> >> > argument, but 3 were provided
> >> > +// CHECK: 4:6: note: candidate function not viable: requires 1
> >> > argument, but 3 were provided
> >>
> >> Would it be appropriate/worthwhile adding some cases where there are
> >> the same number of arguments but they're of different types? Or in
> >> that case do we not show the other candidates at all?
> >
> >
> > What exactly do you mean by "same number of arguments but different
> types"?
> > If you meant that a candidate accepts the right number of arguments but
> the
> > types are wrong,
>
> Yep, that's what I meant.
>
> >  they are already sorted before any arity mismatches. Also,
> > there are already two one-argument candidates that have different
> argument
> > types.
>
> Hmm - should they be in the check line to demonstrate/test that they
> do come before arity mismatches? Or is that covered by other tests
> somewhere?
>

Presumably they are covered by other tests, though it looks like most of
this comparator was written about 3 or 4 years ago...

This test had been the only one that broke when I changed the sort order,
which is why it is the one I modified to test the arity sorting a bit more
thoroughly.


>
>
>
> >>
> >> >  // CHECK: fatal error: too many errors emitted, stopping now
> >> > -// CHECK-NOT: 15:8: error: redefinition of 's1'
> >> > -// CHECK-NOT: 14:8: note: previous definition is here
> >> > +// CHECK-NOT: 16:8: error: redefinition of 's1'
> >> > +// CHECK-NOT: 15:8: note: previous definition is here
> >> >
> >> >
> >> > _______________________________________________
> >> > cfe-commits mailing list
> >> > cfe-commits at cs.uiuc.edu
> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140501/7335cb2c/attachment.html>


More information about the cfe-commits mailing list