r269154 - [Sema] Fix value-dependent enable_if bug.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Tue May 10 18:38:27 PDT 2016
Author: gbiv
Date: Tue May 10 20:38:27 2016
New Revision: 269154
URL: http://llvm.org/viewvc/llvm-project?rev=269154&view=rev
Log:
[Sema] Fix value-dependent enable_if bug.
This patch fixes a bug where we would assume all value-dependent
enable_if conditions give successful results.
Instead, we consider value-dependent enable_if conditions to always
fail. While this isn't ideal, this is the best we can realistically do
without changing both enable_if's semantics and large parts of Sema
(specifically, all of the parts that don't expect type dependence to
come out of nowhere, and that may interact with overload resolution).
Differential Revision: http://reviews.llvm.org/D20130
Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/SemaCXX/enable_if.cpp
Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=269154&r1=269153&r2=269154&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Tue May 10 20:38:27 2016
@@ -283,6 +283,32 @@ their address taken, unless all of the c
ptr = &i; // OK: 'TrueConstant' is a truthy constant
ptr = &j; // error: 'FalseConstant' is a constant, but not truthy
}
+
+Because ``enable_if`` evaluation happens during overload resolution,
+``enable_if`` may give unintuitive results when used with templates, depending
+on when overloads are resolved. In the example below, clang will emit a
+diagnostic about no viable overloads for ``foo`` in ``bar``, but not in ``baz``:
+
+.. code-block:: c++
+
+ double foo(int i) __attribute__((enable_if(i > 0, "")));
+ void *foo(int i) __attribute__((enable_if(i <= 0, "")));
+ template <int I>
+ auto bar() { return foo(I); }
+
+ template <typename T>
+ auto baz() { return foo(T::number); }
+
+ struct WithNumber { constexpr static int number = 1; };
+ void callThem() {
+ bar<sizeof(WithNumber)>();
+ baz<WithNumber>();
+ }
+
+This is because, in ``bar``, ``foo`` is resolved prior to template
+instantiation, so the value for ``I`` isn't known (thus, both ``enable_if``
+conditions for ``foo`` fail). However, in ``baz``, ``foo`` is resolved during
+template instantiation, so the value for ``T::number`` is known.
}];
}
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=269154&r1=269153&r2=269154&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue May 10 20:38:27 2016
@@ -5984,7 +5984,6 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
SFINAETrap Trap(*this);
SmallVector<Expr *, 16> ConvertedArgs;
bool InitializationFailed = false;
- bool ContainsValueDependentExpr = false;
// Convert the arguments.
for (unsigned I = 0, E = Args.size(); I != E; ++I) {
@@ -6006,7 +6005,6 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
break;
}
- ContainsValueDependentExpr |= R.get()->isValueDependent();
ConvertedArgs.push_back(R.get());
}
@@ -6027,7 +6025,6 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
InitializationFailed = true;
break;
}
- ContainsValueDependentExpr |= R.get()->isValueDependent();
ConvertedArgs.push_back(R.get());
}
@@ -6037,18 +6034,14 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
for (auto *EIA : EnableIfAttrs) {
APValue Result;
- if (EIA->getCond()->isValueDependent()) {
- // Don't even try now, we'll examine it after instantiation.
- continue;
- }
-
+ // FIXME: This doesn't consider value-dependent cases, because doing so is
+ // very difficult. Ideally, we should handle them more gracefully.
if (!EIA->getCond()->EvaluateWithSubstitution(
- Result, Context, Function, llvm::makeArrayRef(ConvertedArgs))) {
- if (!ContainsValueDependentExpr)
- return EIA;
- } else if (!Result.isInt() || !Result.getInt().getBoolValue()) {
+ Result, Context, Function, llvm::makeArrayRef(ConvertedArgs)))
+ return EIA;
+
+ if (!Result.isInt() || !Result.getInt().getBoolValue())
return EIA;
- }
}
return nullptr;
}
Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=269154&r1=269153&r2=269154&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Tue May 10 20:38:27 2016
@@ -116,9 +116,9 @@ template <typename T> class C {
void g() { f(); }
};
-int fn3(bool b) __attribute__((enable_if(b, "")));
+int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
template <class T> void test3() {
- fn3(sizeof(T) == 1);
+ fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note at -2{{candidate disabled}}
}
template <typename T>
@@ -138,7 +138,7 @@ void test4() {
void h(int);
template <typename T> void outer() {
void local_function() __attribute__((enable_if(::h(T()), "")));
- local_function();
+ local_function(); // expected-error{{no matching function for call to 'local_function'}} expected-note at -1{{candidate disabled}}
};
namespace PR20988 {
@@ -158,9 +158,9 @@ namespace PR20988 {
fn2(expr); // expected-error{{no matching function for call to 'fn2'}}
}
- int fn3(bool b) __attribute__((enable_if(b, "")));
+ int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
template <class T> void test3() {
- fn3(sizeof(T) == 1);
+ fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note at -2{{candidate disabled}}
}
}
@@ -386,3 +386,34 @@ void runFoo() {
f.bar(1, 2); // expected-error{{too many arguments}}
}
}
+
+// Ideally, we should be able to handle value-dependent expressions sanely.
+// Sadly, that isn't the case at the moment.
+namespace dependent {
+int error(int N) __attribute__((enable_if(N, ""))); // expected-note{{candidate disabled}}
+int error(int N) __attribute__((enable_if(!N, ""))); // expected-note{{candidate disabled}}
+template <int N> int callUnavailable() {
+ return error(N); // expected-error{{no matching function for call to 'error'}}
+}
+
+constexpr int noError(int N) __attribute__((enable_if(N, ""))) { return -1; }
+constexpr int noError(int N) __attribute__((enable_if(!N, ""))) { return -1; }
+constexpr int noError(int N) { return 0; }
+
+template <int N>
+constexpr int callNoError() { return noError(N); }
+static_assert(callNoError<0>() == 0, "");
+static_assert(callNoError<1>() == 0, "");
+
+template <int N> constexpr int templated() __attribute__((enable_if(N, ""))) {
+ return 1;
+}
+
+constexpr int A = templated<0>(); // expected-error{{no matching function for call to 'templated'}} expected-note at -4{{candidate disabled}}
+static_assert(templated<1>() == 1, "");
+
+template <int N> constexpr int callTemplated() { return templated<N>(); }
+
+constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error at -2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note at -9{{candidate disabled}}
+static_assert(callTemplated<1>() == 1, "");
+}
More information about the cfe-commits
mailing list