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@119{{candidate function}} expected-note@120{{candidate function made ineligible by enable_if}} expected-note@121{{candidate function}} - void (*p2)(void) = f2; // expected-error{{initializing 'void (*)(void)' with an expression of incompatible type '<overloaded function type>'}} expected-note@119{{candidate function}} expected-note@120{{candidate function made ineligible by enable_if}} expected-note@121{{candidate function}} - void *p3 = (void*)&f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note@119{{candidate function}} expected-note@120{{candidate function made ineligible by enable_if}} expected-note@121{{candidate function}} - void *p4 = (void*)f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note@119{{candidate function}} expected-note@120{{candidate function made ineligible by enable_if}} expected-note@121{{candidate function}} + void (*p1)(void) = &f2; // expected-error{{initializing 'void (*)(void)' with an expression of incompatible type '<overloaded function type>'}} expected-note@121{{candidate function}} expected-note@122{{candidate function made ineligible by enable_if}} expected-note@123{{candidate function}} + void (*p2)(void) = f2; // expected-error{{initializing 'void (*)(void)' with an expression of incompatible type '<overloaded function type>'}} expected-note@121{{candidate function}} expected-note@122{{candidate function made ineligible by enable_if}} expected-note@123{{candidate function}} + void *p3 = (void*)&f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note@121{{candidate function}} expected-note@122{{candidate function made ineligible by enable_if}} expected-note@123{{candidate function}} + void *p4 = (void*)f2; // expected-error{{address of overloaded function 'f2' is ambiguous}} expected-note@121{{candidate function}} expected-note@122{{candidate function made ineligible by enable_if}} expected-note@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@129{{candidate function made ineligible by enable_if}} expected-note@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@129{{candidate function made ineligible by enable_if}} expected-note@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@129{{candidate function made ineligible by enable_if}} expected-note@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@129{{candidate function made ineligible by enable_if}} expected-note@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@131{{candidate function made ineligible by enable_if}} expected-note@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@131{{candidate function made ineligible by enable_if}} expected-note@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@131{{candidate function made ineligible by enable_if}} expected-note@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@131{{candidate function made ineligible by enable_if}} expected-note@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@191{{candidate function}} expected-note@192{{candidate function}} int (*p2)(int) = &ovlConflict; // expected-error{{address of overloaded function 'ovlConflict' is ambiguous}} expected-note@191{{candidate function}} expected-note@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@224{{candidate function made ineligible by enable_if}} expected-note@226{{candidate function}} expected-note@228{{candidate function}} int (*p0)(int) = &templatedConflict<int>; // expected-error{{address of overloaded function 'templatedConflict' is ambiguous}} expected-note@224{{candidate function made ineligible by enable_if}} expected-note@226{{candidate function}} expected-note@228{{candidate function}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
