Commit 9adda7dd authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook GitHub Bot

recursive function

Summary:
Check that recursion builds with specific versions of gcc. Fix failures with that version.

Addresses these failures:

```
In file included from folly/test/FunctionTest.cpp:17:
folly/Function.h: In substitution of 'template<class ReturnType, class ... Args> template<class F, class R> using IfSafeResult = folly::detail::function::IfSafeResultImpl<R, ReturnType> [with F = folly::Function<RecFolly()>; R = RecFolly; ReturnType = RecFolly; Args = {}]':
folly/Function.h:761:7:   required by substitution of 'template<class Signature, class> folly::Function<RecFolly()>::Function(folly::Function<F>&&) [with Signature = RecFolly(); <template-parameter-1-2> = <missing>]'
folly/Function.h:293:40:   required by substitution of 'template<class From, class To, class> using IfSafeResultImpl = decltype ((void)(static_cast<To>(declval<From>()))) [with From = RecFolly; To = RecFolly; <template-parameter-1-3> = void]'
folly/Function.h:363:9:   required by substitution of 'template<class ReturnType, class ... Args> template<class F, class R> using IfSafeResult = folly::detail::function::IfSafeResultImpl<R, ReturnType> [with F = folly::Function<RecFolly()>; R = RecFolly; ReturnType = RecFolly; Args = {}]'
folly/Function.h:761:7:   required by substitution of 'template<class Signature, class> folly::Function<RecFolly()>::Function(folly::Function<F>&&) [with Signature = RecFolly(); <template-parameter-1-2> = <missing>]'
folly/Function.h:293:40:   required by substitution of 'template<class From, class To, class> using IfSafeResultImpl = decltype ((void)(static_cast<To>(declval<From>()))) [with From = RecFolly; To = RecFolly; <template-parameter-1-3> = void]'
folly/Function.h:363:9:   [ skipping 501 instantiation contexts, use -ftemplate-backtrace-limit=0 to disable ]
folly/Function.h:363:9:   required by substitution of 'template<class ReturnType, class ... Args> template<class F, class R> using IfSafeResult = folly::detail::function::IfSafeResultImpl<R, ReturnType> [with F = folly::Function<RecFolly()>; R = RecFolly; ReturnType = RecFolly; Args = {}]'
folly/Function.h:761:7:   required by substitution of 'template<class Signature, class> folly::Function<RecFolly()>::Function(folly::Function<F>&&) [with Signature = RecFolly(); <template-parameter-1-2> = <missing>]'
folly/Function.h:293:40:   required by substitution of 'template<class From, class To, class> using IfSafeResultImpl = decltype ((void)(static_cast<To>(declval<From>()))) [with From = RecFolly; To = RecFolly; <template-parameter-1-3> = void]'
folly/Function.h:363:9:   required by substitution of 'template<class ReturnType, class ... Args> template<class F, class R> using IfSafeResult = folly::detail::function::IfSafeResultImpl<R, ReturnType> [with F = folly::Function<RecFolly()>; R = RecFolly; ReturnType = RecFolly; Args = {}]'
folly/Function.h:761:7:   required by substitution of 'template<class Signature, class> folly::Function<RecFolly()>::Function(folly::Function<F>&&) [with Signature = RecFolly(); <template-parameter-1-2> = <missing>]'
folly/test/FunctionTest.cpp:212:54:   required from here
folly/Function.h:363:9: fatal error: template instantiation depth exceeds maximum of 512 (use '-ftemplate-depth=' to increase the maximum)
  363 |   using IfSafeResult = IfSafeResultImpl<R, ReturnType>;
      |         ^~~~~~~~~~~~
compilation terminated.
```

```
In file included from folly/test/FunctionTest.cpp:17:
folly/Function.h: In member function 'virtual void Function_SelfMove_Test::TestBody()':
folly/Function.h:709:70: error: 'f.folly::Function<int()>::exec_' is used uninitialized in this function [-Werror=uninitialized]
  709 |   Function(Function&& that) noexcept : call_(that.call_), exec_(that.exec_) {
      |                                                                 ~~~~~^~~~~
folly/Function.h: In member function 'virtual void Function_SelfMove2_Test::TestBody()':
folly/Function.h:709:70: warning: 'f.folly::Function<int()>::exec_' may be used uninitialized in this function [-Wmaybe-uninitialized]
  709 |   Function(Function&& that) noexcept : call_(that.call_), exec_(that.exec_) {
      |                                                                 ~~~~~^~~~~
folly/Function.h: In member function 'virtual void Function_SelfStdSwap_Test::TestBody()':
folly/Function.h:709:70: error: 'f.folly::Function<int()>::exec_' is used uninitialized in this function [-Werror=uninitialized]
  709 |   Function(Function&& that) noexcept : call_(that.call_), exec_(that.exec_) {
      |                                                                 ~~~~~^~~~~
folly/Function.h:709:70: warning: 'f.folly::Function<int()>::exec_' may be used uninitialized in this function [-Wmaybe-uninitialized]
  709 |   Function(Function&& that) noexcept : call_(that.call_), exec_(that.exec_) {
      |                                                                 ~~~~~^~~~~
folly/Function.h:709:70: warning: 'f.folly::Function<int()>::exec_' may be used uninitialized in this function [-Wmaybe-uninitialized]
  709 |   Function(Function&& that) noexcept : call_(that.call_), exec_(that.exec_) {
      |                                                                 ~~~~~^~~~~
```

