[cfe-commits] r167596 - in /cfe/trunk: lib/Sema/SemaOverload.cpp test/SemaCXX/ambiguous-conversion-show-overload.cpp

[Dmitri Gribenko]

Hi Matt,

On Thu, Nov 8, 2012 at 10:50 PM, Matt Beaumont-Gay <matthewbg at google.com> wrote:
> Author: matthewbg
> Date: Thu Nov  8 14:50:02 2012
> New Revision: 167596
>
> URL: http://llvm.org/viewvc/llvm-project?rev=167596&view=rev
> Log:
> Fix a bug I found while preparing my devmtg talk: When passing NULL to a
> function that takes a const Foo&, where Foo is convertible from a large number
> of pointer types, we print ALL the overloads, no matter the setting of
> -fshow-overloads.
>
> There is potential follow-on work in unifying the "print candidates, but not
> too many" logic between OverloadCandidateSet::NoteCandidates and
> ImplicitConversionSequence::DiagnoseAmbiguousConversion.
>
> Added:
>     cfe/trunk/test/SemaCXX/ambiguous-conversion-show-overload.cpp
> Modified:
>     cfe/trunk/lib/Sema/SemaOverload.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=167596&r1=167595&r2=167596&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Nov  8 14:50:02 2012
> @@ -7992,10 +7992,20 @@
>                                   const PartialDiagnostic &PDiag) const {
>    S.Diag(CaretLoc, PDiag)
>      << Ambiguous.getFromType() << Ambiguous.getToType();
> -  for (AmbiguousConversionSequence::const_iterator
> -         I = Ambiguous.begin(), E = Ambiguous.end(); I != E; ++I) {
> +  // FIXME: The note limiting machinery is borrowed from
> +  // OverloadCandidateSet::NoteCandidates; there's an opportunity for
> +  // refactoring here.
> +  const OverloadsShown ShowOverloads = S.Diags.getShowOverloads();
> +  unsigned CandsShown = 0;
> +  AmbiguousConversionSequence::const_iterator I, E;
> +  for (I = Ambiguous.begin(), E = Ambiguous.end(); I != E; ++I) {
> +    if (CandsShown >= 4 && ShowOverloads == Ovl_Best)
> +      break;
> +    ++CandsShown;
>      S.NoteOverloadCandidate(*I);
>    }
> +  if (I != E)
> +    S.Diag(SourceLocation(), diag::note_ovl_too_many_candidates) << int(E - I);
>  }

SourceLocation() is not the best place to attach this since we have CaretLoc.

>  namespace {
>
> Added: cfe/trunk/test/SemaCXX/ambiguous-conversion-show-overload.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ambiguous-conversion-show-overload.cpp?rev=167596&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/ambiguous-conversion-show-overload.cpp (added)
> +++ cfe/trunk/test/SemaCXX/ambiguous-conversion-show-overload.cpp Thu Nov  8 14:50:02 2012
> @@ -0,0 +1,21 @@
> +// RUN: %clang_cc1 -fsyntax-only -fshow-overloads=best -fno-caret-diagnostics %s 2>&1 | FileCheck %s
> +struct S {
> +  S(void*);
> +  S(char*);
> +  S(unsigned char*);
> +  S(signed char*);
> +  S(unsigned short*);
> +  S(signed short*);
> +  S(unsigned int*);
> +  S(signed int*);
> +};
> +void f(const S& s);
> +void g() {
> +  f(0);
> +}
> +// CHECK: {{conversion from 'int' to 'const S' is ambiguous}}
> +// CHECK-NEXT: {{candidate constructor}}
> +// CHECK-NEXT: {{candidate constructor}}
> +// CHECK-NEXT: {{candidate constructor}}
> +// CHECK-NEXT: {{candidate constructor}}
> +// CHECK-NEXT: {{remaining 4 candidates omitted; pass -fshow-overloads=all to show them}}

Why not -verify?

Please review the attached patch that fixes these.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: r167596-fix.patch
Type: application/octet-stream
Size: 1864 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121111/7b06c7d7/attachment.obj>