george.burgess.iv updated this revision to Diff 37965. george.burgess.iv added a comment.
Rebased http://reviews.llvm.org/D13704 Files: include/clang/Basic/AttrDocs.td lib/Sema/SemaOverload.cpp test/CodeGen/overloadable.c test/Sema/overloadable.c
Index: test/Sema/overloadable.c =================================================================== --- test/Sema/overloadable.c +++ test/Sema/overloadable.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify %s -Wincompatible-pointer-types int var __attribute__((overloadable)); // expected-error{{'overloadable' attribute only applies to functions}} void params(void) __attribute__((overloadable(12))); // expected-error {{'overloadable' attribute takes no arguments}} @@ -99,3 +99,24 @@ unsigned char *c; multi_type(c); } + +// Ensure that we allow C-specific type conversions in C +void fn_type_conversions() { + void foo(void *c) __attribute__((overloadable)); + void foo(char *c) __attribute__((overloadable)); + void (*ptr1)(void *) = &foo; + void (*ptr2)(char *) = &foo; + void (*ambiguous)(int *) = &foo; // expected-error{{initializing 'void (*)(int *)' with an expression of incompatible type '<overloaded function type>'}} expected-note@105{{candidate function}} expected-note@106{{candidate function}} + void *vp_ambiguous = &foo; // expected-error{{initializing 'void *' with an expression of incompatible type '<overloaded function type>'}} expected-note@105{{candidate function}} expected-note@106{{candidate function}} + + void (*specific1)(int *) = (void (*)(void *))&foo; // expected-warning{{incompatible pointer types initializing 'void (*)(int *)' with an expression of type 'void (*)(void *)'}} + void *specific2 = (void (*)(void *))&foo; + + void disabled(void *c) __attribute__((overloadable, enable_if(0, ""))); + void disabled(int *c) __attribute__((overloadable, enable_if(0, ""))); + void disabled(char *c) __attribute__((overloadable, enable_if(1, "The function name lies."))); + // To be clear, these should all point to the last overload of 'disabled' + void (*dptr1)(char *c) = &disabled; + void (*dptr2)(void *c) = &disabled; // expected-warning{{incompatible pointer types initializing 'void (*)(void *)' with an expression of type '<overloaded function type>'}} expected-note@115{{candidate function made ineligible by enable_if}} expected-note@116{{candidate function made ineligible by enable_if}} expected-note@117{{candidate function has type mismatch at 1st parameter (expected 'void *' but has 'char *')}} + void *specific_disabled = &disabled; +} Index: test/CodeGen/overloadable.c =================================================================== --- test/CodeGen/overloadable.c +++ test/CodeGen/overloadable.c @@ -29,3 +29,33 @@ cdv = f(cdv); vv = f(vv); } + +// Ensuring that we pick the correct function for taking the address of an +// overload when conversions are involved. + +void addrof_many(int *a) __attribute__((overloadable, enable_if(0, ""))); +void addrof_many(void *a) __attribute__((overloadable)); +void addrof_many(char *a) __attribute__((overloadable)); + +void addrof_single(int *a) __attribute__((overloadable, enable_if(0, ""))); +void addrof_single(char *a) __attribute__((overloadable, enable_if(0, ""))); +void addrof_single(char *a) __attribute__((overloadable)); + +// CHECK-LABEL: define void @foo +void foo() { + // CHECK: store void (i8*)* @_Z11addrof_manyPc + void (*p1)(char *) = &addrof_many; + // CHECK: store void (i8*)* @_Z11addrof_manyPv + void (*p2)(void *) = &addrof_many; + // CHECK: void (i8*)* @_Z11addrof_manyPc + void *vp1 = (void (*)(char *)) & addrof_many; + // CHECK: void (i8*)* @_Z11addrof_manyPv + void *vp2 = (void (*)(void *)) & addrof_many; + + // CHECK: store void (i8*)* @_Z13addrof_singlePc + void (*p3)(char *) = &addrof_single; + // CHECK: @_Z13addrof_singlePc + void (*p4)(int *) = &addrof_single; + // CHECK: @_Z13addrof_singlePc + void *vp3 = &addrof_single; +} Index: lib/Sema/SemaOverload.cpp =================================================================== --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -10010,7 +10010,11 @@ private: // Is A considered a better overload candidate for the desired type than B? bool isBetterCandidate(const FunctionDecl *A, const FunctionDecl *B) { - return hasBetterEnableIfAttrs(S, A, B); + // If there's more than one candidate and no exact match, then we want to + // require the user to disambiguate for us + if (!candidateHasExactlyCorrectType(A)) + return false; + return !candidateHasExactlyCorrectType(B) || hasBetterEnableIfAttrs(S, A, B); } // Returns true if we've eliminated any (read: all but one) candidates, false @@ -10090,15 +10094,20 @@ Specialization = cast<FunctionDecl>(Specialization->getCanonicalDecl()); assert(S.isSameOrCompatibleFunctionType( Context.getCanonicalType(Specialization->getType()), - Context.getCanonicalType(TargetFunctionType)) || - (!S.getLangOpts().CPlusPlus && TargetType->isVoidPointerType())); + Context.getCanonicalType(TargetFunctionType))); if (!isFunctionAlwaysEnabled(S.Context, Specialization)) return false; Matches.push_back(std::make_pair(CurAccessFunPair, Specialization)); return true; } + + bool candidateHasExactlyCorrectType(const FunctionDecl *FD) { + QualType Discard; + return Context.hasSameUnqualifiedType(TargetFunctionType, FD->getType()) || + S.IsNoReturnConversion(FD->getType(), TargetFunctionType, Discard); + } bool AddMatchingNonTemplateFunction(NamedDecl* Fn, const DeclAccessPair& CurAccessFunPair) { @@ -10129,12 +10138,11 @@ if (!isFunctionAlwaysEnabled(S.Context, FunDecl)) return false; - QualType ResultTy; - if (Context.hasSameUnqualifiedType(TargetFunctionType, - FunDecl->getType()) || - S.IsNoReturnConversion(FunDecl->getType(), TargetFunctionType, - ResultTy) || - (!S.getLangOpts().CPlusPlus && TargetType->isVoidPointerType())) { + // If we're in C, we support incompatible pointer conversions for + // non-overloaded functions, so we must support it for overloaded + // functions, as well. + if ((!S.getLangOpts().CPlusPlus && TargetType->isPointerType()) || + candidateHasExactlyCorrectType(FunDecl)) { Matches.push_back(std::make_pair( CurAccessFunPair, cast<FunctionDecl>(FunDecl->getCanonicalDecl()))); FoundNonTemplateFunction = true; Index: include/clang/Basic/AttrDocs.td =================================================================== --- include/clang/Basic/AttrDocs.td +++ include/clang/Basic/AttrDocs.td @@ -260,6 +260,18 @@ not ODR-equivalent. Query for this feature with ``__has_attribute(enable_if)``. + +Note that functions with one or more ``enable_if`` attributes ay *not* have +their address taken. This is to prevent cases like the following from compiling: + +.. code-block:: c++ + + int f(int a) __attribute__((enable_if(a > 0, ""))); + int (*fptr)(int) = &f; + int result = fptr(-1); + +It is legal to take the address of a function with ``enable_if`` attributes if +and only if all of the conditions in said attributes always evaluate to true. }]; } @@ -344,6 +356,22 @@ would in C. Query for this feature with ``__has_extension(attribute_overloadable)``. + +When taking the address of an overloaded function in C, the overload is +generally selected as it is in C++: clang will try to match the target type with +the overload. If this is not possible, (read: if you're trying to implicitly +cast it to an incompatible pointer type) then you may need to add a cast to the +overloaded function, like so: + +.. code-block:: c + + int foo(int) __attribute__((overloadable)); + int foo(int *) __attribute__((overloadable)); + + int (*ptr1)(void *) = foo; // Error: Ambiguous overload + int (*ptr2)(void *) = (int (*)(int *))foo; // OK -- warns if clang was given + // -Wincompatible-pointer-types + }]; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits