r269005 - [Sema] Fix an overload resolution bug with enable_if.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Mon May 9 18:59:35 PDT 2016
Author: gbiv
Date: Mon May 9 20:59:34 2016
New Revision: 269005
URL: http://llvm.org/viewvc/llvm-project?rev=269005&view=rev
Log:
[Sema] Fix an overload resolution bug with enable_if.
Currently, if clang::isBetterOverloadCandidate encounters an enable_if
attribute on either candidate that it's inspecting, it will ignore all
lower priority attributes (e.g. pass_object_size). This is problematic
in cases like:
```
void foo(char *c) __attribute__((enable_if(1, "")));
void foo(char *c __attribute__((pass_object_size(0))))
__attribute__((enable_if(1, "")));
```
...Because we would ignore the pass_object_size attribute in the second
`foo`, and consider any call to `foo` to be ambiguous.
This patch makes overload resolution consult further tiebreakers (e.g.
pass_object_size) if two candidates have equally good enable_if
attributes.
Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CodeGen/enable_if.c
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=269005&r1=269004&r2=269005&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Mon May 9 20:59:34 2016
@@ -8503,16 +8503,31 @@ 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(const Sema &S, const FunctionDecl *Cand1,
- const FunctionDecl *Cand2) {
+namespace {
+enum class Comparison { Equal, Better, Worse };
+}
+
+/// Compares the enable_if attributes of two FunctionDecls, for the purposes of
+/// overload resolution.
+///
+/// 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.
+///
+/// Note that you can have a pair of candidates such that Cand1's enable_if
+/// attributes are worse than Cand2's, and Cand2's enable_if attributes are
+/// worse than Cand1's.
+static Comparison compareEnableIfAttrs(const Sema &S, const FunctionDecl *Cand1,
+ const FunctionDecl *Cand2) {
+ // Common case: One (or both) decls don't have enable_if attrs.
+ bool Cand1Attr = Cand1->hasAttr<EnableIfAttr>();
+ bool Cand2Attr = Cand2->hasAttr<EnableIfAttr>();
+ if (!Cand1Attr || !Cand2Attr) {
+ if (Cand1Attr == Cand2Attr)
+ return Comparison::Equal;
+ return Cand1Attr ? Comparison::Better : Comparison::Worse;
+ }
// FIXME: The next several lines are just
// specific_attr_iterator<EnableIfAttr> but going in declaration order,
@@ -8520,10 +8535,10 @@ static bool hasBetterEnableIfAttrs(const
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;
+ // It's impossible for Cand1 to be better than (or equal to) Cand2 if Cand1
+ // has fewer enable_if attributes than Cand2.
+ if (Cand1Attrs.size() < Cand2Attrs.size())
+ return Comparison::Worse;
auto Cand1I = Cand1Attrs.begin();
llvm::FoldingSetNodeID Cand1ID, Cand2ID;
@@ -8535,10 +8550,10 @@ static bool hasBetterEnableIfAttrs(const
Cand1A->getCond()->Profile(Cand1ID, S.getASTContext(), true);
Cand2A->getCond()->Profile(Cand2ID, S.getASTContext(), true);
if (Cand1ID != Cand2ID)
- return false;
+ return Comparison::Worse;
}
- return true;
+ return Cand1I == Cand1Attrs.end() ? Comparison::Equal : Comparison::Better;
}
/// isBetterOverloadCandidate - Determines whether the first overload
@@ -8649,10 +8664,11 @@ 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>()))
- return hasBetterEnableIfAttrs(S, Cand1.Function, Cand2.Function);
+ if (Cand1.Function && Cand2.Function) {
+ Comparison Cmp = compareEnableIfAttrs(S, Cand1.Function, Cand2.Function);
+ if (Cmp != Comparison::Equal)
+ return Cmp == Comparison::Better;
+ }
if (S.getLangOpts().CUDA && Cand1.Function && Cand2.Function) {
FunctionDecl *Caller = dyn_cast<FunctionDecl>(S.CurContext);
@@ -10331,7 +10347,7 @@ private:
// disambiguate for us if there are multiple candidates and no exact match.
return candidateHasExactlyCorrectType(A) &&
(!candidateHasExactlyCorrectType(B) ||
- hasBetterEnableIfAttrs(S, A, B));
+ compareEnableIfAttrs(S, A, B) == Comparison::Better);
}
/// \return true if we were able to eliminate all but one overload candidate,
Modified: cfe/trunk/test/CodeGen/enable_if.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/enable_if.c?rev=269005&r1=269004&r2=269005&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/enable_if.c (original)
+++ cfe/trunk/test/CodeGen/enable_if.c Mon May 9 20:59:34 2016
@@ -80,3 +80,16 @@ void test4() {
// CHECK: store void (i32)* @_Z3quxUa9enable_ifIXLi1EEXL_Z9TRUEFACTSEEEi
p = &qux;
}
+
+// There was a bug where, when enable_if was present, overload resolution
+// wouldn't pay attention to lower-priority attributes.
+// (N.B. `foo` with pass_object_size should always be preferred)
+// CHECK-LABEL: define void @test5
+void test5() {
+ int foo(char *i) __attribute__((enable_if(1, ""), overloadable));
+ int foo(char *i __attribute__((pass_object_size(0))))
+ __attribute__((enable_if(1, ""), overloadable));
+
+ // CHECK: call i32 @_Z3fooUa9enable_ifIXLi1EEEPcU17pass_object_size0
+ foo((void*)0);
+}
More information about the cfe-commits
mailing list