Replace boost::ptr_map<name, etc> with std::map<name, shared_ptr>.

On Windows, unlike on Mac or Linux, boost::ptr_map<> started insisting on this
concept of clonability. In other words, it wants to own a unique instance of
the pointee; if you copy a value_type -- even to dereference an iterator! --
it wants to construct a whole new instance of the mapped_type. That's nuts. A
std::map<..., boost::shared_ptr<>> has the property I want (the mapped_type
goes away when the entry is erased), plus it's willing to pass around the
shared_ptr to the same instance of the mapped_type. This change also permits
simplifying a couple awkward kludges I'd already had to make to accommodate
ptr_map's idiosyncracies.

2 files changed, 23 insertions, 26 deletions

diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp
index e00cca366f..52660105c4 100644
--- a/indra/llcommon/lleventdispatcher.cpp
+++ b/indra/llcommon/lleventdispatcher.cpp
@@ -524,9 +524,9 @@ void LLEventDispatcher::addArrayParamsDispatchEntry(const std::string& name,
                                                     const invoker_function& invoker,
                                                     LLSD::Integer arity)
 {
-    // Peculiar to me that boost::ptr_map() accepts std::auto_ptr but not dumb ptr
-    mDispatch.insert(name, std::auto_ptr<DispatchEntry>(
-                         new ArrayParamsDispatchEntry(desc, invoker, arity)));
+    mDispatch.insert(
+        DispatchMap::value_type(name, DispatchMap::mapped_type(
+                                    new ArrayParamsDispatchEntry(desc, invoker, arity))));
 }
 
 void LLEventDispatcher::addMapParamsDispatchEntry(const std::string& name,
@@ -535,16 +535,18 @@ void LLEventDispatcher::addMapParamsDispatchEntry(const std::string& name,
                                                   const LLSD& params,
                                                   const LLSD& defaults)
 {
-    mDispatch.insert(name, std::auto_ptr<DispatchEntry>(
-                         new MapParamsDispatchEntry(name, desc, invoker, params, defaults)));
+    mDispatch.insert(
+        DispatchMap::value_type(name, DispatchMap::mapped_type(
+                                    new MapParamsDispatchEntry(name, desc, invoker, params, defaults))));
 }
 
 /// Register a callable by name
 void LLEventDispatcher::add(const std::string& name, const std::string& desc,
                             const Callable& callable, const LLSD& required)
 {
-    mDispatch.insert(name, std::auto_ptr<DispatchEntry>(
-                         new LLSDDispatchEntry(desc, callable, required)));
+    mDispatch.insert(
+        DispatchMap::value_type(name, DispatchMap::mapped_type(
+                                    new LLSDDispatchEntry(desc, callable, required))));
 }
 
 void LLEventDispatcher::addFail(const std::string& name, const std::string& classname) const
diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h
index b37189b58b..7acc61de4e 100644
--- a/indra/llcommon/lleventdispatcher.h
+++ b/indra/llcommon/lleventdispatcher.h
@@ -57,7 +57,7 @@ static const int& nil(nil_);
 #endif
 
 #include <string>
-#include <boost/ptr_container/ptr_map.hpp>
+#include <boost/shared_ptr.hpp>
 #include <boost/function.hpp>
 #include <boost/bind.hpp>
 #include <boost/iterator/transform_iterator.hpp>
@@ -286,40 +286,35 @@ private:
     struct DispatchEntry
     {
         DispatchEntry(const std::string& desc);
+        virtual ~DispatchEntry() {} // suppress MSVC warning, sigh
 
         std::string mDesc;
 
         virtual void call(const std::string& desc, const LLSD& event) const = 0;
         virtual LLSD addMetadata(LLSD) const = 0;
     };
-    typedef boost::ptr_map<std::string, DispatchEntry> DispatchMap;
+    // Tried using boost::ptr_map<std::string, DispatchEntry>, but ptr_map<>
+    // wants its value type to be "clonable," even just to dereference an
+    // iterator. I don't want to clone entries -- if I have to copy an entry
+    // around, I want it to continue pointing to the same DispatchEntry
+    // subclass object. However, I definitely want DispatchMap to destroy
+    // DispatchEntry if no references are outstanding at the time an entry is
+    // removed. This looks like a job for boost::shared_ptr.
+    typedef std::map<std::string, boost::shared_ptr<DispatchEntry> > DispatchMap;
 
 public:
     /// We want the flexibility to redefine what data we store per name,
     /// therefore our public interface doesn't expose DispatchMap iterators,
     /// or DispatchMap itself, or DispatchEntry. Instead we explicitly
     /// transform each DispatchMap item to NameDesc on dereferencing.
-    typedef boost::transform_iterator<NameDesc(*)(DispatchMap::value_type), DispatchMap::iterator> const_iterator;
+    typedef boost::transform_iterator<NameDesc(*)(const DispatchMap::value_type&), DispatchMap::const_iterator> const_iterator;
     const_iterator begin() const
     {
-        // Originally we used DispatchMap::const_iterator, which Just Worked
-        // when DispatchMap was a std::map. Now that it's a boost::ptr_map,
-        // using DispatchMap::const_iterator doesn't work so well: it
-        // dereferences to a pair<string, const T*>, whereas
-        // DispatchMap::value_type is just pair<string, T*>. Trying to pass a
-        // dereferenced iterator to the value_type didn't work because the
-        // compiler won't let you convert from const T* to plain T*. Changing
-        // our const_iterator definition above to be based on non-const
-        // DispatchMap::iterator works better, but of course we have to cast
-        // away the constness of mDispatch to use non-const iterator. (Sigh.)
-        return boost::make_transform_iterator(const_cast<DispatchMap&>(mDispatch).begin(),
-                                              makeNameDesc);
+        return boost::make_transform_iterator(mDispatch.begin(), makeNameDesc);
     }
     const_iterator end() const
     {
-        // see begin() comments
-        return boost::make_transform_iterator(const_cast<DispatchMap&>(mDispatch).end(),
-                                              makeNameDesc);
+        return boost::make_transform_iterator(mDispatch.end(), makeNameDesc);
     }
     //@}
 
@@ -349,7 +344,7 @@ private:
     std::string mDesc, mKey;
     DispatchMap mDispatch;
 
-    static NameDesc makeNameDesc(DispatchMap::value_type item)
+    static NameDesc makeNameDesc(const DispatchMap::value_type& item)
     {
         return NameDesc(item.first, item.second->mDesc);
     }