r250486 - [Sema] Fix address-of + enable_if overloading logic
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 15 18:17:38 PDT 2015
Author: gbiv
Date: Thu Oct 15 20:17:38 2015
New Revision: 250486
URL: http://llvm.org/viewvc/llvm-project?rev=250486&view=rev
Log:
[Sema] Fix address-of + enable_if overloading logic
Previously, our logic when taking the address of an overloaded function
would not consider enable_if attributes, so long as all of the enable_if
conditions on a given candidate were true. So, two functions with
identical signatures (one with enable_if attributes, the other without),
would be considered equally good overloads. If we were calling the
function instead of taking its address, then the function with enable_if
attributes would be preferred.
This patch makes us prefer the candidate with enable_if regardless of if
we're calling or taking the address of an overloaded function.
Differential Revision: http://reviews.llvm.org/D13795
Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CodeGen/enable_if.c
cfe/trunk/test/CodeGenCXX/enable_if.cpp
cfe/trunk/test/Sema/enable_if.c
cfe/trunk/test/SemaCXX/enable_if.cpp
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=250486&r1=250485&r2=250486&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Oct 15 20:17:38 2015
@@ -5855,23 +5855,31 @@ ObjCMethodDecl *Sema::SelectBestMethod(S
return nullptr;
}
-static bool IsNotEnableIfAttr(Attr *A) { return !isa<EnableIfAttr>(A); }
+// specific_attr_iterator iterates over enable_if attributes in reverse, and
+// enable_if is order-sensitive. As a result, we need to reverse things
+// sometimes. Size of 4 elements is arbitrary.
+static SmallVector<EnableIfAttr *, 4>
+getOrderedEnableIfAttrs(const FunctionDecl *Function) {
+ SmallVector<EnableIfAttr *, 4> Result;
+ if (!Function->hasAttrs())
+ return Result;
+
+ const auto &FuncAttrs = Function->getAttrs();
+ for (Attr *Attr : FuncAttrs)
+ if (auto *EnableIf = dyn_cast<EnableIfAttr>(Attr))
+ Result.push_back(EnableIf);
+
+ std::reverse(Result.begin(), Result.end());
+ return Result;
+}
EnableIfAttr *Sema::CheckEnableIf(FunctionDecl *Function, ArrayRef<Expr *> Args,
bool MissingImplicitThis) {
- // FIXME: specific_attr_iterator<EnableIfAttr> iterates in reverse order, but
- // we need to find the first failing one.
- if (!Function->hasAttrs())
- return nullptr;
- AttrVec Attrs = Function->getAttrs();
- AttrVec::iterator E = std::remove_if(Attrs.begin(), Attrs.end(),
- IsNotEnableIfAttr);
- if (Attrs.begin() == E)
+ auto EnableIfAttrs = getOrderedEnableIfAttrs(Function);
+ if (EnableIfAttrs.empty())
return nullptr;
- std::reverse(Attrs.begin(), E);
SFINAETrap Trap(*this);
-
SmallVector<Expr *, 16> ConvertedArgs;
bool InitializationFailed = false;
bool ContainsValueDependentExpr = false;
@@ -5908,7 +5916,7 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
}
if (InitializationFailed || Trap.hasErrorOccurred())
- return cast<EnableIfAttr>(Attrs[0]);
+ return EnableIfAttrs[0];
// Push default arguments if needed.
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
@@ -5929,12 +5937,11 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
}
if (InitializationFailed || Trap.hasErrorOccurred())
- return cast<EnableIfAttr>(Attrs[0]);
+ return EnableIfAttrs[0];
}
- for (AttrVec::iterator I = Attrs.begin(); I != E; ++I) {
+ for (auto *EIA : EnableIfAttrs) {
APValue Result;
- EnableIfAttr *EIA = cast<EnableIfAttr>(*I);
if (EIA->getCond()->isValueDependent()) {
// Don't even try now, we'll examine it after instantiation.
continue;
@@ -8397,6 +8404,44 @@ Sema::AddArgumentDependentLookupCandidat
}
}
+// Determines whether Cand1 is "better" in terms of its enable_if attrs than
+// Cand2 for overloading. This function assumes that all of the enable_if attrs
+// on Cand1 and Cand2 have conditions that evaluate to true.
+//
+// Cand1's set of enable_if attributes are said to be "better" than Cand2's iff
+// Cand1's first N enable_if attributes have precisely the same conditions as
+// Cand2's first N enable_if attributes (where N = the number of enable_if
+// attributes on Cand2), and Cand1 has more than N enable_if attributes.
+static bool hasBetterEnableIfAttrs(Sema &S, const FunctionDecl *Cand1,
+ const FunctionDecl *Cand2) {
+
+ // FIXME: The next several lines are just
+ // specific_attr_iterator<EnableIfAttr> but going in declaration order,
+ // instead of reverse order which is how they're stored in the AST.
+ auto Cand1Attrs = getOrderedEnableIfAttrs(Cand1);
+ auto Cand2Attrs = getOrderedEnableIfAttrs(Cand2);
+
+ // Candidate 1 is better if it has strictly more attributes and
+ // the common sequence is identical.
+ if (Cand1Attrs.size() <= Cand2Attrs.size())
+ return false;
+
+ auto Cand1I = Cand1Attrs.begin();
+ llvm::FoldingSetNodeID Cand1ID, Cand2ID;
+ for (auto &Cand2A : Cand2Attrs) {
+ Cand1ID.clear();
+ Cand2ID.clear();
+
+ auto &Cand1A = *Cand1I++;
+ Cand1A->getCond()->Profile(Cand1ID, S.getASTContext(), true);
+ Cand2A->getCond()->Profile(Cand2ID, S.getASTContext(), true);
+ if (Cand1ID != Cand2ID)
+ return false;
+ }
+
+ return true;
+}
+
/// isBetterOverloadCandidate - Determines whether the first overload
/// candidate is a better candidate than the second (C++ 13.3.3p1).
bool clang::isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1,
@@ -8507,47 +8552,8 @@ bool clang::isBetterOverloadCandidate(Se
// Check for enable_if value-based overload resolution.
if (Cand1.Function && Cand2.Function &&
(Cand1.Function->hasAttr<EnableIfAttr>() ||
- Cand2.Function->hasAttr<EnableIfAttr>())) {
- // FIXME: The next several lines are just
- // specific_attr_iterator<EnableIfAttr> but going in declaration order,
- // instead of reverse order which is how they're stored in the AST.
- AttrVec Cand1Attrs;
- if (Cand1.Function->hasAttrs()) {
- Cand1Attrs = Cand1.Function->getAttrs();
- Cand1Attrs.erase(std::remove_if(Cand1Attrs.begin(), Cand1Attrs.end(),
- IsNotEnableIfAttr),
- Cand1Attrs.end());
- std::reverse(Cand1Attrs.begin(), Cand1Attrs.end());
- }
-
- AttrVec Cand2Attrs;
- if (Cand2.Function->hasAttrs()) {
- Cand2Attrs = Cand2.Function->getAttrs();
- Cand2Attrs.erase(std::remove_if(Cand2Attrs.begin(), Cand2Attrs.end(),
- IsNotEnableIfAttr),
- Cand2Attrs.end());
- std::reverse(Cand2Attrs.begin(), Cand2Attrs.end());
- }
-
- // Candidate 1 is better if it has strictly more attributes and
- // the common sequence is identical.
- if (Cand1Attrs.size() <= Cand2Attrs.size())
- return false;
-
- auto Cand1I = Cand1Attrs.begin();
- for (auto &Cand2A : Cand2Attrs) {
- auto &Cand1A = *Cand1I++;
- llvm::FoldingSetNodeID Cand1ID, Cand2ID;
- cast<EnableIfAttr>(Cand1A)->getCond()->Profile(Cand1ID,
- S.getASTContext(), true);
- cast<EnableIfAttr>(Cand2A)->getCond()->Profile(Cand2ID,
- S.getASTContext(), true);
- if (Cand1ID != Cand2ID)
- return false;
- }
-
- return true;
- }
+ Cand2.Function->hasAttr<EnableIfAttr>()))
+ return hasBetterEnableIfAttrs(S, Cand1.Function, Cand2.Function);
if (S.getLangOpts().CUDA && S.getLangOpts().CUDATargetOverloads &&
Cand1.Function && Cand2.Function) {
@@ -9986,7 +9992,7 @@ public:
if (FindAllFunctionsThatMatchTargetTypeExactly()) {
// C++ [over.over]p4:
// If more than one function is selected, [...]
- if (Matches.size() > 1) {
+ if (Matches.size() > 1 && !eliminiateSuboptimalOverloadCandidates()) {
if (FoundNonTemplateFunction)
EliminateAllTemplateMatches();
else
@@ -10002,6 +10008,36 @@ public:
bool hasComplained() const { return HasComplained; }
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);
+ }
+
+ // Returns true if we've eliminated any (read: all but one) candidates, false
+ // otherwise.
+ bool eliminiateSuboptimalOverloadCandidates() {
+ // Same algorithm as overload resolution -- one pass to pick the "best",
+ // another pass to be sure that nothing is better than the best.
+ auto Best = Matches.begin();
+ for (auto I = Matches.begin()+1, E = Matches.end(); I != E; ++I)
+ if (isBetterCandidate(I->second, Best->second))
+ Best = I;
+
+ const FunctionDecl *BestFn = Best->second;
+ auto IsBestOrInferiorToBest = [this, BestFn](
+ const std::pair<DeclAccessPair, FunctionDecl *> &Pair) {
+ return BestFn == Pair.second || isBetterCandidate(BestFn, Pair.second);
+ };
+
+ // Note: We explicitly leave Matches unmodified if there isn't a clear best
+ // option, so we can potentially give the user a better error
+ if (!std::all_of(Matches.begin(), Matches.end(), IsBestOrInferiorToBest))
+ return false;
+ Matches[0] = *Best;
+ Matches.resize(1);
+ return true;
+ }
+
bool isTargetTypeAFunction() const {
return TargetFunctionType->isFunctionType();
}
Modified: cfe/trunk/test/CodeGen/enable_if.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/enable_if.c?rev=250486&r1=250485&r2=250486&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/enable_if.c (original)
+++ cfe/trunk/test/CodeGen/enable_if.c Thu Oct 15 20:17:38 2015
@@ -49,3 +49,34 @@ void test2() {
// CHECK: store i8* bitcast (void (i32)* @_Z3barUa9enable_ifIXLi1EEEi to i8*)
vp1 = (void*)bar;
}
+
+void baz(int m) __attribute__((overloadable, enable_if(1, "")));
+void baz(int m) __attribute__((overloadable));
+// CHECK-LABEL: define void @test3
+void test3() {
+ // CHECK: store void (i32)* @_Z3bazUa9enable_ifIXLi1EEEi
+ void (*p)(int) = baz;
+ // CHECK: store void (i32)* @_Z3bazUa9enable_ifIXLi1EEEi
+ void (*p2)(int) = &baz;
+ // CHECK: store void (i32)* @_Z3bazUa9enable_ifIXLi1EEEi
+ p = baz;
+ // CHECK: store void (i32)* @_Z3bazUa9enable_ifIXLi1EEEi
+ p = &baz;
+}
+
+
+const int TRUEFACTS = 1;
+void qux(int m) __attribute__((overloadable, enable_if(1, ""),
+ enable_if(TRUEFACTS, "")));
+void qux(int m) __attribute__((overloadable, enable_if(1, "")));
+// CHECK-LABEL: define void @test4
+void test4() {
+ // CHECK: store void (i32)* @_Z3quxUa9enable_ifIXLi1EEXL_Z9TRUEFACTSEEEi
+ void (*p)(int) = qux;
+ // CHECK: store void (i32)* @_Z3quxUa9enable_ifIXLi1EEXL_Z9TRUEFACTSEEEi
+ void (*p2)(int) = &qux;
+ // CHECK: store void (i32)* @_Z3quxUa9enable_ifIXLi1EEXL_Z9TRUEFACTSEEEi
+ p = qux;
+ // CHECK: store void (i32)* @_Z3quxUa9enable_ifIXLi1EEXL_Z9TRUEFACTSEEEi
+ p = &qux;
+}
Modified: cfe/trunk/test/CodeGenCXX/enable_if.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/enable_if.cpp?rev=250486&r1=250485&r2=250486&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/enable_if.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/enable_if.cpp Thu Oct 15 20:17:38 2015
@@ -1,4 +1,13 @@
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-pc-linux-gnu| FileCheck %s
+
+// Test address-of overloading logic
+int test5(int);
+template <typename T>
+T test5(T) __attribute__((enable_if(1, "better than non-template")));
+
+// CHECK: @_Z5test5IiEUa9enable_ifIXLi1EEET_S0_
+int (*Ptr)(int) = &test5;
+
// Test itanium mangling for attribute enable_if
// CHECK: _Z5test1Ua9enable_ifIXeqfL0p_Li1EEEi
Modified: cfe/trunk/test/Sema/enable_if.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/enable_if.c?rev=250486&r1=250485&r2=250486&view=diff
==============================================================================
--- cfe/trunk/test/Sema/enable_if.c (original)
+++ cfe/trunk/test/Sema/enable_if.c Thu Oct 15 20:17:38 2015
@@ -5,6 +5,8 @@
typedef int mode_t;
typedef unsigned long size_t;
+const int TRUE = 1;
+
int open(const char *pathname, int flags) __attribute__((enable_if(!(flags & O_CREAT), "must specify mode when using O_CREAT"))) __attribute__((overloadable)); // expected-note{{candidate disabled: must specify mode when using O_CREAT}}
int open(const char *pathname, int flags, mode_t mode) __attribute__((overloadable)); // expected-note{{candidate function not viable: requires 3 arguments, but 2 were provided}}
@@ -118,20 +120,20 @@ void test_return_cst() { return_cst(); }
void f2(void) __attribute__((overloadable)) __attribute__((enable_if(1, "always chosen")));
void f2(void) __attribute__((overloadable)) __attribute__((enable_if(0, "never chosen")));
-void f2(void) __attribute__((overloadable));
+void f2(void) __attribute__((overloadable)) __attribute__((enable_if(TRUE, "always chosen #2")));
void test6() {
- void (*p1)(void) = &f2; // expected-error{{initializing 'void (*)(void)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 119{{candidate function}} expected-note at 120{{candidate function made ineligible by enable_if}} expected-note at 121{{candidate function}}
- void (*p2)(void) = f2; // expected-error{{initializing 'void (*)(void)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 119{{candidate function}} expected-note at 120{{candidate function made ineligible by enable_if}} expected-note at 121{{candidate function}}
- void *p3 = (void*)&f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note at 119{{candidate function}} expected-note at 120{{candidate function made ineligible by enable_if}} expected-note at 121{{candidate function}}
- void *p4 = (void*)f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note at 119{{candidate function}} expected-note at 120{{candidate function made ineligible by enable_if}} expected-note at 121{{candidate function}}
+ void (*p1)(void) = &f2; // expected-error{{initializing 'void (*)(void)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 121{{candidate function}} expected-note at 122{{candidate function made ineligible by enable_if}} expected-note at 123{{candidate function}}
+ void (*p2)(void) = f2; // expected-error{{initializing 'void (*)(void)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 121{{candidate function}} expected-note at 122{{candidate function made ineligible by enable_if}} expected-note at 123{{candidate function}}
+ void *p3 = (void*)&f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note at 121{{candidate function}} expected-note at 122{{candidate function made ineligible by enable_if}} expected-note at 123{{candidate function}}
+ void *p4 = (void*)f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note at 121{{candidate function}} expected-note at 122{{candidate function made ineligible by enable_if}} expected-note at 123{{candidate function}}
}
void f3(int m) __attribute__((overloadable)) __attribute__((enable_if(m >= 0, "positive")));
void f3(int m) __attribute__((overloadable)) __attribute__((enable_if(m < 0, "negative")));
void test7() {
- void (*p1)(int) = &f3; // expected-error{{initializing 'void (*)(int)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 129{{candidate function made ineligible by enable_if}} expected-note at 130{{candidate function made ineligible by enable_if}}
- void (*p2)(int) = f3; // expected-error{{initializing 'void (*)(int)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 129{{candidate function made ineligible by enable_if}} expected-note at 130{{candidate function made ineligible by enable_if}}
- void *p3 = (void*)&f3; // expected-error{{address of overloaded function 'f3' does not match required type 'void'}} expected-note at 129{{candidate function made ineligible by enable_if}} expected-note at 130{{candidate function made ineligible by enable_if}}
- void *p4 = (void*)f3; // expected-error{{address of overloaded function 'f3' does not match required type 'void'}} expected-note at 129{{candidate function made ineligible by enable_if}} expected-note at 130{{candidate function made ineligible by enable_if}}
+ void (*p1)(int) = &f3; // expected-error{{initializing 'void (*)(int)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 131{{candidate function made ineligible by enable_if}} expected-note at 132{{candidate function made ineligible by enable_if}}
+ void (*p2)(int) = f3; // expected-error{{initializing 'void (*)(int)' with an expression of incompatible type '<overloaded function type>'}} expected-note at 131{{candidate function made ineligible by enable_if}} expected-note at 132{{candidate function made ineligible by enable_if}}
+ void *p3 = (void*)&f3; // expected-error{{address of overloaded function 'f3' does not match required type 'void'}} expected-note at 131{{candidate function made ineligible by enable_if}} expected-note at 132{{candidate function made ineligible by enable_if}}
+ void *p4 = (void*)f3; // expected-error{{address of overloaded function 'f3' does not match required type 'void'}} expected-note at 131{{candidate function made ineligible by enable_if}} expected-note at 132{{candidate function made ineligible by enable_if}}
}
#endif
Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=250486&r1=250485&r2=250486&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Thu Oct 15 20:17:38 2015
@@ -189,7 +189,7 @@ namespace FnPtrs {
}
int ovlConflict(int m) __attribute__((enable_if(true, "")));
- int ovlConflict(int m);
+ int ovlConflict(int m) __attribute__((enable_if(1, "")));
void test3() {
int (*p)(int) = ovlConflict; // expected-error{{address of overloaded function 'ovlConflict' is ambiguous}} expected-note at 191{{candidate function}} expected-note at 192{{candidate function}}
int (*p2)(int) = &ovlConflict; // expected-error{{address of overloaded function 'ovlConflict' is ambiguous}} expected-note at 191{{candidate function}} expected-note at 192{{candidate function}}
@@ -225,7 +225,7 @@ namespace FnPtrs {
template <typename T>
T templatedConflict(T m) __attribute__((enable_if(true, ""))) { return T(); }
template <typename T>
- T templatedConflict(T m) { return T(); }
+ T templatedConflict(T m) __attribute__((enable_if(1, ""))) { return T(); }
void test6() {
int (*p)(int) = templatedConflict<int>; // expected-error{{address of overloaded function 'templatedConflict' is ambiguous}} expected-note at 224{{candidate function made ineligible by enable_if}} expected-note at 226{{candidate function}} expected-note at 228{{candidate function}}
int (*p0)(int) = &templatedConflict<int>; // expected-error{{address of overloaded function 'templatedConflict' is ambiguous}} expected-note at 224{{candidate function made ineligible by enable_if}} expected-note at 226{{candidate function}} expected-note at 228{{candidate function}}
More information about the cfe-commits
mailing list