From 5623085aabacd5ded93e98f06ba0779133457ad6 Mon Sep 17 00:00:00 2001
From: Chao Yang <chaoyc@fb.com>
Date: Thu, 30 Apr 2015 11:26:23 -0700
Subject: [PATCH] Tag dispatch for enqueue/dequeue implementation

Summary:
clang (>=3.6?) reports potential object slicing bug when MPMCQueue is used for
polymorphic class as the queue item, e.g. as in P19814469. This can be false
positive however, since the choice is based on the type trait already.  This
diff uses tag dispatch to selectively compile the overload that will be
executed, therefore if there is no-throw move ctor supplied clang will not
examine the simulated relocation code.

This doesn't avoid object slicing bug however if the client insists to use
MPMCQueue to hold base class while enqueue and dequeue with subclassed item.

Test Plan: compile with --clang

Reviewed By: tudorb@fb.com

Subscribers: folly-diffs@, yfeldblum, chalfant

FB internal diff: D2029949

Signature: t1:2029949:1430264357:af479117adf90bc1915c071e7376a30aacb72f46
---
 folly/MPMCQueue.h | 104 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 72 insertions(+), 32 deletions(-)

diff --git a/folly/MPMCQueue.h b/folly/MPMCQueue.h
index ede1716d..2ab40033 100644
--- a/folly/MPMCQueue.h
+++ b/folly/MPMCQueue.h
@@ -782,7 +782,7 @@ struct SingleElementQueue {
   /// enqueue using in-place noexcept construction
   template <typename ...Args,
             typename = typename std::enable_if<
-                std::is_nothrow_constructible<T,Args...>::value>::type>
+              std::is_nothrow_constructible<T,Args...>::value>::type>
   void enqueue(const uint32_t turn,
                Atom<uint32_t>& spinCutoff,
                const bool updateSpinCutoff,
@@ -803,19 +803,13 @@ struct SingleElementQueue {
                Atom<uint32_t>& spinCutoff,
                const bool updateSpinCutoff,
                T&& goner) noexcept {
-    if (std::is_nothrow_constructible<T,T&&>::value) {
-      // this is preferred
-      sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff);
-      new (&contents_) T(std::move(goner));
-      sequencer_.completeTurn(turn * 2);
-    } else {
-      // simulate nothrow move with relocation, followed by default
-      // construction to fill the gap we created
-      sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff);
-      memcpy(&contents_, &goner, sizeof(T));
-      sequencer_.completeTurn(turn * 2);
-      new (&goner) T();
-    }
+    enqueueImpl(
+        turn,
+        spinCutoff,
+        updateSpinCutoff,
+        std::move(goner),
+        typename std::conditional<std::is_nothrow_constructible<T,T&&>::value,
+                                  ImplByMove, ImplByRelocation>::type());
   }
 
   bool mayEnqueue(const uint32_t turn) const noexcept {
@@ -826,24 +820,13 @@ struct SingleElementQueue {
                Atom<uint32_t>& spinCutoff,
                const bool updateSpinCutoff,
                T& elem) noexcept {
-    if (folly::IsRelocatable<T>::value) {
-      // this version is preferred, because we do as much work as possible
-      // before waiting
-      try {
-        elem.~T();
-      } catch (...) {
-        // unlikely, but if we don't complete our turn the queue will die
-      }
-      sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
-      memcpy(&elem, &contents_, sizeof(T));
-      sequencer_.completeTurn(turn * 2 + 1);
-    } else {
-      // use nothrow move assignment
-      sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
-      elem = std::move(*ptr());
-      destroyContents();
-      sequencer_.completeTurn(turn * 2 + 1);
-    }
+    dequeueImpl(turn,
+                spinCutoff,
+                updateSpinCutoff,
+                elem,
+                typename std::conditional<folly::IsRelocatable<T>::value,
+                                          ImplByRelocation,
+                                          ImplByMove>::type());
   }
 
   bool mayDequeue(const uint32_t turn) const noexcept {
@@ -871,6 +854,63 @@ struct SingleElementQueue {
     memset(&contents_, 'Q', sizeof(T));
 #endif
   }
+
+  /// Tag classes for dispatching to enqueue/dequeue implementation.
+  struct ImplByRelocation {};
+  struct ImplByMove {};
+
+  /// enqueue using nothrow move construction.
+  void enqueueImpl(const uint32_t turn,
+                   Atom<uint32_t>& spinCutoff,
+                   const bool updateSpinCutoff,
+                   T&& goner,
+                   ImplByMove) noexcept {
+    sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff);
+    new (&contents_) T(std::move(goner));
+    sequencer_.completeTurn(turn * 2);
+  }
+
+  /// enqueue by simulating nothrow move with relocation, followed by
+  /// default construction to a noexcept relocation.
+  void enqueueImpl(const uint32_t turn,
+                   Atom<uint32_t>& spinCutoff,
+                   const bool updateSpinCutoff,
+                   T&& goner,
+                   ImplByRelocation) noexcept {
+    sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff);
+    memcpy(&contents_, &goner, sizeof(T));
+    sequencer_.completeTurn(turn * 2);
+    new (&goner) T();
+  }
+
+  /// dequeue by destructing followed by relocation.  This version is preferred,
+  /// because as much work as possible can be done before waiting.
+  void dequeueImpl(uint32_t turn,
+                   Atom<uint32_t>& spinCutoff,
+                   const bool updateSpinCutoff,
+                   T& elem,
+                   ImplByRelocation) noexcept {
+    try {
+      elem.~T();
+    } catch (...) {
+      // unlikely, but if we don't complete our turn the queue will die
+    }
+    sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
+    memcpy(&elem, &contents_, sizeof(T));
+    sequencer_.completeTurn(turn * 2 + 1);
+  }
+
+  /// dequeue by nothrow move assignment.
+  void dequeueImpl(uint32_t turn,
+                   Atom<uint32_t>& spinCutoff,
+                   const bool updateSpinCutoff,
+                   T& elem,
+                   ImplByMove) noexcept {
+    sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
+    elem = std::move(*ptr());
+    destroyContents();
+    sequencer_.completeTurn(turn * 2 + 1);
+  }
 };
 
 } // namespace detail
-- 
2.34.1