tweaks to DelayedInit

Summary:
Tweaks to `folly::DelayedInit`:
* No potential for ambiguity in placement-new by using `::new` and casting the address to `void*`.
* Helper functions `slot` and `store` simplify `try_emplace_with` and `try_emplace`.
* `try_emplace_with` and `try_emplace` are no longer `const`-qualified, removing the need for `mutable` qualifiers.

The emplacement members should not be `const`-qualified since they are not semantically non-modifying. The `const` qualifier signals semantic non-modification. While it is the case that there are customs around `const` qualifiers and concurrency, these customs are not the core meaning of `const`.

Reviewed By: praihan

Differential Revision: D25919779

fbshipit-source-id: 95e5c4b4abf5285fbb00dddda1192998baf6ee59

folly/ConcurrentLazy.h

@@ -39,34 +39,38 @@ namespace folly {
  * Otherwise, all design considerations from `folly::Lazy` are reflected here.
  */
 
-template <class Func>
+template <class Ctor>
 struct ConcurrentLazy {
-  using result_type = invoke_result_t<Func>;
+  using result_type = invoke_result_t<Ctor>;
 
   static_assert(
-      !std::is_const<Func>::value,
+      !std::is_const<Ctor>::value,
       "Func should not be a const-qualified type");
   static_assert(
-      !std::is_reference<Func>::value,
+      !std::is_reference<Ctor>::value,
       "Func should not be a reference type");
 
-  explicit ConcurrentLazy(Func&& f) : func_(std::move(f)) {}
-  explicit ConcurrentLazy(const Func& f) : func_(f) {}
+  template <
+      typename F,
+      std::enable_if_t<std::is_constructible_v<Ctor, F>, int> = 0>
+  explicit ConcurrentLazy(F&& f) noexcept(
+      std::is_nothrow_constructible_v<Ctor, F>)
+      : ctor_(static_cast<F&&>(f)) {}
 
   const result_type& operator()() const {
-    return value_.try_emplace_with(std::ref(func_));
+    return value_.try_emplace_with(std::ref(ctor_));
   }
 
-  result_type& operator()() { return value_.try_emplace_with(std::ref(func_)); }
+  result_type& operator()() { return value_.try_emplace_with(std::ref(ctor_)); }
 
  private:
-  folly::DelayedInit<result_type> value_;
-  mutable Func func_;
+  mutable folly::DelayedInit<result_type> value_;
+  mutable Ctor ctor_;
 };
 
 template <class Func>
-auto concurrent_lazy(Func&& fun) {
-  return ConcurrentLazy<remove_cvref_t<Func>>(std::forward<Func>(fun));
+ConcurrentLazy<remove_cvref_t<Func>> concurrent_lazy(Func&& func) {
+  return ConcurrentLazy<remove_cvref_t<Func>>(static_cast<Func&&>(func));
 }
 
 } // namespace folly

folly/experimental/observer/SimpleObservable.h

@@ -51,7 +51,8 @@ class SimpleObservable {
   struct Wrapper;
   std::shared_ptr<Context> context_;
 
-  folly::DelayedInit<Observer<typename observer_detail::Unwrap<T>::type>>
+  mutable folly::DelayedInit<
+      Observer<typename observer_detail::Unwrap<T>::type>>
       observer_;
 };
 } // namespace observer

folly/synchronization/DelayedInit.h

@@ -17,7 +17,7 @@
 #pragma once
 
 #include <initializer_list>
-#include <memory>
+#include <new>
 #include <stdexcept>
 #include <type_traits>
 
@@ -80,10 +80,8 @@ struct DelayedInit {
    * then the provided function is not called.
    */
   template <typename Func>
-  T& try_emplace_with(Func func) const {
-    call_once(storage_.init, [&]() mutable {
-      new (std::addressof(storage_.value)) T(func());
-    });
+  T& try_emplace_with(Func func) {
+    call_once(storage_.init, [&] { ::new (slot()) T(func()); });
     return storage_.value;
   }
 
@@ -91,25 +89,19 @@ struct DelayedInit {
    * Gets the pre-existing value if already initialized or constructs the value
    * in-place by direct-initializing with the provided arguments.
    */
-  template <typename... Args>
-  T& try_emplace(Args&&... args) const {
-    call_once(
-        storage_.init,
-        [this](Args&&... forwardedArgs) mutable {
-          new (std::addressof(storage_.value))
-              T(std::forward<Args>(forwardedArgs)...);
-        },
-        std::forward<Args>(args)...);
+  template <typename... A>
+  T& try_emplace(A&&... a) {
+    call_once(storage_.init, [&] { store(static_cast<A&&>(a)...); });
     return storage_.value;
   }
   template <
       typename U,
-      typename... Args,
+      typename... A,
       typename = std::enable_if_t<
-          std::is_constructible<T, std::initializer_list<U>, Args...>::value>>
-  T& try_emplace(std::initializer_list<U> ilist, Args&&... args) const {
-    return try_emplace<std::initializer_list<U>, Args...>(
-        std::move(ilist), std::forward<Args>(args)...);
+          std::is_constructible<T, std::initializer_list<U>, A...>::value>>
+  T& try_emplace(std::initializer_list<U> ilist, A&&... a) {
+    call_once(storage_.init, [&] { store(ilist, static_cast<A&&>(a)...); });
+    return storage_.value;
   }
 
   bool has_value() const { return test_once(storage_.init); }
@@ -182,7 +174,14 @@ struct DelayedInit {
       StorageTriviallyDestructible,
       StorageNonTriviallyDestructible>;
 
-  mutable Storage storage_;
+  void* slot() { return static_cast<void*>(std::addressof(storage_.value)); }
+
+  template <typename... A>
+  void store(A&&... a) {
+    ::new (slot()) T(static_cast<A&&>(a)...);
+  }
+
+  Storage storage_;
 };
 
 } // namespace folly