[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
EricWF commandeered this revision. EricWF edited reviewers, added: arphaman; removed: EricWF. EricWF added a comment. I committed a different version of this fix in r312890. Commandeering and closing this review. @arphaman Thanks for the initial patch. Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
mclow.lists added inline comments.
Comment at:
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp:22
+int main() {
+ return 0;
+}
Is copy constructible; can be instantiated.
static_assert( std::is_copy_constructible::value, ""
);
IncompleteReturnType ir;
assert( ir.fn == nullptr );
Repository:
rL LLVM
https://reviews.llvm.org/D37104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
rsmith added inline comments.
Comment at:
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp:17-19
+struct IncompleteReturnType {
+ std::function fn;
+};
Would it make sense to also check that this struct is copy-constructible?
As-is, whether this test tests anything is entirely dependent on what amount to
implementation details of the constructor.
Repository:
rL LLVM
https://reviews.llvm.org/D37104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
mclow.lists added inline comments. Comment at: test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp:1 +//===--===// +// mclow.lists wrote: > Is this file in the right place? > If it's supposed to test functionality that is required by the standard, it > should go in `test/std` somewhere. > If it's supposed to test a libc++ extension, then it should go into > `test/libcxx`. Never mind - I missed the "Non-standard extension" bit. Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
mclow.lists added inline comments. Comment at: test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp:1 +//===--===// +// Is this file in the right place? If it's supposed to test functionality that is required by the standard, it should go in `test/std` somewhere. If it's supposed to test a libc++ extension, then it should go into `test/libcxx`. Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
arphaman added a comment. FYI, I'd like to see this merged into LLVM5 if it's still possible, but it's not super critical Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
arphaman added a comment. Not at all, go ahead Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
EricWF added a comment. @arphaman Do you mind if I commit my own version of this? Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
arphaman created this revision.
This patch fixes PR34298 (https://bugs.llvm.org/show_bug.cgi?id=34298). Since
Clang changed in r284549, Clang and libc++ prohibit the use of `std::function`
with an incomplete return type, like in the example below:
struct Continuation {
std::function fn;
};
This code failed to compile because of the way SFINAE checks were performed in
libc++. Essentially when `Continuation` was defining a copy-constructor, it
tried to find a matching overload for the copy constructor of `fn`, and thus
tried to instantiate `std::function`s `function(_Fp)` constructor with a
`std::function` substitute for `_Fp`. That constructor did
check against `function` in its SFINAE checks, but it did so after trying to
invoke `_Fp`. This caused an error while evaluating `__invokable_r` because
`Continuation` is incomplete, and the incomplete type checker for
`__invokable_r` didn't take `std::function<>` types into account.
This patch ensures that the SFINAE check verify that `_Fp` is not a
`std::function` before trying to figure out if `_Fp` is invokable.
Repository:
rL LLVM
https://reviews.llvm.org/D37104
Files:
include/functional
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
Index:
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
===
--- /dev/null
+++
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Non-standard extension:
+// std::function is allowed to take a function with an incomplete return type.
+
+// [func.require]
+
+#include
+
+struct IncompleteReturnType {
+ std::function fn;
+};
+
+int main() {
+ return 0;
+}
Index: include/functional
===
--- include/functional
+++ include/functional
@@ -1623,7 +1623,10 @@
function(const function&);
function(function&&) _NOEXCEPT;
template::value && !is_same<_Fp, function>::value
+is_same::value>::type,
+void
+>::value &&
+__callable<_Fp>::value
>::type>
function(_Fp);
@@ -1648,8 +1651,11 @@
template
typename enable_if
<
-__callable::type>::value &&
-!is_same::type, function>::value,
+is_same::type,
function>::value>::type,
+ void
+>::value &&
+__callable::type>::value,
function&
>::type
operator=(_Fp&&);
@@ -1857,8 +1863,11 @@
template
typename enable_if
<
-function<_Rp(_ArgTypes...)>::template __callable::type>::value &&
-!is_same::type,
function<_Rp(_ArgTypes...)>>::value,
+is_same::type,
function<_Rp(_ArgTypes...)>>::value>::type,
+ void
+>::value &&
+function<_Rp(_ArgTypes...)>::template __callable::type>::value,
function<_Rp(_ArgTypes...)>&
>::type
function<_Rp(_ArgTypes...)>::operator=(_Fp&& __f)
Index: test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
===
--- /dev/null
+++ test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Non-standard extension:
+// std::function is allowed to take a function with an incomplete return type.
+
+// [func.require]
+
+#include
+
+struct IncompleteReturnType {
+ std::function fn;
+};
+
+int main() {
+ return 0;
+}
Index: include/functional
===
--- include/functional
+++ include/functional
@@ -1623,7 +1623,10 @@
function(const function&);
function(function&&) _NOEXCEPT;
template::value && !is_same<_Fp, function>::value
+is_same::value>::type,
+void
+>::value &&
+__callable<_Fp>::value
>::type>
function(_Fp);
@@ -1648,8 +1651,11 @@
template
typename enable_if
<
-__callable::type>::value &&
-!is_same::type, function>::value,
+is_same::type, function>::value>::type,
+ void
+>::value &&
+__callable::type>::value,
function&
>::type
operator=(_Fp&&);
@@
