george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added a subscriber: cfe-commits.
Two smallish patches in one. Happy to split into two (for review and/or commit)
if that's preferred.
In C, we allow (as an extension) incompatible pointer conversions, like so:
```
int foo(int *);
int (*p)(void *) = foo; // Compiles; warns if -Wincompatible-pointer-types is
specified
```
So, we should provide primitive support for this with overloadable functions.
Specifically, this patch is trying to fix cases like this:
```
int foo(void *) __attribute__((overloadable));
int foo(double *d) __attribute__((overloadable, enable_if(d != nullptr, "")));
int (*p)(int *) = foo; // Before patch, error because we couldn't match the
function type exactly. After patch, we see that 'int foo(void *)' is the only
option, and choose it. A warning is emitted if -Wincompatible-pointer-types is
given
```
Which means that explicit casts are necessary if there's more than one overload
that isn't disabled by enable_if attrs. The updated docs have an example of
this, but here's another:
```
int foo(void *) __attribute__((overloadable));
int foo(double *d) __attribute__((overloadable));
int (*p)(int *) = foo; // 0 candidates before patch, 2 after. Won't compile
either way.
int (*p2)(int *) = (int (*)(void *))foo; // OK. Issues warning if
-Wincompatible-pointer-types is given
```
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
@@ -9997,6 +9997,9 @@
if (S.getLangOpts().CUDA && S.getLangOpts().CUDATargetOverloads &&
Matches.size() > 1)
EliminateSuboptimalCudaMatches();
+
+ if (!S.getLangOpts().CPlusPlus && Matches.size() > 1)
+ MaybeEliminateSuboptimalCSpecificMatches();
}
bool hasComplained() const { return HasComplained; }
@@ -10054,15 +10057,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) {
@@ -10093,12 +10101,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;
@@ -10195,6 +10202,21 @@
S.EraseUnwantedCUDAMatches(dyn_cast<FunctionDecl>(S.CurContext), Matches);
}
+ void MaybeEliminateSuboptimalCSpecificMatches() {
+ assert(!S.getLangOpts().CPlusPlus);
+ auto IsImperfectMatch = [this](
+ const std::pair<DeclAccessPair, FunctionDecl *> &Pair) {
+ return !CandidateHasExactlyCorrectType(Pair.second);
+ };
+
+ // If there are no perfect matches, we should complain about too many
+ // imperfect options, rather than offering the user no options at all.
+ if (std::all_of(Matches.begin(), Matches.end(), IsImperfectMatch))
+ return;
+ auto End = std::remove_if(Matches.begin(), Matches.end(), IsImperfectMatch);
+ Matches.erase(End, Matches.end());
+ }
+
public:
void ComplainNoMatchesFound() const {
assert(Matches.empty());
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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
