From a9650ba22219d91438d91fd9371966f510884cbf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 29 May 2015 15:10:54 -0400 Subject: MAINT-5232: Per Vir review, use Boost.Signals2 for LLPounceable. Vir points out that "queue of callables" is pretty much exactly what a signal does. Add unit tests to verify chronological order, also queue reset when fired. --- indra/llcommon/llpounceable.h | 43 ++++++++++++++---------------- indra/llcommon/tests/llpounceable_test.cpp | 30 +++++++++++++++++++++ 2 files changed, 50 insertions(+), 23 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/llpounceable.h b/indra/llcommon/llpounceable.h index 94d508d917..9c8aba2154 100644 --- a/indra/llcommon/llpounceable.h +++ b/indra/llcommon/llpounceable.h @@ -40,8 +40,7 @@ #include #include #include -#include -#include +#include // Forward declare the user template, since we want to be able to point to it // in some of its implementation classes. @@ -51,10 +50,10 @@ class LLPounceable; template struct LLPounceableTraits { + // Our "queue" is a signal object with correct signature. + typedef boost::signals2::signal::param_type)> signal_t; // Call callWhenReady() with any callable accepting T. - typedef boost::function::param_type)> func_t; - // Our actual queue is a simple queue of such callables. - typedef std::queue queue_t; + typedef typename signal_t::slot_type func_t; // owner pointer type typedef LLPounceable* owner_ptr; }; @@ -79,17 +78,17 @@ class LLPounceableQueueSingleton: private: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; // For a given held type T, every LLPounceable // instance will call on the SAME LLPounceableQueueSingleton instance -- // given how class statics work. We must keep a separate queue for each // LLPounceable instance. Use a hash map for that. - typedef boost::unordered_map map_t; + typedef boost::unordered_map map_t; public: // Disambiguate queues belonging to different LLPounceables. - queue_t& get(owner_ptr owner) + signal_t& get(owner_ptr owner) { // operator[] has find-or-create semantics -- just what we want! return mMap[owner]; @@ -106,9 +105,9 @@ class LLPounceableQueueImpl public: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; - queue_t& get(owner_ptr owner) const + signal_t& get(owner_ptr owner) const { // this Impl contains nothing; it delegates to the Singleton return LLPounceableQueueSingleton::instance().get(owner); @@ -124,15 +123,15 @@ class LLPounceableQueueImpl public: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; - queue_t& get(owner_ptr) + signal_t& get(owner_ptr) { return mQueue; } private: - queue_t mQueue; + signal_t mQueue; }; // LLPounceable is for an LLPounceable instance on the heap or the stack. @@ -143,7 +142,7 @@ class LLPounceable private: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; public: typedef typename traits::func_t func_t; @@ -177,12 +176,9 @@ public: // If this new value is non-empty, flush anything pending in the queue. if (mHeld != mEmpty) { - queue_t& queue(get_queue()); - while (! queue.empty()) - { - queue.front()(mHeld); - queue.pop(); - } + signal_t& signal(get_signal()); + signal(mHeld); + signal.disconnect_all_slots(); } } @@ -196,13 +192,14 @@ public: } else { - // held value still empty, queue func() for later - get_queue().push(func); + // Held value still empty, queue func() for later. By default, + // connect() enqueues slots in FIFO order. + get_signal().connect(func); } } private: - queue_t& get_queue() { return mQueue.get(this); } + signal_t& get_signal() { return mQueue.get(this); } // Store both the current and the empty value. // MAYBE: Might be useful to delegate to LLPounceableTraits the meaning of diff --git a/indra/llcommon/tests/llpounceable_test.cpp b/indra/llcommon/tests/llpounceable_test.cpp index 1f8cdca145..25458865a6 100644 --- a/indra/llcommon/tests/llpounceable_test.cpp +++ b/indra/llcommon/tests/llpounceable_test.cpp @@ -20,6 +20,13 @@ // other Linden headers #include "../test/lltut.h" +/*----------------------------- string testing -----------------------------*/ +void append(std::string* dest, const std::string& src) +{ + dest->append(src); +} + +/*-------------------------- Data-struct testing ---------------------------*/ struct Data { Data(const std::string& data): @@ -191,6 +198,29 @@ namespace tut template<> template<> void object::test<6>() + { + set_test_name("queue order"); + std::string data; + LLPounceable pounceable; + pounceable.callWhenReady(boost::bind(append, _1, "a")); + pounceable.callWhenReady(boost::bind(append, _1, "b")); + pounceable.callWhenReady(boost::bind(append, _1, "c")); + pounceable = &data; + ensure_equals("callWhenReady() must preserve chronological order", + data, "abc"); + + std::string data2; + pounceable = NULL; + pounceable.callWhenReady(boost::bind(append, _1, "d")); + pounceable.callWhenReady(boost::bind(append, _1, "e")); + pounceable.callWhenReady(boost::bind(append, _1, "f")); + pounceable = &data2; + ensure_equals("LLPounceable must reset queue when fired", + data2, "def"); + } + + template<> template<> + void object::test<7>() { set_test_name("compile-fail test, uncomment to check"); // The following declaration should fail: only LLPounceableQueue and -- cgit v1.2.3