From 3c127a77f9b9fb201f98da646bbe0ea06cd6d55b Mon Sep 17 00:00:00 2001
From: Aaryaman Sagar <aary@instagram.com>
Date: Thu, 28 Dec 2017 08:53:05 -0800
Subject: [PATCH] folly::Indestructible interface improvement

Summary:
As it stands the user cannot use initializer list constructors in the
underlying type, this fixes that and provides a good interface change.
This allows them to use list initialization, which works with initializer
lists

Reviewed By: yfeldblum

Differential Revision: D6620994

fbshipit-source-id: c29199f97b434d84dd8d4cee2f00c5eccb316166
---
 folly/Indestructible.h            | 64 +++++++++++++++++++++++++++----
 folly/test/IndestructibleTest.cpp | 36 +++++++++++++++++
 2 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/folly/Indestructible.h b/folly/Indestructible.h
index 56ece823..9dbeb7d3 100644
--- a/folly/Indestructible.h
+++ b/folly/Indestructible.h
@@ -17,8 +17,11 @@
 #pragma once
 
 #include <cassert>
+#include <type_traits>
 #include <utility>
 
+#include <folly/Traits.h>
+
 namespace folly {
 
 /***
@@ -61,10 +64,57 @@ class Indestructible final {
   template <typename S = T, typename = decltype(S())>
   constexpr Indestructible() noexcept(noexcept(T())) {}
 
-  template <typename... Args, typename = decltype(T(std::declval<Args&&>()...))>
+  /**
+   * Constructor accepting a single argument by forwarding reference, this
+   * allows using list initialzation without the overhead of things like
+   * in_place, etc and also works with std::initializer_list constructors
+   * which can't be deduced, the default parameter helps there.
+   *
+   *    auto i = folly::Indestructible<std::map<int, int>>{{{1, 2}}};
+   *
+   * This provides convenience
+   *
+   * There are two versions of this constructor - one for when the element is
+   * implicitly constructible from the given argument and one for when the
+   * type is explicitly but not implicitly constructible from the given
+   * argument.
+   */
+  template <
+      typename U = T,
+      _t<std::enable_if<std::is_constructible<T, U&&>::value>>* = nullptr,
+      _t<std::enable_if<
+          !std::is_same<Indestructible<T>, remove_cvref_t<U>>::value>>* =
+          nullptr,
+      _t<std::enable_if<!std::is_convertible<U&&, T>::value>>* = nullptr>
+  explicit constexpr Indestructible(U&& u) noexcept(
+      noexcept(T(std::declval<U>())))
+      : storage_(std::forward<U>(u)) {}
+  template <
+      typename U = T,
+      _t<std::enable_if<std::is_constructible<T, U&&>::value>>* = nullptr,
+      _t<std::enable_if<
+          !std::is_same<Indestructible<T>, remove_cvref_t<U>>::value>>* =
+          nullptr,
+      _t<std::enable_if<std::is_convertible<U&&, T>::value>>* = nullptr>
+  /* implicit */ constexpr Indestructible(U&& u) noexcept(
+      noexcept(T(std::declval<U>())))
+      : storage_(std::forward<U>(u)) {}
+
+  template <typename... Args, typename = decltype(T(std::declval<Args>()...))>
   explicit constexpr Indestructible(Args&&... args) noexcept(
-      noexcept(T(std::declval<Args&&>()...)))
+      noexcept(T(std::declval<Args>()...)))
       : storage_(std::forward<Args>(args)...) {}
+  template <
+      typename U,
+      typename... Args,
+      typename = decltype(
+          T(std::declval<std::initializer_list<U>&>(),
+            std::declval<Args>()...))>
+  explicit constexpr Indestructible(std::initializer_list<U> il, Args... args) noexcept(
+      noexcept(
+          T(std::declval<std::initializer_list<U>&>(),
+            std::declval<Args>()...)))
+      : storage_(il, std::forward<Args>(args)...) {}
 
   ~Indestructible() = default;
 
@@ -72,12 +122,12 @@ class Indestructible final {
   Indestructible& operator=(Indestructible const&) = delete;
 
   Indestructible(Indestructible&& other) noexcept(
-      noexcept(T(std::declval<T&&>())))
+      noexcept(T(std::declval<T>())))
       : storage_(std::move(other.storage_.value)) {
     other.erased_ = true;
   }
   Indestructible& operator=(Indestructible&& other) noexcept(
-      noexcept(T(std::declval<T&&>()))) {
+      noexcept(T(std::declval<T>()))) {
     storage_.value = std::move(other.storage_.value);
     other.erased_ = true;
   }
@@ -106,11 +156,9 @@ class Indestructible final {
     template <typename S = T, typename = decltype(S())>
     constexpr Storage() noexcept(noexcept(T())) : value() {}
 
-    template <
-        typename... Args,
-        typename = decltype(T(std::declval<Args&&>()...))>
+    template <typename... Args, typename = decltype(T(std::declval<Args>()...))>
     explicit constexpr Storage(Args&&... args) noexcept(
-        noexcept(T(std::declval<Args&&>()...)))
+        noexcept(T(std::declval<Args>()...)))
         : value(std::forward<Args>(args)...) {}
 
     ~Storage() {}
diff --git a/folly/test/IndestructibleTest.cpp b/folly/test/IndestructibleTest.cpp
index ad57b1fa..a4869837 100644
--- a/folly/test/IndestructibleTest.cpp
+++ b/folly/test/IndestructibleTest.cpp
@@ -20,6 +20,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <tuple>
 
 #include <folly/Memory.h>
 #include <folly/portability/GTest.h>
@@ -119,3 +120,38 @@ TEST_F(IndestructibleTest, disabled_default_ctor) {
   EXPECT_FALSE((std::is_constructible<Indestructible<Foo>, Magic>::value));
   EXPECT_TRUE((std::is_constructible<Indestructible<Foo>, int>::value));
 }
+
+TEST_F(IndestructibleTest, list_initialization) {
+  auto map = folly::Indestructible<std::map<int, int>>{{{1, 2}}};
+  EXPECT_EQ(map->at(1), 2);
+}
+
+namespace {
+class InitializerListConstructible {
+ public:
+  InitializerListConstructible(InitializerListConstructible&&) = default;
+  explicit InitializerListConstructible(std::initializer_list<int>) {}
+  InitializerListConstructible(std::initializer_list<double>, double) {}
+};
+} // namespace
+
+TEST_F(IndestructibleTest, initializer_list_in_place_initialization) {
+  using I = InitializerListConstructible;
+  std::ignore = Indestructible<I>{{1, 2, 3, 4}};
+  std::ignore = Indestructible<I>{{1.2}, 4.2};
+}
+
+namespace {
+class ExplicitlyMoveConstructible {
+ public:
+  ExplicitlyMoveConstructible() = default;
+  explicit ExplicitlyMoveConstructible(ExplicitlyMoveConstructible&&) = default;
+};
+} // namespace
+
+TEST_F(IndestructibleTest, list_initialization_explicit_implicit) {
+  using E = ExplicitlyMoveConstructible;
+  using I = std::map<int, int>;
+  EXPECT_TRUE((!std::is_convertible<E, Indestructible<E>>::value));
+  EXPECT_TRUE((std::is_convertible<I, Indestructible<I>>::value));
+}
-- 
2.34.1