Notes:
* For some versions of gcc, the compiler sees the converting-move-ctor as a better match than the actual move-ctor and this leads the compiler to infinite recursion; to work around this, explicitly exclude the same-type case of the converting-move-ctor.
* `std::exchange` on `exec_` would be incorrect for self-move because of order of operations.
* `std::exchange` on `call_` might be correct but it would be an extra template instantiation which is not desireable for `folly::Function`.

Differential Revision: D33881266

fbshipit-source-id: 5403b83498b9c1afa772564246eba4f112495761
parent fa2f89f9
...@@ -758,7 +758,10 @@ class Function final : private detail::function::FunctionTraits<FunctionType> { ...@@ -758,7 +758,10 @@ class Function final : private detail::function::FunctionTraits<FunctionType> {
*/ */
template < template <
typename Signature, typename Signature,
typename = typename Traits::template IfSafeResult<Function<Signature>>> typename Fun = Function<Signature>,
// prevent gcc from making this a better match than move-ctor
typename = std::enable_if_t<!std::is_same<Function, Fun>::value>,
typename = typename Traits::template IfSafeResult<Fun>>
Function(Function<Signature>&& that) noexcept( Function(Function<Signature>&& that) noexcept(
noexcept(Function(std::move(that), CoerceTag{}))) noexcept(Function(std::move(that), CoerceTag{})))
: Function(std::move(that), CoerceTag{}) {} : Function(std::move(that), CoerceTag{}) {}
...@@ -800,18 +803,12 @@ class Function final : private detail::function::FunctionTraits<FunctionType> { ...@@ -800,18 +803,12 @@ class Function final : private detail::function::FunctionTraits<FunctionType> {
* then `*this` is left in a valid but unspecified state. * then `*this` is left in a valid but unspecified state.
*/ */
Function& operator=(Function&& that) noexcept { Function& operator=(Function&& that) noexcept {
// Q: Why is it safe to destroy and reconstruct this object in place? exec(Op::NUKE, &data_, nullptr);
// A: Two reasons: First, `Function` is a final class, so in doing this that.exec(Op::MOVE, &that.data_, &data_);
// we aren't slicing off any derived parts. And second, the move exec_ = that.exec_;
// operation is guaranteed not to throw so we always leave the object call_ = that.call_;
// in a valid state. that.exec_ = nullptr;
// In the case of self-move (this == &that), this leaves the object in that.call_ = &Traits::uninitCall;
// a default-constructed state. First the object is destroyed, then we
// pass the destroyed object to the move constructor. The first thing the
// move constructor does is default-construct the object. That object is
// "moved" into itself, which is a no-op for a default-constructed Function.
this->~Function();
::new (this) Function(std::move(that));
return *this; return *this;
} }
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include <array> #include <array>
#include <cstdarg> #include <cstdarg>
#include <functional>
#include <folly/Memory.h> #include <folly/Memory.h>
#include <folly/lang/Keep.h> #include <folly/lang/Keep.h>
...@@ -198,6 +199,21 @@ static_assert( ...@@ -198,6 +199,21 @@ static_assert(
static_assert(std::is_nothrow_destructible<Function<int(int)>>::value, ""); static_assert(std::is_nothrow_destructible<Function<int(int)>>::value, "");
struct RecStd {
using type = std::function<RecStd()>;
/* implicit */ RecStd(type f) : func(f) {}
explicit operator type() { return func; }
type func;
};
// Recursive class - regression case
struct RecFolly {
using type = folly::Function<RecFolly()>;
/* implicit */ RecFolly(type f) : func(std::move(f)) {}
explicit operator type() { return std::move(func); }
type func;
};
// TEST ===================================================================== // TEST =====================================================================
// InvokeFunctor & InvokeReference // InvokeFunctor & InvokeReference
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment