From fbad5b2f1b7dfd1c7e16252315e63c6b3ebb1cc3 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Wed, 7 Sep 2016 20:03:19 +0000 Subject: [PATCH] [Sema] Compare bad conversions in overload resolution. r280553 introduced an issue where we'd emit ambiguity errors for code like: ``` void foo(int *, int); void foo(unsigned int *, unsigned int); void callFoo() { unsigned int i; foo(&i, 0); // ambiguous: int->unsigned int is worse than int->int, // but unsigned int*->unsigned int* is better than // int*->int*. } ``` This patch fixes this issue by changing how we handle ill-formed (but valid) implicit conversions. Candidates with said conversions now always rank worse than candidates without them, and two candidates are considered to be equally bad if they both have these conversions for the same argument. Additionally, this fixes a case in C++11 where we'd complain about an ambiguity in a case like: ``` void f(char *, int); void f(const char *, unsigned); void g() { f("abc", 0); } ``` ...Since conversion to char* from a string literal is considered ill-formed in C++11 (and deprecated in C++03), but we accept it as an extension. llvm-svn: 280847 --- clang/include/clang/Basic/AttrDocs.td | 9 +++++---- clang/lib/Sema/SemaOverload.cpp | 33 ++++++++++++++++++++++++++++++--- clang/test/CodeGen/overloadable.c | 20 ++++++++++++++++++++ clang/test/SemaCXX/overload-call.cpp | 11 +++++++++++ 4 files changed, 66 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 3b43959..804a334 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -470,10 +470,11 @@ semantics: * A conversion from type ``T`` to a value of type ``U`` is permitted if ``T`` and ``U`` are compatible types. This conversion is given "conversion" rank. -* A conversion from a pointer of type ``T*`` to a pointer of type ``U*``, where - ``T`` and ``U`` are incompatible, is allowed, but is ranked below all other - types of conversions. Please note: ``U`` lacking qualifiers that are present - on ``T`` is sufficient for ``T`` and ``U`` to be incompatible. +* If no viable candidates are otherwise available, we allow a conversion from a + pointer of type ``T*`` to a pointer of type ``U*``, where ``T`` and ``U`` are + incompatible. This conversion is ranked below all other types of conversions. + Please note: ``U`` lacking qualifiers that are present on ``T`` is sufficient + for ``T`` and ``U`` to be incompatible. The declaration of ``overloadable`` functions is restricted to function declarations and definitions. Most importantly, if any function with a given diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 183fdf4..10aa997 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -8600,13 +8600,40 @@ bool clang::isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1, if (Cand1.IgnoreObjectArgument || Cand2.IgnoreObjectArgument) StartArg = 1; + auto IsIllFormedConversion = [&](const ImplicitConversionSequence &ICS) { + // We don't allow incompatible pointer conversions in C++. + if (!S.getLangOpts().CPlusPlus) + return ICS.isStandard() && + ICS.Standard.Second == ICK_Incompatible_Pointer_Conversion; + + // The only ill-formed conversion we allow in C++ is the string literal to + // char* conversion, which is only considered ill-formed after C++11. + return S.getLangOpts().CPlusPlus11 && !S.getLangOpts().WritableStrings && + hasDeprecatedStringLiteralToCharPtrConversion(ICS); + }; + + // Define functions that don't require ill-formed conversions for a given + // argument to be better candidates than functions that do. + unsigned NumArgs = Cand1.NumConversions; + assert(Cand2.NumConversions == NumArgs && "Overload candidate mismatch"); + bool HasBetterConversion = false; + for (unsigned ArgIdx = StartArg; ArgIdx < NumArgs; ++ArgIdx) { + bool Cand1Bad = IsIllFormedConversion(Cand1.Conversions[ArgIdx]); + bool Cand2Bad = IsIllFormedConversion(Cand2.Conversions[ArgIdx]); + if (Cand1Bad != Cand2Bad) { + if (Cand1Bad) + return false; + HasBetterConversion = true; + } + } + + if (HasBetterConversion) + return true; + // C++ [over.match.best]p1: // A viable function F1 is defined to be a better function than another // viable function F2 if for all arguments i, ICSi(F1) is not a worse // conversion sequence than ICSi(F2), and then... - unsigned NumArgs = Cand1.NumConversions; - assert(Cand2.NumConversions == NumArgs && "Overload candidate mismatch"); - bool HasBetterConversion = false; for (unsigned ArgIdx = StartArg; ArgIdx < NumArgs; ++ArgIdx) { switch (CompareImplicitConversionSequences(S, Loc, Cand1.Conversions[ArgIdx], diff --git a/clang/test/CodeGen/overloadable.c b/clang/test/CodeGen/overloadable.c index d9769c2..2ec6fe4 100644 --- a/clang/test/CodeGen/overloadable.c +++ b/clang/test/CodeGen/overloadable.c @@ -74,3 +74,23 @@ void bar() { // CHECK: call void @_Z7ovl_barPc ovl_bar(ucharbuf); } + +// CHECK-LABEL: define void @baz +void ovl_baz(int *, int) __attribute__((overloadable)); +void ovl_baz(unsigned int *, unsigned int) __attribute__((overloadable)); +void ovl_baz2(int, int *) __attribute__((overloadable)); +void ovl_baz2(unsigned int, unsigned int *) __attribute__((overloadable)); +void baz() { + unsigned int j; + // Initial rules for incompatible pointer conversions made this overload + // ambiguous. + // CHECK: call void @_Z7ovl_bazPjj + ovl_baz(&j, 0); + // CHECK: call void @_Z7ovl_bazPjj + ovl_baz(&j, 0u); + + // CHECK: call void @_Z8ovl_baz2jPj + ovl_baz2(0, &j); + // CHECK: call void @_Z8ovl_baz2jPj + ovl_baz2(0u, &j); +} diff --git a/clang/test/SemaCXX/overload-call.cpp b/clang/test/SemaCXX/overload-call.cpp index 7eaf98b..3a01bf2 100644 --- a/clang/test/SemaCXX/overload-call.cpp +++ b/clang/test/SemaCXX/overload-call.cpp @@ -647,3 +647,14 @@ namespace PR20218 { g(y); // expected-error {{ambiguous}} } } + +namespace StringLiteralToCharAmbiguity { + void f(char *, int); + void f(const char *, unsigned); + void g() { f("foo", 0); } +#if __cplusplus <= 199711L + // expected-error@-2 {{call to 'f' is ambiguous}} + // expected-note@-5 {{candidate function}} + // expected-note@-5 {{candidate function}} +#endif +} -- 2.7.4