From f98cd52265e495efa29d715e1e179f1a26783534 Mon Sep 17 00:00:00 2001
From: Paul Nathan Stickney <pstickne@gmail.com>
Date: Sun, 27 Oct 2019 19:12:19 -0700
Subject: [PATCH 1/7] #521: eliminate lambda-closure (and VirtualMsgPtr
 entirely)

- Folded VirtualMsgPtr into MsgSharedPtr making
  'to' and 'toVirtual' synonyms.

  If the wrapped message type has a non-trivial destructor
  then a proxy 'deref' object is is used so that all derefs
  will apply to the original T-type (and thus invoke the
  delete-expression appropriately).

  This type-erasure ensures that 'messageDeref' is always
  invoked upon the correct T for expeted delete-expr behavior.

- Remove unused code and unified methods

- Ensure move contructor and added move-assignment operator
  (steal resources!!!!)

- Remove messageDeref<Msg<T>> templates; if having a MsgSharedPtr, use it..
---
 src/vt/messaging/message/refs.h              |   6 -
 src/vt/messaging/message/refs.impl.h         |  10 -
 src/vt/messaging/message/smart_ptr.h         | 274 +++++++++++--------
 src/vt/messaging/message/smart_ptr_virtual.h | 104 -------
 src/vt/messaging/pending_send.cc             |   2 +-
 src/vt/objgroup/manager.cc                   |   2 +-
 src/vt/objgroup/manager.fwd.h                |   1 -
 src/vt/objgroup/manager.h                    |   2 +-
 8 files changed, 158 insertions(+), 243 deletions(-)
 delete mode 100644 src/vt/messaging/message/smart_ptr_virtual.h

diff --git a/src/vt/messaging/message/refs.h b/src/vt/messaging/message/refs.h
index ccea1bba36..26d70383dd 100644
--- a/src/vt/messaging/message/refs.h
+++ b/src/vt/messaging/message/refs.h
@@ -51,15 +51,9 @@
 
 namespace vt {
 
-template <template <typename> class MsgPtrT, typename MsgT>
-void messageRef(MsgPtrT<MsgT> msg);
-
 template <typename MessageT>
 void messageRef(MessageT* msg);
 
-template <template <typename> class MsgPtrT, typename MsgT>
-void messageDeref(MsgPtrT<MsgT> msg);
-
 template <typename MessageT>
 void messageDeref(MessageT* msg);
 
diff --git a/src/vt/messaging/message/refs.impl.h b/src/vt/messaging/message/refs.impl.h
index 1c02d6fa23..7b1e58fee5 100644
--- a/src/vt/messaging/message/refs.impl.h
+++ b/src/vt/messaging/message/refs.impl.h
@@ -62,11 +62,6 @@ void messageRef(MessageT* msg) {
   );
 }
 
-template <template <typename> class MsgPtrT, typename MsgT>
-void messageRef(MsgPtrT<MsgT> msg) {
-  return messageRef(msg.get());
-}
-
 template <typename MessageT>
 void messageDeref(MessageT* msg) {
   envelopeDeref(msg->env);
@@ -93,11 +88,6 @@ void messageDeref(MessageT* msg) {
   }
 }
 
