logging: make XLOG_GET_CATEGORY() safe for all callers

[folly.git] / folly / Function.h

diff --git a/folly/Function.h b/folly/Function.h
index 6c64647ddb95a69744cc20d858b3d95b0a092cd0..263b0af07d8ac940b186038dd9bbd5c9e4b436df 100644 (file)
--- a/folly/Function.h
+++ b/folly/Function.h
@@ -424,9 +424,6 @@ inline auto invoke(M(C::*d), Args&&... args)
 } // namespace function
 } // namespace detail
 
-FOLLY_PUSH_WARNING
-FOLLY_MSVC_DISABLE_WARNING(4521) // Multiple copy constructors
-FOLLY_MSVC_DISABLE_WARNING(4522) // Multiple assignment operators
 template <typename FunctionType>
 class Function final : private detail::function::FunctionTraits<FunctionType> {
   // These utility types are defined outside of the template to reduce
@@ -578,17 +575,23 @@ class Function final : private detail::function::FunctionTraits<FunctionType> {
 
   /**
    * Move assignment operator
+   *
+   * \note Leaves `that` in a valid but unspecified state. If `&that == this`
+   * then `*this` is left in a valid but unspecified state.
    */
   Function& operator=(Function&& that) noexcept {
-    if (&that != this) {
-      // Q: Why is is safe to destroy and reconstruct this object in place?
-      // A: Two reasons: First, `Function` is a final class, so in doing this
-      //    we aren't slicing off any derived parts. And second, the move
-      //    operation is guaranteed not to throw so we always leave the object
-      //    in a valid state.
-      this->~Function();
-      ::new (this) Function(std::move(that));
-    }
+    // Q: Why is is safe to destroy and reconstruct this object in place?
+    // A: Two reasons: First, `Function` is a final class, so in doing this
+    //    we aren't slicing off any derived parts. And second, the move
+    //    operation is guaranteed not to throw so we always leave the object
+    //    in a valid state.
+    // In the case of self-move (this == &that), this leaves the object in
+    // 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;
   }
 
@@ -694,7 +697,6 @@ class Function final : private detail::function::FunctionTraits<FunctionType> {
     return std::move(*this).asSharedProxy();
   }
 };
-FOLLY_POP_WARNING
 
 template <typename FunctionType>
 void swap(Function<FunctionType>& lhs, Function<FunctionType>& rhs) noexcept {
@@ -789,7 +791,11 @@ class FunctionRef<ReturnType(Args...)> final {
   /**
    * Construct a FunctionRef from a reference to a callable object.
    */
-  template <typename Fun>
+  template <
+      typename Fun,
+      typename std::enable_if<
+          !std::is_same<FunctionRef, typename std::decay<Fun>::type>::value,
+          int>::type = 0>
   /* implicit */ FunctionRef(Fun&& fun) noexcept {
     using ReferencedType = typename std::remove_reference<Fun>::type;