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

[David Blaikie]

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?

>
>>
>> >  // 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
>
>