revise Indestructible to elide dtors

Summary:
For global variables with nontrivial destructors, the compiler inserts calls to `atexit` to destroy the variables at shutdown time. For global variables with trivial destructors, the compiler elides such calls. Revise `Indestructible` to have a trivial destructor.

Some compilers optimize further and remove such calls even for some nontrivial destructors, but not all do so.

Also remove move-construction and move-assignment. And remove actual `constexpr`-constructibility since `::new` is not a constant expression.

Reviewed By: chadaustin

Differential Revision: D34026842

fbshipit-source-id: 9ab2f957cbe9a67809a6c99e6b9370c1f4315777

folly/Indestructible.h

@@ -17,10 +17,12 @@
 #pragma once
 
 #include <cassert>
+#include <new>
 #include <type_traits>
 #include <utility>
 
 #include <folly/Traits.h>
+#include <folly/Utility.h>
 
 namespace folly {
 
@@ -61,7 +63,7 @@ template <typename T>
 class Indestructible final {
  public:
   template <typename S = T, typename = decltype(S())>
-  constexpr Indestructible() noexcept(noexcept(T())) {}
+  constexpr Indestructible() noexcept(noexcept(T())) : storage_{in_place} {}
 
   /**
    * Constructor accepting a single argument by forwarding reference, this
@@ -87,7 +89,7 @@ class Indestructible final {
       std::enable_if_t<!std::is_convertible<U&&, T>::value>* = nullptr>
   explicit constexpr Indestructible(U&& u) noexcept(
       noexcept(T(std::declval<U>())))
-      : storage_(std::forward<U>(u)) {}
+      : storage_{in_place, std::forward<U>(u)} {}
   template <
       typename U = T,
       std::enable_if_t<std::is_constructible<T, U&&>::value>* = nullptr,
@@ -97,12 +99,12 @@ class Indestructible final {
       std::enable_if_t<std::is_convertible<U&&, T>::value>* = nullptr>
   /* implicit */ constexpr Indestructible(U&& u) noexcept(
       noexcept(T(std::declval<U>())))
-      : storage_(std::forward<U>(u)) {}
+      : storage_{in_place, std::forward<U>(u)} {}
 
   template <typename... Args, typename = decltype(T(std::declval<Args>()...))>
   explicit constexpr Indestructible(Args&&... args) noexcept(
       noexcept(T(std::declval<Args>()...)))
-      : storage_(std::forward<Args>(args)...) {}
+      : storage_{in_place, std::forward<Args>(args)...} {}
   template <
       typename U,
       typename... Args,
@@ -111,31 +113,14 @@ class Indestructible final {
   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;
+      : storage_{in_place, il, std::forward<Args>(args)...} {}
 
   Indestructible(Indestructible const&) = delete;
   Indestructible& operator=(Indestructible const&) = delete;
 
-  Indestructible(Indestructible&& other) noexcept(
-      noexcept(T(std::declval<T>())))
-      : storage_(std::move(other.storage_.value)) {
-    other.erased_ = true;
-  }
-  Indestructible& operator=(Indestructible&& other) noexcept(
-      noexcept(T(std::declval<T>()))) {
-    storage_.value = std::move(other.storage_.value);
-    other.erased_ = true;
-  }
-
-  T* get() noexcept {
-    check();
-    return &storage_.value;
-  }
+  T* get() noexcept { return reinterpret_cast<T*>(&storage_.bytes); }
   T const* get() const noexcept {
-    check();
-    return &storage_.value;
+    return reinterpret_cast<T const*>(&storage_.bytes);
   }
   T& operator*() noexcept { return *get(); }
   T const& operator*() const noexcept { return *get(); }
@@ -143,23 +128,16 @@ class Indestructible final {
   T const* operator->() const noexcept { return get(); }
 
  private:
-  void check() const noexcept { assert(!erased_); }
-
-  union Storage {
-    T value;
-
-    template <typename S = T, typename = decltype(S())>
-    constexpr Storage() noexcept(noexcept(T())) : value() {}
+  struct Storage {
+    aligned_storage_for_t<T> bytes;
 
     template <typename... Args, typename = decltype(T(std::declval<Args>()...))>
-    explicit constexpr Storage(Args&&... args) noexcept(
-        noexcept(T(std::declval<Args>()...)))
-        : value(std::forward<Args>(args)...) {}
-
-    ~Storage() {}
+    explicit constexpr Storage(in_place_t, Args&&... args) noexcept(
+        noexcept(T(std::declval<Args>()...))) {
+      ::new (&bytes) T(std::forward<Args>(args)...);
+    }
   };
 
   Storage storage_{};
-  bool erased_{false};
 };
 } // namespace folly

folly/executors/InlineExecutor.cpp

@@ -21,7 +21,7 @@
 namespace folly {
 
 InlineExecutor& InlineExecutor::instance_slow() noexcept {
-  static auto instance = Indestructible<InlineExecutor>{};
+  static Indestructible<InlineExecutor> instance;
   cache.store(&*instance, std::memory_order_release);
   return *instance;
 }

folly/test/IndestructibleTest.cpp

@@ -47,12 +47,21 @@ struct Magic {
   ~Magic() { dtor_(); }
 };
 
+template <typename T>
+struct DeferredDtor {
+  folly::Indestructible<T>& obj_;
+  explicit constexpr DeferredDtor(folly::Indestructible<T>& obj) noexcept
+      : obj_{obj} {}
+  ~DeferredDtor() { obj_->~T(); }
+};
+
 class IndestructibleTest : public testing::Test {};
 } // namespace
 
 TEST_F(IndestructibleTest, access) {
-  static const Indestructible<map<string, int>> data{
+  Indestructible<map<string, int>> data{
       map<string, int>{{"key1", 17}, {"key2", 19}, {"key3", 23}}};
+  DeferredDtor s{data};
 
   auto& m = *data;
   EXPECT_EQ(19, m.at("key2"));
@@ -62,7 +71,7 @@ TEST_F(IndestructibleTest, no_destruction) {
   int state = 0;
   int value = 0;
 
-  static Indestructible<Magic> sing(
+  Indestructible<Magic> sing(
       [&] {
         ++state;
         value = 7;
@@ -77,39 +86,11 @@ TEST_F(IndestructibleTest, no_destruction) {
 }
 
 TEST_F(IndestructibleTest, empty) {
-  static const Indestructible<map<string, int>> data;
+  const Indestructible<map<string, int>> data;
   auto& m = *data;
   EXPECT_EQ(0, m.size());
 }
 
-TEST_F(IndestructibleTest, move) {
-  int state = 0;
-  int value = 0;
-  int moves = 0;
-
-  static Indestructible<Magic> sing( // move assignment
-      [&] {
-        ++state;
-        value = 7;
-      },
-      [&] { state = -1; },
-      [&] { ++moves; });
-
-  EXPECT_EQ(1, state);
-  EXPECT_EQ(7, value);
-  EXPECT_EQ(0, moves);
-
-  // move constructor
-  static Indestructible<Magic> move_ctor(std::move(sing));
-  EXPECT_EQ(1, state);
-  EXPECT_EQ(1, moves);
-
-  // move assignment
-  static Indestructible<Magic> move_assign = std::move(move_ctor);
-  EXPECT_EQ(1, state);
-  EXPECT_EQ(2, moves);
-}
-
 TEST_F(IndestructibleTest, disabled_default_ctor) {
   EXPECT_TRUE((std::is_constructible<Indestructible<int>>::value)) << "sanity";
 
@@ -122,7 +103,9 @@ TEST_F(IndestructibleTest, disabled_default_ctor) {
 }
 
 TEST_F(IndestructibleTest, list_initialization) {
-  auto map = folly::Indestructible<std::map<int, int>>{{{1, 2}}};
+  folly::Indestructible<std::map<int, int>> map{{{1, 2}}};
+  DeferredDtor s{map};
+
   EXPECT_EQ(map->at(1), 2);
 }