-template <template <typename> class MsgPtrT, typename MsgT>
-void messageDeref(MsgPtrT<MsgT> msg) {
-  return messageDeref(msg.get());
-}
-
 template <typename MessageT>
 bool isSharedMessage(MessageT* msg) {
   return envelopeGetRef(msg->env) != not_shared_message;
diff --git a/src/vt/messaging/message/smart_ptr.h b/src/vt/messaging/message/smart_ptr.h
index 4416df73c6..3c623db397 100644
--- a/src/vt/messaging/message/smart_ptr.h
+++ b/src/vt/messaging/message/smart_ptr.h
@@ -48,58 +48,51 @@
 #include "vt/config.h"
 #include "vt/messaging/message/message.h"
 #include "vt/messaging/message/refs.h"
-#include "vt/messaging/message/smart_ptr_virtual.h"
 
 #include <iosfwd>
 
 namespace vt { namespace messaging {
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-variable"
-static struct MsgInitOwnerType { } MsgInitOwnerTag { };
-static struct MsgInitNonOwnerType { } MsgInitNonOwnerTag { };
-#pragma GCC diagnostic pop
+struct MsgDerefBase {
+  // Invoke messageDeref on the appropriate type.
+  virtual void messageDeref(void* msg_ptr) = 0;
+  // Create an independent clone (that must also be deleted).
+  virtual MsgDerefBase* clone() = 0;
+  virtual ~MsgDerefBase() {}
+};
+
+template <typename MsgT>
+struct MsgDerefTyped : MsgDerefBase {
+  virtual void messageDeref(void* msg_ptr) {
+    vt::messageDeref(static_cast<MsgT*>(msg_ptr));
+  }
+  virtual MsgDerefBase* clone() {
+    return new MsgDerefTyped<MsgT>();
+  }
+};
 
 template <typename T>
 struct MsgSharedPtr final {
   using MsgType        = T;
-  using MsgPtrType     = MsgType*;
   using BaseMsgType    = BaseMessage;
-  using BaseMsgPtrType = BaseMsgType*;
-
-  MsgSharedPtr(MsgInitOwnerType, T* in)
-    : ptr_(in)
-  {
-    if (in) {
-      shared_ = isSharedMessage<T>(in);
-      if (shared_) {
-        vtAssertInfo(
-          envelopeGetRef(in->env) > 0, "owner: ref must be greater than 0",
-          shared_, envelopeGetRef(in->env)
-        );
-      }
-    }
+
+  MsgSharedPtr(std::nullptr_t) {}
+
+  MsgSharedPtr(T* in, bool takeRef) {
+    init(in, takeRef, nullptr);
   }
-  MsgSharedPtr(MsgInitNonOwnerType, T* in)
-    : ptr_(in)
-  {
-    if (in) {
-      shared_ = isSharedMessage<T>(in);
-      if (shared_) {
-        vtAssertInfo(
-          envelopeGetRef(in->env) > 0, "non-owner: ref must be greater than 0",
-          shared_, envelopeGetRef(in->env)
-        );
-        messageRef(in);
-      }
-    }
+
+  MsgSharedPtr(T* in, bool takeRef, MsgDerefBase* deref) {
+    init(in, takeRef, deref);
   }
-  MsgSharedPtr(std::nullptr_t) : ptr_(nullptr) { }
-  MsgSharedPtr(MsgSharedPtr<T> const& in) : ptr_(in.ptr_) { ref(); }
-  MsgSharedPtr(MsgSharedPtr<T>&& in)      : ptr_(in.ptr_) { ref(); }
 
-  template <typename U>
-  explicit MsgSharedPtr(MsgSharedPtr<U> const& in) : ptr_(in.get()) { ref(); }
+  MsgSharedPtr(MsgSharedPtr<T> const& in) {
+    init(in.get(), true, in.deref_ ? in.deref_->clone() : nullptr);
+  }
+
+  MsgSharedPtr(MsgSharedPtr<T>&& in) {
+    moveFrom(std::move(in));
+  }
 
   ~MsgSharedPtr() {
     clear();
@@ -107,76 +100,62 @@ struct MsgSharedPtr final {
 
   template <typename U>
   explicit operator MsgSharedPtr<U>() const { return to<U>(); }
-  explicit operator MsgPtrType() const { return get(); }
 
+  explicit operator T*() const { return get(); }
+
+  /// Access as another (usually base) message type.
   template <typename U>
   MsgSharedPtr<U> to() const {
-    static_assert(
-      std::is_trivially_destructible<T>(),
-      "Message shall not be downcast unless trivially destructible"
-    );
+    // Establish type-erased deref if needed.
+    // Non-shared message are not deref'ed and trivially destructible types
+    // do not really care on which type the delete-expr is invoked.
+    if (std::is_trivially_destructible<T>() or not shared_) {
+      vtAssert(not deref_, "Invalid state: cannot have typed deref");
+      return MsgSharedPtr<U>(
+        reinterpret_cast<U*>(ptr_), true);
+    }
 
-    return MsgSharedPtr<U>{*this};
+    return MsgSharedPtr<U>(
+      reinterpret_cast<U*>(ptr_), true,
+      deref_ ? deref_->clone() : new MsgDerefTyped<T>());
   }
 
+  // Obsolete. Use to() as MsgVirtualPtr <-> MsgSharedPtr.
   template <typename U>
-  MsgVirtualPtr<U> toVirtual() const {
-    return MsgVirtualPtr<U>(*this);
+  MsgSharedPtr<U> toVirtual() const {
+    return to<U>();
   }
 
-  MsgSharedPtr<T>& operator=(std::nullptr_t) { clear(); return *this; }
-  MsgSharedPtr<T>& operator=(MsgSharedPtr<T> const& n) { set(n); return *this; }
-  bool operator==(MsgSharedPtr<T> const& n) const { return ptr_ == n.ptr_; }
-  bool operator!=(MsgSharedPtr<T> const& n) const { return ptr_ != n.ptr_; }
-  bool operator==(std::nullptr_t) const           { return ptr_ == nullptr; }
-  bool operator!=(std::nullptr_t) const           { return ptr_ != nullptr; }
+  MsgSharedPtr<T>& operator=(std::nullptr_t) {
+    clear();
+    return *this;
+  }
 
-  inline bool valid() const { return ptr_ != nullptr; }
-  inline void set(MsgSharedPtr<T> const& n) {
+  MsgSharedPtr<T>& operator=(MsgSharedPtr<T> const& in) {
     clear();
-    if (n.valid()) {
-      shared_ = isSharedMessage<T>(n.get());
-      if (shared_) {
-        vtAssertInfo(
-          envelopeGetRef(n.get()->env) > 0, "Bad Ref (before ref set)",
-          shared_, envelopeGetRef(n.get()->env)
-        );
-        debug_print(
-          pool, node,
-          "MsgSmartPtr: (auto) set(), ptr={}, ref={}, address={}\n",
-          print_ptr(n.get()), envelopeGetRef(n.get()->env), print_ptr(this)
-        );
-        messageRef(n.get());
-      }
-      ptr_ = n.get();
-    }
+    init(in.get(), true, in.deref_ ? in.deref_->clone() : nullptr);
+    return *this;
   }
-  inline void ref() {
-    if (valid()) {
-      shared_ = isSharedMessage<T>(get());
-      if (shared_) {
-        vtAssertInfo(
-          envelopeGetRef(get()->env) > 0, "Bad Ref (before ref)",
-          shared_, envelopeGetRef(get()->env)
-        );
-        debug_print(
-          pool, node,
-          "MsgSmartPtr: (auto) ref(), ptr={}, ref={}, address={}\n",
-          print_ptr(get()), envelopeGetRef(get()->env), print_ptr(this)
-        );
-        messageRef(get());
-      }
-    }
+
+  MsgSharedPtr<T>& operator=(MsgSharedPtr<T>&& in) {
+    clear();
+    moveFrom(std::move(in));
+    return *this;
   }
 
+  bool operator==(MsgSharedPtr<T> const& n) const { return ptr_ == n.ptr_; }
+  bool operator!=(MsgSharedPtr<T> const& n) const { return ptr_ != n.ptr_; }
+  bool operator==(std::nullptr_t) const           { return ptr_ == nullptr; }
+  bool operator!=(std::nullptr_t) const           { return ptr_ != nullptr; }
+
   inline T* operator->() const { return get(); }
   inline T& operator*() const { return *ptr_; }
   inline T* get() const {
-    return ptr_ ? reinterpret_cast<MsgPtrType>(ptr_) : nullptr;
+    return ptr_ ? reinterpret_cast<T*>(ptr_) : nullptr;
   }
 
   friend std::ostream& operator<<(std::ostream&os, MsgSharedPtr<T> const& m) {
-    auto nrefs = m.valid() && m.shared_ ? envelopeGetRef(m.get()->env) : -1;
+    auto nrefs = m.ptr_ && m.shared_ ? envelopeGetRef(m.get()->env) : -1;
     return os << "MsgSharedPtr("
               <<              m.ptr_    << ","
               << "shared=" << m.shared_ << ","
@@ -184,61 +163,118 @@ struct MsgSharedPtr final {
               << ")";
   }
 
-protected:
-  inline void set(T* t) {
-    clear();
-    debug_print(
-      pool, node,
-      "MsgSmartPtr: (auto) set() BARE, ptr={}, ref={}, address={}\n",
-      print_ptr(t), envelopeGetRef(t->env), print_ptr(this)
-    );
-    setPtr(t);
-  }
+private:
+
+  // Performs state-ownership, optionally taking an additional message ref.
+  // Should probably be called every constructor; must ONLY be
+  // called from fresh/clear state. The deref object is fully owned.
+  void init(T* msgPtr, bool takeRef, MsgDerefBase* deref) {
+    vtAssert(msgPtr, "Invalid args: null msgPtr");
+    vtAssert(not ptr_, "Invalid state: already assigned");
+
+    ptr_ = msgPtr;
+    deref_ = deref;
+    bool shared = (shared_ = isSharedMessage<T>(msgPtr));
+
+    vtAssert(not deref_ or (deref_ and shared), "Invalid state: deref requires shared message");
 
-  inline void setPtr(T* t) {
-    ptr_ = reinterpret_cast<BaseMsgPtrType>(t);
+    if (shared) {
+      vtAssertInfo(
+        envelopeGetRef(msgPtr->env) > 0, "Bad Ref (before ref)",
+        shared, envelopeGetRef(msgPtr->env)
+      );
+      debug_print(
+        pool, node,
+        "MsgSmartPtr: (auto) init(), ptr={}, envRef={}, takeRef={} address={}\n",
+        print_ptr(msgPtr), envelopeGetRef(msgPtr->env), takeRef, print_ptr(this)
+      );
+
+      if (takeRef) {
+        messageRef(msgPtr);
+      }
+    }
   }
 
-  inline void clear() {
-    if (valid()) {
-      if (shared_) {
-        vtAssertInfo(
-          envelopeGetRef(get()->env) > 0, "Bad Ref (before deref)",
-          shared_, envelopeGetRef(get()->env)
-        );
-        debug_print(
-          pool, node,
-          "MsgSmartPtr: (auto) clear(), ptr={}, ref={}, address={}\n",
-          print_ptr(get()), envelopeGetRef(get()->env), print_ptr(this)
-        );
-        messageDeref(get());
+  void clear() {
+    bool shared = shared_;
+
+    vtAssert(not deref_ or (deref_ and shared), "Invalid state: deref requires shared message");
+
+    if (shared) {
+      T* msgPtr = get();
+      vtAssertInfo(
+        envelopeGetRef(msgPtr->env) > 0, "Bad Ref (before deref)",
+        shared, envelopeGetRef(msgPtr->env)
+      );
+      debug_print(
+        pool, node,
+        "MsgSmartPtr: (auto) clear(), ptr={}, envRef={}, address={}\n",
+        print_ptr(msgPtr), envelopeGetRef(msgPtr->env), print_ptr(this)
+      );
+
+      MsgDerefBase* deref = deref_;
+      if (deref) {
+        deref->messageDeref(msgPtr);
+        delete deref;
+      } else {
+        messageDeref<T>(msgPtr);
       }
+
+      shared_ = false;
+      deref_ = nullptr;
     }
+
     ptr_ = nullptr;
   }
 
+  /// Move. Must be invoked on fresh/clear state.
+  void moveFrom(MsgSharedPtr<T>&& in) {
+    ptr_ = in.ptr_;
+    shared_ = in.shared_;
+    deref_ = in.deref_;
+    // clean take - nullify other cleanup
+    in.ptr_ = nullptr;
+    in.shared_ = false;
+    in.deref_ = nullptr;
+  }
+
 private:
-  BaseMsgPtrType ptr_ = nullptr;
-  bool shared_        = true;
+  // Underlying raw message - access as correct type via get()
+  BaseMsgType* ptr_ = nullptr;
+  // Is this a shared message?
+  // If so, then it will have a deref done on delete.
+  bool shared_      = false;
+  // Type-erased type to invoke messageDeref on correct type.
+  // If set, use. Otherwise invoke messageDeref directly.
+  MsgDerefBase* deref_ = nullptr;
 };
 
 }} /* end namespace vt::messaging */
 
 namespace vt {
 
+// For historic reasons;
+// Functionality is now part of MsgSharedPtr
+template <typename T>
+using MsgVirtualPtr = messaging::MsgSharedPtr<T>;
+
+// For historic reasons;
+// Functionality is now part of MsgSharedPtr
+using MsgVirtualPtrAny = messaging::MsgSharedPtr<ShortMessage>;
+
 template <typename T>
 using MsgSharedPtr = messaging::MsgSharedPtr<T>;
 
 template <typename T>
 inline messaging::MsgSharedPtr<T> promoteMsgOwner(T* const msg) {
   msg->has_owner_ = true;
-  return MsgSharedPtr<T>{messaging::MsgInitOwnerTag,msg};
+  return MsgSharedPtr<T>{msg,false};
 }
 
 template <typename T>
 inline messaging::MsgSharedPtr<T> promoteMsg(MsgSharedPtr<T> msg) {
   vtAssert(msg->has_owner_, "promoteMsg shared ptr must have owner");
-  return MsgSharedPtr<T>{messaging::MsgInitNonOwnerTag,msg.get()};
+  return MsgSharedPtr<T>{msg.get(),true};
 }
 
 template <typename T>
@@ -246,7 +282,7 @@ inline messaging::MsgSharedPtr<T> promoteMsg(T* msg) {
   if (!msg->has_owner_) {
     return promoteMsgOwner(msg);
   } else {
-    return MsgSharedPtr<T>{messaging::MsgInitNonOwnerTag,msg};
+    return MsgSharedPtr<T>{msg,true};
   }
 }
 
diff --git a/src/vt/messaging/message/smart_ptr_virtual.h b/src/vt/messaging/message/smart_ptr_virtual.h
deleted file mode 100644
index 6717bd329a..0000000000
--- a/src/vt/messaging/message/smart_ptr_virtual.h
+++ /dev/null
@@ -1,104 +0,0 @@
-/*
-//@HEADER
-// *****************************************************************************
-//
-//                             smart_ptr_virtual.h
-//                           DARMA Toolkit v. 1.0.0
-//                       DARMA/vt => Virtual Transport
-//
-// Copyright 2019 National Technology & Engineering Solutions of Sandia, LLC
-// (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S.
-// Government retains certain rights in this software.
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are met:
-//
-// * Redistributions of source code must retain the above copyright notice,
-//   this list of conditions and the following disclaimer.
-//
-// * Redistributions in binary form must reproduce the above copyright notice,
-//   this list of conditions and the following disclaimer in the documentation
-//   and/or other materials provided with the distribution.
-//
-// * Neither the name of the copyright holder nor the names of its
-//   contributors may be used to endorse or promote products derived from this
-//   software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
-// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
-// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
-// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
-// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
-// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
-// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
-// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
-// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
-// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
-// POSSIBILITY OF SUCH DAMAGE.
-//
-// Questions? Contact darma@sandia.gov
-//
-// *****************************************************************************
-//@HEADER
-*/
-
-#if !defined INCLUDED_VT_MESSAGING_MESSAGE_SMART_PTR_VIRTUAL_H
-#define INCLUDED_VT_MESSAGING_MESSAGE_SMART_PTR_VIRTUAL_H
-
-#include "vt/config.h"
-#include "vt/messaging/message/message.h"
-
-namespace vt { namespace messaging {
-
-template <typename T>
-struct MsgSharedPtr;
-
-template <typename T>
-struct MsgVirtualPtr final {
-
-  template <typename U>
-  explicit MsgVirtualPtr(MsgSharedPtr<U> in_ptr)
-    : ptr_(MsgSharedPtr<T>(in_ptr)),
-      closure_([in_ptr]{ })
-  { }
-
-  template <typename U>
-  explicit MsgVirtualPtr(MsgVirtualPtr<U> in_vrt)
-    : ptr_(MsgSharedPtr<T>(in_vrt.ptr_)),
-      closure_(in_vrt.closure_)
-  { }
-
-  MsgVirtualPtr(std::nullptr_t) { }
-
-  inline void operator=(std::nullptr_t)
-  {
-    ptr_ = nullptr;
-    closure_ = nullptr;
-  }
-
-  inline T* get() const { return ptr_.get(); }
-  inline T* operator->() const { return ptr_.get(); }
-  inline T& operator*() const { return *ptr_.get(); }
-  inline bool operator==(std::nullptr_t) const { return ptr_ == nullptr; }
-  inline bool operator!=(std::nullptr_t) const { return ptr_ != nullptr; }
-
-private:
-  MsgSharedPtr<T> ptr_ = nullptr;
-  std::function<void()> closure_ = nullptr;
-
-  friend struct PendingSend;
-  MsgSharedPtr<T>& getShared() { return ptr_; }
-};
-
-}} /* end namespace vt::messaging */
-
-namespace vt {
-
-template <typename U>
-using MsgVirtualPtr = messaging::MsgVirtualPtr<U>;
-
-using MsgVirtualPtrAny = messaging::MsgVirtualPtr<ShortMessage>;
-
-} /* end namespace vt */
-
-#endif /*INCLUDED_VT_MESSAGING_MESSAGE_SMART_PTR_VIRTUAL_H*/
diff --git a/src/vt/messaging/pending_send.cc b/src/vt/messaging/pending_send.cc
index 5afb08faf6..1abc2f5908 100644
--- a/src/vt/messaging/pending_send.cc
+++ b/src/vt/messaging/pending_send.cc
@@ -49,7 +49,7 @@ namespace vt { namespace messaging {
 
 void PendingSend::sendMsg() {
   if (send_action_ == nullptr) {
-    theMsg()->sendMsgSized(msg_.getShared(), msg_size_);
+    theMsg()->sendMsgSized(msg_, msg_size_);
   } else {
     send_action_(msg_);
   }
diff --git a/src/vt/objgroup/manager.cc b/src/vt/objgroup/manager.cc
index 7d6f41ebf1..349b8e3133 100644
--- a/src/vt/objgroup/manager.cc
+++ b/src/vt/objgroup/manager.cc
@@ -48,7 +48,7 @@
 #include "vt/objgroup/proxy/proxy_bits.h"
 #include "vt/objgroup/type_registry/registry.h"
 #include "vt/context/context.h"
-#include "vt/messaging/message/smart_ptr_virtual.h"
+#include "vt/messaging/message/smart_ptr.h"
 
 namespace vt { namespace objgroup {
 
diff --git a/src/vt/objgroup/manager.fwd.h b/src/vt/objgroup/manager.fwd.h
index 903dc67a44..b4208396a3 100644
--- a/src/vt/objgroup/manager.fwd.h
+++ b/src/vt/objgroup/manager.fwd.h
@@ -47,7 +47,6 @@
 
 #include "vt/config.h"
 #include "vt/messaging/message/smart_ptr.h"
-#include "vt/messaging/message/smart_ptr_virtual.h"
 
 namespace vt { namespace objgroup {
 
diff --git a/src/vt/objgroup/manager.h b/src/vt/objgroup/manager.h
index b841a28c77..866678663d 100644
--- a/src/vt/objgroup/manager.h
+++ b/src/vt/objgroup/manager.h
@@ -54,7 +54,7 @@
 #include "vt/objgroup/holder/holder_basic.h"
 #include "vt/objgroup/dispatch/dispatch.h"
 #include "vt/messaging/message/message.h"
-#include "vt/messaging/message/smart_ptr_virtual.h"
+#include "vt/messaging/message/smart_ptr.h"
 
 #include <memory>
 #include <functional>

From 609e141708f67bd29ebc97e460ae6b93f9aa70f0 Mon Sep 17 00:00:00 2001
From: Paul Nathan Stickney <pstickne@gmail.com>
Date: Mon, 4 Nov 2019 23:19:47 -0800
Subject: [PATCH 2/7] #521: MsgSharedPtr uses static-lifetime deref

- Reduce new-delete (and associated complexity) through use
  of static template objects.

- Use 'assert' for developer-code asserts (critical flaws,
  not logic usages).
---
 src/vt/messaging/message/smart_ptr.h | 48 ++++++++++++++++++----------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/src/vt/messaging/message/smart_ptr.h b/src/vt/messaging/message/smart_ptr.h
index 3c623db397..9d8f170bd6 100644
--- a/src/vt/messaging/message/smart_ptr.h
+++ b/src/vt/messaging/message/smart_ptr.h
@@ -50,14 +50,28 @@
 #include "vt/messaging/message/refs.h"
 
 #include <iosfwd>
+#include <cassert>
+
+namespace vt { namespace messaging {
+  // Fwd decl for statics
+  template <typename T>
+  struct MsgDerefTyped;
+}} // end namespace vt::messaging
+
+namespace vt { namespace messaging { namespace statics {
+  /// Static message derefs for global lifetime and elimination
+  /// of allocation as they are stateless objects with expected
+  /// high reuse through the program.
+  template <typename T>
+  static MsgDerefTyped<T> StaticMsgDerefs;
+}}} // end namespace vt::messaging::statics
+
 
 namespace vt { namespace messaging {
 
 struct MsgDerefBase {
   // Invoke messageDeref on the appropriate type.
   virtual void messageDeref(void* msg_ptr) = 0;
-  // Create an independent clone (that must also be deleted).
-  virtual MsgDerefBase* clone() = 0;
   virtual ~MsgDerefBase() {}
 };
 
@@ -66,11 +80,9 @@ struct MsgDerefTyped : MsgDerefBase {
   virtual void messageDeref(void* msg_ptr) {
     vt::messageDeref(static_cast<MsgT*>(msg_ptr));
   }
-  virtual MsgDerefBase* clone() {
-    return new MsgDerefTyped<MsgT>();
-  }
 };
 
+
 template <typename T>
 struct MsgSharedPtr final {
   using MsgType        = T;
@@ -87,7 +99,7 @@ struct MsgSharedPtr final {
   }
 
   MsgSharedPtr(MsgSharedPtr<T> const& in) {
-    init(in.get(), true, in.deref_ ? in.deref_->clone() : nullptr);
+    init(in.get(), true, in.deref_);
   }
 
   MsgSharedPtr(MsgSharedPtr<T>&& in) {
@@ -110,14 +122,14 @@ struct MsgSharedPtr final {
     // Non-shared message are not deref'ed and trivially destructible types
     // do not really care on which type the delete-expr is invoked.
     if (std::is_trivially_destructible<T>() or not shared_) {
-      vtAssert(not deref_, "Invalid state: cannot have typed deref");
+      assert("should not have deref" && not deref_);
       return MsgSharedPtr<U>(
         reinterpret_cast<U*>(ptr_), true);
     }
 
     return MsgSharedPtr<U>(
       reinterpret_cast<U*>(ptr_), true,
-      deref_ ? deref_->clone() : new MsgDerefTyped<T>());
+      deref_ ? deref_ : &statics::StaticMsgDerefs<T>);
   }
 
   // Obsolete. Use to() as MsgVirtualPtr <-> MsgSharedPtr.
@@ -133,7 +145,7 @@ struct MsgSharedPtr final {
 
   MsgSharedPtr<T>& operator=(MsgSharedPtr<T> const& in) {
     clear();
-    init(in.get(), true, in.deref_ ? in.deref_->clone() : nullptr);
+    init(in.get(), true, in.deref_);
     return *this;
   }
 
@@ -167,16 +179,16 @@ struct MsgSharedPtr final {
 
   // Performs state-ownership, optionally taking an additional message ref.
   // Should probably be called every constructor; must ONLY be
-  // called from fresh/clear state. The deref object is fully owned.
+  // called from fresh/clear state.
   void init(T* msgPtr, bool takeRef, MsgDerefBase* deref) {
-    vtAssert(msgPtr, "Invalid args: null msgPtr");
-    vtAssert(not ptr_, "Invalid state: already assigned");
+    assert("given message" && msgPtr);
+    assert("not initialized" && not ptr_);
 
     ptr_ = msgPtr;
     deref_ = deref;
     bool shared = (shared_ = isSharedMessage<T>(msgPtr));
 
-    vtAssert(not deref_ or (deref_ and shared), "Invalid state: deref requires shared message");
+    assert("deref -> shared" && (not deref_ or (deref_ and shared)));
 
     if (shared) {
       vtAssertInfo(
@@ -198,10 +210,12 @@ struct MsgSharedPtr final {
   void clear() {
     bool shared = shared_;
 
-    vtAssert(not deref_ or (deref_ and shared), "Invalid state: deref requires shared message");
+    assert("deref -> shared" && (not deref_ or (deref_ and shared)));
 
     if (shared) {
+      assert("shared -> message ptr" && ptr_);
       T* msgPtr = get();
+
       vtAssertInfo(
         envelopeGetRef(msgPtr->env) > 0, "Bad Ref (before deref)",
         shared, envelopeGetRef(msgPtr->env)
@@ -215,7 +229,6 @@ struct MsgSharedPtr final {
       MsgDerefBase* deref = deref_;
       if (deref) {
         deref->messageDeref(msgPtr);
-        delete deref;
       } else {
         messageDeref<T>(msgPtr);
       }
@@ -244,8 +257,9 @@ struct MsgSharedPtr final {
   // Is this a shared message?
   // If so, then it will have a deref done on delete.
   bool shared_      = false;
-  // Type-erased type to invoke messageDeref on correct type.
-  // If set, use. Otherwise invoke messageDeref directly.
+  // Type-erased to invoke messageDeref on correct type for delete-expr.
+  // If set, use. Otherwise invoke messageDeref<T> directly.
+  // Object has a STATIC LIFETIME and should not be deleted.
   MsgDerefBase* deref_ = nullptr;
 };
 

From a8cfdaec1c303ee27003cb809c19de35e8490e80 Mon Sep 17 00:00:00 2001
From: Paul Nathan Stickney <pstickne@gmail.com>
Date: Mon, 4 Nov 2019 23:24:47 -0800
Subject: [PATCH 3/7] #521: move lifetime-operators close to lifetime-ctors

- Copy, move, and such.. should all look the same, as
  easier to visualize by proximity.

- Improve comments and such.
---
 src/vt/messaging/message/smart_ptr.h | 49 ++++++++++++++++------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/src/vt/messaging/message/smart_ptr.h b/src/vt/messaging/message/smart_ptr.h
index 9d8f170bd6..579b214e41 100644
--- a/src/vt/messaging/message/smart_ptr.h
+++ b/src/vt/messaging/message/smart_ptr.h
@@ -53,7 +53,7 @@
 #include <cassert>
 
 namespace vt { namespace messaging {
-  // Fwd decl for statics
+  // Fwd decl for statics.
   template <typename T>
   struct MsgDerefTyped;
 }} // end namespace vt::messaging
@@ -106,6 +106,23 @@ struct MsgSharedPtr final {
     moveFrom(std::move(in));
   }
 
+  MsgSharedPtr<T>& operator=(std::nullptr_t) {
+    clear();
+    return *this;
+  }
+
+  MsgSharedPtr<T>& operator=(MsgSharedPtr<T> const& in) {
+    clear();
+    init(in.get(), true, in.deref_);
+    return *this;
+  }
+
+  MsgSharedPtr<T>& operator=(MsgSharedPtr<T>&& in) {
+    clear();
+    moveFrom(std::move(in));
+    return *this;
+  }
+
   ~MsgSharedPtr() {
     clear();
   }
@@ -138,23 +155,6 @@ struct MsgSharedPtr final {
     return to<U>();
   }
 
-  MsgSharedPtr<T>& operator=(std::nullptr_t) {
-    clear();
-    return *this;
-  }
-
-  MsgSharedPtr<T>& operator=(MsgSharedPtr<T> const& in) {
-    clear();
-    init(in.get(), true, in.deref_);
-    return *this;
-  }
-
-  MsgSharedPtr<T>& operator=(MsgSharedPtr<T>&& in) {
-    clear();
-    moveFrom(std::move(in));
-    return *this;
-  }
-
   bool operator==(MsgSharedPtr<T> const& n) const { return ptr_ == n.ptr_; }
   bool operator!=(MsgSharedPtr<T> const& n) const { return ptr_ != n.ptr_; }
   bool operator==(std::nullptr_t) const           { return ptr_ == nullptr; }
@@ -279,20 +279,27 @@ using MsgVirtualPtrAny = messaging::MsgSharedPtr<ShortMessage>;
 template <typename T>
 using MsgSharedPtr = messaging::MsgSharedPtr<T>;
 
+/// Steal ownership of the message (no ref-increase).
 template <typename T>
-inline messaging::MsgSharedPtr<T> promoteMsgOwner(T* const msg) {
+inline MsgSharedPtr<T> promoteMsgOwner(T* const msg) {
   msg->has_owner_ = true;
   return MsgSharedPtr<T>{msg,false};
 }
 
+/// Take additional ownership of the message (increase message ref).
+/// The message must already have an owner.
+// TODO: eliminate if possible as it has confusing semantics
+// with overload and is duplicated by copy/move ctors.
 template <typename T>
-inline messaging::MsgSharedPtr<T> promoteMsg(MsgSharedPtr<T> msg) {
+inline MsgSharedPtr<T> promoteMsg(MsgSharedPtr<T> msg) {
   vtAssert(msg->has_owner_, "promoteMsg shared ptr must have owner");
   return MsgSharedPtr<T>{msg.get(),true};
 }
 
+/// If the message does not have an owner, steal ownership (no ref-increase).
+/// Otherwise, take additional ownership (increate message ref).
 template <typename T>
-inline messaging::MsgSharedPtr<T> promoteMsg(T* msg) {
+inline MsgSharedPtr<T> promoteMsg(T* msg) {
   if (!msg->has_owner_) {
     return promoteMsgOwner(msg);
   } else {

From 8dcf0350a1ad4ef572c3060d2a7529e7de3f7c0e Mon Sep 17 00:00:00 2001
From: Paul Nathan Stickney <pstickne@gmail.com>
Date: Tue, 5 Nov 2019 01:21:48 -0800
Subject: [PATCH 4/7] #521: minorish consistency updates

- Envelop deref returns ref value (post-deref).

- Ordering, comments, type names, eg.
---
 src/vt/messaging/envelope/envelope_ref.h      |  2 +-
 src/vt/messaging/envelope/envelope_ref.impl.h |  6 +--
 src/vt/messaging/message/refs.h               | 12 +++---
 src/vt/messaging/message/refs.impl.h          | 18 ++++----
 src/vt/messaging/message/shared_message.h     | 27 ++++++------
 .../messaging/message/shared_message.impl.h   | 43 ++++++++++---------
 6 files changed, 55 insertions(+), 53 deletions(-)

diff --git a/src/vt/messaging/envelope/envelope_ref.h b/src/vt/messaging/envelope/envelope_ref.h
index 889f6101ed..457e44e2f8 100644
--- a/src/vt/messaging/envelope/envelope_ref.h
+++ b/src/vt/messaging/envelope/envelope_ref.h
@@ -57,7 +57,7 @@ template <typename Env>
 inline void envelopeRef(Env& env);
 
 template <typename Env>
-inline void envelopeDeref(Env& env);
+inline RefType envelopeDeref(Env& env);
 
 } /* end namespace vt */
 
diff --git a/src/vt/messaging/envelope/envelope_ref.impl.h b/src/vt/messaging/envelope/envelope_ref.impl.h
index 24465e02db..244d41e07c 100644
--- a/src/vt/messaging/envelope/envelope_ref.impl.h
+++ b/src/vt/messaging/envelope/envelope_ref.impl.h
@@ -52,12 +52,12 @@ namespace vt {
 
 template <typename Env>
 inline void envelopeRef(Env& env) {
-  reinterpret_cast<Envelope*>(&env)->ref++;
+  (reinterpret_cast<Envelope*>(&env))->ref++;
 }
 
 template <typename Env>
-inline void envelopeDeref(Env& env) {
-  reinterpret_cast<Envelope*>(&env)->ref--;
+inline RefType envelopeDeref(Env& env) {
+  return --(reinterpret_cast<Envelope*>(&env))->ref;
 }
 
 } /* end namespace vt */
diff --git a/src/vt/messaging/message/refs.h b/src/vt/messaging/message/refs.h
index 26d70383dd..e2938276bf 100644
--- a/src/vt/messaging/message/refs.h
+++ b/src/vt/messaging/message/refs.h
@@ -51,14 +51,14 @@
 
 namespace vt {
 
-template <typename MessageT>
-void messageRef(MessageT* msg);
+template <typename MsgT>
+void messageRef(MsgT* msg);
 
-template <typename MessageT>
-void messageDeref(MessageT* msg);
+template <typename MsgT>
+void messageDeref(MsgT* msg);
 
-template <typename MessageT>
-bool isSharedMessage(MessageT* msg);
+template <typename MsgT>
+bool isSharedMessage(MsgT* msg);
 
 } /* end namespace vt */
 
diff --git a/src/vt/messaging/message/refs.impl.h b/src/vt/messaging/message/refs.impl.h
index 7b1e58fee5..835a7229e7 100644
--- a/src/vt/messaging/message/refs.impl.h
+++ b/src/vt/messaging/message/refs.impl.h
@@ -51,8 +51,8 @@
 
 namespace vt {
 
-template <typename MessageT>
-void messageRef(MessageT* msg) {
+template <typename MsgT>
+void messageRef(MsgT* msg) {
   envelopeRef(msg->env);
 
   debug_print(
@@ -62,17 +62,17 @@ void messageRef(MessageT* msg) {
   );
 }
 
-template <typename MessageT>
-void messageDeref(MessageT* msg) {
-  envelopeDeref(msg->env);
+template <typename MsgT>
+void messageDeref(MsgT* msg) {
+  RefType refcount = envelopeDeref(msg->env);
 
   debug_print(
     pool, node,
     "messageDeref msg={}, refs={}\n",
-    print_ptr(msg), envelopeGetRef(msg->env)
+    print_ptr(msg), refcount
   );
 
-  if (envelopeGetRef(msg->env) == 0) {
+  if (refcount == 0) {
     /* @todo: what is the correct strategy here? invoking dealloc does not
      * invoke the destructor
      *
@@ -88,8 +88,8 @@ void messageDeref(MessageT* msg) {
   }
 }
 
-template <typename MessageT>
-bool isSharedMessage(MessageT* msg) {
+template <typename MsgT>
+bool isSharedMessage(MsgT* msg) {
   return envelopeGetRef(msg->env) != not_shared_message;
 }
 
diff --git a/src/vt/messaging/message/shared_message.h b/src/vt/messaging/message/shared_message.h
index b8ec853757..f6455ef767 100644
--- a/src/vt/messaging/message/shared_message.h
+++ b/src/vt/messaging/message/shared_message.h
@@ -52,29 +52,30 @@
 
 namespace vt {
 
-template <typename MessageT, typename... Args>
-MessageT* makeSharedMessage(Args&&... args);
-
 template <typename MsgT, typename... Args>
-MsgSharedPtr<MsgT> makeMessage(Args&&... args);
+MsgT* makeSharedMessage(Args&&... args);
 
 template <typename MsgT, typename... Args>
-MsgSharedPtr<MsgT> makeMsg(Args&&... args);
+MsgT* makeSharedMessageSz(std::size_t size, Args&&... args);
 
-template <typename MessageT, typename... Args>
-MessageT* makeSharedMessageSz(std::size_t size, Args&&... args);
+template <typename MsgT, typename... Args>
+MsgSharedPtr<MsgT> makeMessage(Args&&... args);
 
 template <typename MsgT, typename... Args>
 MsgSharedPtr<MsgT> makeMessageSz(std::size_t size, Args&&... args);
 
-template <typename MessageT>
-void messageConvertToShared(MessageT* msg);
+///[obsolete] Use makeMessage instead.
+template <typename MsgT, typename... Args>
+MsgSharedPtr<MsgT> makeMsg(Args&&... args);
+
+template <typename MsgT>
+void messageConvertToShared(MsgT* msg);
 
-template <typename MessageT>
-void messageSetUnmanaged(MessageT* msg);
+template <typename MsgT>
+void messageSetUnmanaged(MsgT* msg);
 
-template <typename MsgPtrT>
-void messageResetDeserdes(MsgPtrT const& msg);
+template <typename MsgT>
+void messageResetDeserdes(MsgSharedPtr<MsgT> const& msg);
 
 } //end namespace vt
 
diff --git a/src/vt/messaging/message/shared_message.impl.h b/src/vt/messaging/message/shared_message.impl.h
index 5cb457d18d..488f97b05f 100644
--- a/src/vt/messaging/message/shared_message.impl.h
+++ b/src/vt/messaging/message/shared_message.impl.h
@@ -69,7 +69,15 @@ void messageTypeChecks() {
 template <typename MsgT, typename... Args>
 MsgT* makeSharedMessage(Args&&... args) {
   messageTypeChecks<MsgT>();
-  auto msg = new MsgT{std::forward<Args>(args)...};
+  MsgT* msg = new MsgT{std::forward<Args>(args)...};
+  envelopeSetRef(msg->env, 1);
+  msg->has_owner_ = false;
+  return msg;
+}
+
+template <typename MsgT, typename... Args>
+MsgT* makeSharedMessageSz(std::size_t size, Args&&... args) {
+  MsgT* msg = new (size) MsgT{std::forward<Args>(args)...};
   envelopeSetRef(msg->env, 1);
   msg->has_owner_ = false;
   return msg;
@@ -78,41 +86,34 @@ MsgT* makeSharedMessage(Args&&... args) {
 template <typename MsgT, typename... Args>
 MsgSharedPtr<MsgT> makeMessage(Args&&... args) {
   messageTypeChecks<MsgT>();
-  auto msg = makeSharedMessage<MsgT>(std::forward<Args>(args)...);
+  MsgT* msg = makeSharedMessage<MsgT>(std::forward<Args>(args)...);
   return promoteMsgOwner<MsgT>(msg);
 }
 
 template <typename MsgT, typename... Args>
-MsgSharedPtr<MsgT> makeMsg(Args&&... args) {
-  return makeMessage<MsgT>(std::forward<Args>(args)...);
-}
-
-template <typename MessageT, typename... Args>
-MessageT* makeSharedMessageSz(std::size_t size, Args&&... args) {
-  MessageT* msg = new (size) MessageT{std::forward<Args>(args)...};
-  envelopeSetRef(msg->env, 1);
-  msg->has_owner_ = false;
-  return msg;
+MsgSharedPtr<MsgT> makeMessageSz(std::size_t size, Args&&... args) {
+  MsgT* msg = makeSharedMessageSz<MsgT>(size,std::forward<Args>(args)...);
+  return promoteMsgOwner<MsgT>(msg);
 }
 
+///[obsolete] Use makeMessage instead.
 template <typename MsgT, typename... Args>
-MsgSharedPtr<MsgT> makeMessageSz(std::size_t size, Args&&... args) {
-  auto msg = makeSharedMessageSz<MsgT>(size,std::forward<Args>(args)...);
-  return promoteMsgOwner<MsgT>(msg);
+MsgSharedPtr<MsgT> makeMsg(Args&&... args) {
+  return makeMessage<MsgT>(std::forward<Args>(args)...);
 }
 
-template <typename MessageT>
-void messageConvertToShared(MessageT* msg) {
+template <typename MsgT>
+void messageConvertToShared(MsgT* msg) {
   envelopeSetRef(msg->env, 1);
 }
 
-template <typename MessageT>
-void messageSetUnmanaged(MessageT* msg) {
+template <typename MsgT>
+void messageSetUnmanaged(MsgT* msg) {
   envelopeSetRef(msg->env, not_shared_message);
 }
 
-template <typename MsgPtrT>
-void messageResetDeserdes(MsgPtrT const& msg) {
+template <typename MsgT>
+void messageResetDeserdes(MsgSharedPtr<MsgT> const& msg) {
   envelopeSetRef(msg->env, 1);
   msg->has_owner_ = true;
 }

From 67b6eaeb4a137a2155fbea284285ae9537ff5a9e Mon Sep 17 00:00:00 2001
From: Paul Nathan Stickney <pstickne@gmail.com>
Date: Sat, 23 Nov 2019 10:59:27 -0800
Subject: [PATCH 5/7] #521: simplify and unify type-erased SharedMsgPtr impl

- Instead of 'sometimes using' an implementation, which
  involves several scattered conditional checks, the implementation
  is always used. Then "to" or "toVirtual" function as a virtue
  of propogating the object for the original Message type.

  Objects all exist with a static lifetime so there is effectively
  no additional cost (beyond the potential for a few more [cheap] template
  instantiations in cases where `to` or `toVirtual` where never
  invoked upon the original MsgT).

 - Renamed types and variables to reflect the larger scope;
   while messageDeref is the only current method, the implementation
   can be expanded (even more of a pimpl..) without name violation.
---
 src/vt/messaging/message/smart_ptr.h | 83 ++++++++++++----------------
 1 file changed, 35 insertions(+), 48 deletions(-)

diff --git a/src/vt/messaging/message/smart_ptr.h b/src/vt/messaging/message/smart_ptr.h
index 579b214e41..7353f888ec 100644
--- a/src/vt/messaging/message/smart_ptr.h
+++ b/src/vt/messaging/message/smart_ptr.h
@@ -55,29 +55,31 @@
 namespace vt { namespace messaging {
   // Fwd decl for statics.
   template <typename T>
-  struct MsgDerefTyped;
+  struct MsgPtrImplTyped;
 }} // end namespace vt::messaging
 
 namespace vt { namespace messaging { namespace statics {
-  /// Static message derefs for global lifetime and elimination
-  /// of allocation as they are stateless objects with expected
-  /// high reuse through the program.
+  /// Static objects with global lifetime and elimination of allocation.
+  /// They are stateless and have a very high/constant reuse pattern.
   template <typename T>
-  static MsgDerefTyped<T> StaticMsgDerefs;
+  static MsgPtrImplTyped<T> TypedMsgPtrImpls;
 }}} // end namespace vt::messaging::statics
 
 
 namespace vt { namespace messaging {
 
-struct MsgDerefBase {
-  // Invoke messageDeref on the appropriate type.
+/// Message-type agnostic virtual base class.
+struct MsgPtrImplBase {
+  /// Invoke messageDeref on the appropriate type.
+  /// (Ensure a valid delete-expression on virtual message types.)
   virtual void messageDeref(void* msg_ptr) = 0;
-  virtual ~MsgDerefBase() {}
+  virtual ~MsgPtrImplBase() {}
 };
 
 template <typename MsgT>
-struct MsgDerefTyped : MsgDerefBase {
+struct MsgPtrImplTyped : MsgPtrImplBase {
   virtual void messageDeref(void* msg_ptr) {
+    // N.B. messageDeref<T> invokes delete-expr T.
     vt::messageDeref(static_cast<MsgT*>(msg_ptr));
   }
 };
@@ -91,15 +93,16 @@ struct MsgSharedPtr final {
   MsgSharedPtr(std::nullptr_t) {}
 
   MsgSharedPtr(T* in, bool takeRef) {
-    init(in, takeRef, nullptr);
+    init(in, takeRef, &statics::TypedMsgPtrImpls<T>);
   }
 
-  MsgSharedPtr(T* in, bool takeRef, MsgDerefBase* deref) {
-    init(in, takeRef, deref);
+  // Overload to retain ORIGINAL type-erased implementation.
+  MsgSharedPtr(T* in, bool takeRef, MsgPtrImplBase* impl) {
+    init(in, takeRef, impl);
   }
 
   MsgSharedPtr(MsgSharedPtr<T> const& in) {
-    init(in.get(), true, in.deref_);
+    init(in.get(), true, in.impl_);
   }
 
   MsgSharedPtr(MsgSharedPtr<T>&& in) {
@@ -113,7 +116,7 @@ struct MsgSharedPtr final {
 
   MsgSharedPtr<T>& operator=(MsgSharedPtr<T> const& in) {
     clear();
-    init(in.get(), true, in.deref_);
+    init(in.get(), true, in.impl_);
     return *this;
   }
 
@@ -135,21 +138,13 @@ struct MsgSharedPtr final {
   /// Access as another (usually base) message type.
   template <typename U>
   MsgSharedPtr<U> to() const {
-    // Establish type-erased deref if needed.
-    // Non-shared message are not deref'ed and trivially destructible types
-    // do not really care on which type the delete-expr is invoked.
-    if (std::is_trivially_destructible<T>() or not shared_) {
-      assert("should not have deref" && not deref_);
-      return MsgSharedPtr<U>(
-        reinterpret_cast<U*>(ptr_), true);
-    }
-
     return MsgSharedPtr<U>(
       reinterpret_cast<U*>(ptr_), true,
-      deref_ ? deref_ : &statics::StaticMsgDerefs<T>);
+      /*N.B. retain ORIGINAL-type implementation*/ impl_);
   }
 
-  // Obsolete. Use to() as MsgVirtualPtr <-> MsgSharedPtr.
+  /// [obsolete] Use to() as MsgVirtualPtr <-> MsgSharedPtr.
+  /// Both methods are equivalent in funciton.
   template <typename U>
   MsgSharedPtr<U> toVirtual() const {
     return to<U>();
@@ -177,19 +172,18 @@ struct MsgSharedPtr final {
 
 private:
 
-  // Performs state-ownership, optionally taking an additional message ref.
-  // Should probably be called every constructor; must ONLY be
-  // called from fresh/clear state.
-  void init(T* msgPtr, bool takeRef, MsgDerefBase* deref) {
+  /// Performs state-ownership, optionally taking an additional message ref.
+  /// Should probably be called every constructor; must ONLY be
+  /// called from fresh (zero-init member) or clear() state.
+  void init(T* msgPtr, bool takeRef, MsgPtrImplBase* impl) {
     assert("given message" && msgPtr);
     assert("not initialized" && not ptr_);
+    assert("given impl" && impl);
 
     ptr_ = msgPtr;
-    deref_ = deref;
+    impl_ = impl;
     bool shared = (shared_ = isSharedMessage<T>(msgPtr));
 
-    assert("deref -> shared" && (not deref_ or (deref_ and shared)));
-
     if (shared) {
       vtAssertInfo(
         envelopeGetRef(msgPtr->env) > 0, "Bad Ref (before ref)",
@@ -202,16 +196,17 @@ struct MsgSharedPtr final {
       );
 
       if (takeRef) {
+        // Could be moved to type-erased impl..
         messageRef(msgPtr);
       }
     }
   }
 
+  /// Clear all internal state. Effectively destructor in operation,
+  /// with guarantee that init() can be used again after.
   void clear() {
     bool shared = shared_;
 
-    assert("deref -> shared" && (not deref_ or (deref_ and shared)));
-
     if (shared) {
       assert("shared -> message ptr" && ptr_);
       T* msgPtr = get();
@@ -226,15 +221,9 @@ struct MsgSharedPtr final {
         print_ptr(msgPtr), envelopeGetRef(msgPtr->env), print_ptr(this)
       );
 
-      MsgDerefBase* deref = deref_;
-      if (deref) {
-        deref->messageDeref(msgPtr);
-      } else {
-        messageDeref<T>(msgPtr);
-      }
+      impl_->messageDeref(msgPtr);
 
       shared_ = false;
-      deref_ = nullptr;
     }
 
     ptr_ = nullptr;
@@ -244,11 +233,10 @@ struct MsgSharedPtr final {
   void moveFrom(MsgSharedPtr<T>&& in) {
     ptr_ = in.ptr_;
     shared_ = in.shared_;
-    deref_ = in.deref_;
-    // clean take - nullify other cleanup
+    impl_ = in.impl_;
+    // clean take - nullify/prevent other cleanup
     in.ptr_ = nullptr;
     in.shared_ = false;
-    in.deref_ = nullptr;
   }
 
 private:
@@ -257,10 +245,9 @@ struct MsgSharedPtr final {
   // Is this a shared message?
   // If so, then it will have a deref done on delete.
   bool shared_      = false;
-  // Type-erased to invoke messageDeref on correct type for delete-expr.
-  // If set, use. Otherwise invoke messageDeref<T> directly.
-  // Object has a STATIC LIFETIME and should not be deleted.
-  MsgDerefBase* deref_ = nullptr;
+  // Type-erased implementation support.
+  // Object has a STATIC LIFETIME / is not owned / should not be deleted.
+  MsgPtrImplBase* impl_ = nullptr;
 };
 
 }} /* end namespace vt::messaging */

From 39baac13b5201684a945e14458963afd7a369f7c Mon Sep 17 00:00:00 2001
From: Jonathan Lifflander <jliffla@sandia.gov>
Date: Fri, 29 Nov 2019 22:52:55 -0800
Subject: [PATCH 6/7] #521: msg: fix internal linkage -> global templated
 static

---
 src/vt/messaging/message/smart_ptr.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/vt/messaging/message/smart_ptr.h b/src/vt/messaging/message/smart_ptr.h
index 7353f888ec..8deac8af73 100644
--- a/src/vt/messaging/message/smart_ptr.h
+++ b/src/vt/messaging/message/smart_ptr.h
@@ -61,8 +61,13 @@ namespace vt { namespace messaging {
 namespace vt { namespace messaging { namespace statics {
   /// Static objects with global lifetime and elimination of allocation.
   /// They are stateless and have a very high/constant reuse pattern.
+  struct Holder {
+    template <typename T>
+    static MsgPtrImplTyped<T> TypedMsgPtrImpls;
+  };
+
   template <typename T>
-  static MsgPtrImplTyped<T> TypedMsgPtrImpls;
+  /*static*/ MsgPtrImplTyped<T> Holder::TypedMsgPtrImpls;
 }}} // end namespace vt::messaging::statics
 
 
@@ -93,7 +98,7 @@ struct MsgSharedPtr final {
   MsgSharedPtr(std::nullptr_t) {}
 
   MsgSharedPtr(T* in, bool takeRef) {
-    init(in, takeRef, &statics::TypedMsgPtrImpls<T>);
+    init(in, takeRef, &statics::Holder::TypedMsgPtrImpls<T>);
   }
 
   // Overload to retain ORIGINAL type-erased implementation.

From 39ac08e6014546eeabf81c98aa4d1ff96a81ad89 Mon Sep 17 00:00:00 2001
From: Jonathan Lifflander <jliffla@sandia.gov>
Date: Fri, 29 Nov 2019 22:53:17 -0800
Subject: [PATCH 7/7] #521: config: fix bug forcing memory pool on

---
 src/vt/configs/features/features_defines.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vt/configs/features/features_defines.h b/src/vt/configs/features/features_defines.h
index b3a6c3d78e..5a11b971f6 100644
--- a/src/vt/configs/features/features_defines.h
+++ b/src/vt/configs/features/features_defines.h
@@ -63,7 +63,7 @@
 #define vt_feature_print_term_msgs    0 || vt_feature_cmake_print_term_msgs
 #define vt_feature_default_threading  0 || vt_feature_cmake_default_threading
 #define vt_feature_no_pool_alloc_env  0 || vt_feature_cmake_no_pool_alloc_env
-#define vt_feature_memory_pool        1 || vt_feature_cmake_memory_pool
+#define vt_feature_memory_pool        0 || vt_feature_cmake_memory_pool
 #define vt_feature_priorities         0 || vt_feature_cmake_priorities
 #define vt_feature_cons_multi_idx     0 || vt_feature_cmake_cons_multi_idx
 #define vt_feature_fcontext           0 || vt_feature_cmake_fcontext