From a7db674c9edab2fea9c4d56bd7e685850d434334 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 14 Aug 2014 13:29:09 -0700 Subject: [PATCH 1/8] Changed old shared_ptr construction to newer make_shared factory --- autowiring/AutoPacket.h | 3 +-- src/autowiring/test/AnySharedPointerTest.cpp | 19 +++++++++---------- src/autowiring/test/AutoConstructTest.cpp | 2 +- src/autowiring/test/AutoFilterTest.cpp | 4 ++-- src/autowiring/test/ContextCleanupTest.cpp | 5 +++-- src/autowiring/test/CoreThreadTest.cpp | 14 +++++++------- 6 files changed, 23 insertions(+), 24 deletions(-) diff --git a/autowiring/AutoPacket.h b/autowiring/AutoPacket.h index 8e03fb81e..836c01bcd 100644 --- a/autowiring/AutoPacket.h +++ b/autowiring/AutoPacket.h @@ -411,8 +411,7 @@ class AutoPacket: /// template void AddRecipient(std::function f) { - std::shared_ptr> filter(new MicroAutoFilter(f)); - InitializeRecipient(MakeAutoFilterDescriptor(filter)); + InitializeRecipient(MakeAutoFilterDescriptor(std::make_shared>(f))); } /// diff --git a/src/autowiring/test/AnySharedPointerTest.cpp b/src/autowiring/test/AnySharedPointerTest.cpp index ffee4d5b9..fbf458256 100644 --- a/src/autowiring/test/AnySharedPointerTest.cpp +++ b/src/autowiring/test/AnySharedPointerTest.cpp @@ -52,8 +52,7 @@ TEST_F(AnySharedPointerTest, SimpleDestructorStrike) SharedPointerSlot& slot = *new(buf) SharedPointerSlot; // In-place polymorphism on the slot: - std::shared_ptr unused(new MyUnusedClass); - slot = unused; + slot = std::make_shared(); // Destructor shouldn't be hit until we call it: ASSERT_FALSE(mucSlot.dtorStrike()) << "Destructor was struck prematurely"; @@ -79,9 +78,9 @@ TEST_F(AnySharedPointerTest, AnySharedPointerRelease) { } TEST_F(AnySharedPointerTest, SlotReassignment) { - std::shared_ptr sharedPointerA(new bool); - std::shared_ptr sharedPointerB(new bool); - std::shared_ptr sharedPointerC(new int); + auto sharedPointerA = std::make_shared(); + auto sharedPointerB = std::make_shared(); + auto sharedPointerC = std::make_shared(); // Make our slot, and start off with a shared pointer of one type: AnySharedPointer slot; @@ -101,7 +100,7 @@ TEST_F(AnySharedPointerTest, SlotReassignment) { } TEST_F(AnySharedPointerTest, SlotsInVector) { - std::shared_ptr sharedPtr(new bool); + auto sharedPtr = std::make_shared(); { std::vector slots; @@ -118,7 +117,7 @@ TEST_F(AnySharedPointerTest, SlotsInVector) { } TEST_F(AnySharedPointerTest, SlotDuplication) { - std::shared_ptr sharedPtr(new bool); + auto sharedPtr = std::make_shared(); AnySharedPointer slot2; @@ -143,8 +142,8 @@ TEST_F(AnySharedPointerTest, SlotDuplication) { } TEST_F(AnySharedPointerTest, TrivialRelease) { - std::shared_ptr a(new bool); - std::shared_ptr b(new int); + auto a = std::make_shared(); + auto b = std::make_shared(); // Assign the slot to two different values, and make sure that they are released properly AnySharedPointer slot; @@ -160,7 +159,7 @@ TEST_F(AnySharedPointerTest, TrivialRelease) { } TEST_F(AnySharedPointerTest, NoMultipleDelete) { - std::shared_ptr a(new bool); + auto a = std::make_shared(); std::weak_ptr b = a; // Create a slot and validate the behavior of reset, and to ensure that the underlying diff --git a/src/autowiring/test/AutoConstructTest.cpp b/src/autowiring/test/AutoConstructTest.cpp index fc4fded0f..95374b7b3 100644 --- a/src/autowiring/test/AutoConstructTest.cpp +++ b/src/autowiring/test/AutoConstructTest.cpp @@ -43,7 +43,7 @@ TEST_F(AutoConstructTest, CanConstructRvalueCtor) { AutoCreateContext ctxt; CurrentContextPusher pshr(ctxt); - std::unique_ptr> forwarded(new std::shared_ptr(originalPtr)); + auto forwarded = std::make_unique>(originalPtr); AutoConstruct coami(std::move(forwarded)); // Should have the correct number of references, no more and no less diff --git a/src/autowiring/test/AutoFilterTest.cpp b/src/autowiring/test/AutoFilterTest.cpp index d1898c70f..b8cb80d91 100644 --- a/src/autowiring/test/AutoFilterTest.cpp +++ b/src/autowiring/test/AutoFilterTest.cpp @@ -174,7 +174,7 @@ TEST_F(AutoFilterTest, VerifyNoMultiDecorate) { EXPECT_ANY_THROW(packet->Decorate(isDeco0type())) << "Typedef failed to throw exception"; //NOTE: A shared_ptr to an existing type will throw an exception - std::shared_ptr> sharedDeco0(new Decoration<0>); + auto sharedDeco0 = std::make_shared>(); EXPECT_ANY_THROW(packet->Decorate(sharedDeco0)) << "Reduction of shared_ptr to base type failed"; //NOTE: Inheritance will not throw an exception @@ -347,7 +347,7 @@ TEST_F(AutoFilterTest, VerifyTeardownArrangement) { std::shared_ptr packet; { // Create the filter and subscribe it - std::shared_ptr filterA(new FilterA); + auto filterA = std::make_shared(); filterAWeak = filterA; factory->AddSubscriber(filterA); diff --git a/src/autowiring/test/ContextCleanupTest.cpp b/src/autowiring/test/ContextCleanupTest.cpp index 1dfbc3a04..1421599d4 100644 --- a/src/autowiring/test/ContextCleanupTest.cpp +++ b/src/autowiring/test/ContextCleanupTest.cpp @@ -19,8 +19,9 @@ TEST_F(ContextCleanupTest, ValidateTeardownOrder) { std::weak_ptr self; }; - - std::shared_ptr(new WeakPtrChecker); + + // Construct, the destroy + std::make_shared(); } TEST_F(ContextCleanupTest, VerifyNoEarlyDtor) { diff --git a/src/autowiring/test/CoreThreadTest.cpp b/src/autowiring/test/CoreThreadTest.cpp index 0a8a5d374..338a23aea 100644 --- a/src/autowiring/test/CoreThreadTest.cpp +++ b/src/autowiring/test/CoreThreadTest.cpp @@ -172,7 +172,7 @@ TEST_F(CoreThreadTest, AUTOTHROW_VerifyNoLeakOnExecptions) { CurrentContextPusher pshr(ctxt); AutoRequired listener; - std::shared_ptr value(new std::string("sentinal")); + auto value = std::make_shared("sentinal"); std::weak_ptr watcher(value); try @@ -205,8 +205,8 @@ TEST_F(CoreThreadTest, VerifyDelayedDispatchQueueSimple) { std::this_thread::sleep_for(std::chrono::milliseconds(1)); // These are flags--we'll set them to true as the test proceeds - std::shared_ptr x(new bool(false)); - std::shared_ptr y(new bool(false)); + auto x = std::make_shared(false); + auto y = std::make_shared(false); // Pend a delayed event first, and then an immediate event right afterwards: *t += std::chrono::hours(1), [x] { *x = true; }; @@ -223,7 +223,7 @@ TEST_F(CoreThreadTest, VerifyNoDelayDoubleFree) { ctxt->Initiate(); // We won't actually be referencing this, we just want to make sure it's not destroyed early - std::shared_ptr x(new bool); + auto x = std::make_shared(); // This deferred pend will never actually be executed: AutoRequired t; @@ -244,8 +244,8 @@ TEST_F(CoreThreadTest, VerifyDoublePendedDispatchDelay) { ctxt->Initiate(); // Some variables that we will set to true as the test proceeds: - std::shared_ptr x(new bool(false)); - std::shared_ptr y(new bool(false)); + auto x = std::make_shared(false); + auto y = std::make_shared(false); // Create a thread as before, and pend a few events. The order, here, is important. We intentionally // pend an event that won't happen for awhile, in order to trick the dispatch queue into waiting for @@ -295,7 +295,7 @@ TEST_F(CoreThreadTest, VerifyPendByTimePoint) { AutoRequired t; // Pend by an absolute time point, nothing really special here - std::shared_ptr x(new bool(false)); + auto x = std::make_shared(false); *t += (std::chrono::steady_clock::now() + std::chrono::milliseconds(1)), [&t, x] { *x = true; t->Stop(); From 74122cce49091bdcfe2b179b8bb28041a1468651 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 14 Aug 2014 15:47:49 -0700 Subject: [PATCH 2/8] Changed uses of old looping style to range-based loops --- autowiring/AutoPacket.h | 10 ++--- autowiring/ContextCreator.h | 27 ++++++------ autowiring/ContextCreatorBase.h | 12 +++--- autowiring/ContextMap.h | 9 ++-- autowiring/EventOutputStream.h | 5 +-- autowiring/InvokeRelay.h | 6 +-- src/autowiring/BasicThread.cpp | 5 +-- src/autowiring/CoreContext.cpp | 67 +++++++++++++++-------------- src/autowiring/DispatchQueue.cpp | 4 +- src/autowiring/TeardownNotifier.cpp | 4 +- 10 files changed, 72 insertions(+), 77 deletions(-) diff --git a/autowiring/AutoPacket.h b/autowiring/AutoPacket.h index 836c01bcd..528991dcf 100644 --- a/autowiring/AutoPacket.h +++ b/autowiring/AutoPacket.h @@ -365,8 +365,7 @@ class AutoPacket: // None of the inputs may be shared pointers--if any of the inputs are shared pointers, they must be attached // to this packet via Decorate, or else dereferenced and used that way. static_assert( - !is_shared_ptr::value && - !is_any...>::value, + !is_any, is_shared_ptr...>::value, "DecorateImmediate must not be used to attach a shared pointer, use Decorate on such a decoration instead" ); @@ -399,8 +398,7 @@ class AutoPacket: } // Now trigger a rescan to hit any deferred, unsatisfiable entries: - static const std::type_info* lamda_typeInfos [] = {&typeid(T), &typeid(Ts)...}; - for(auto ti : lamda_typeInfos) + for(const std::type_info* ti : {&typeid(T), &typeid(Ts)...}) MarkUnsatisfiable(*ti); }), PulseSatisfaction(pTypeSubs, 1 + sizeof...(Ts)); @@ -411,7 +409,9 @@ class AutoPacket: /// template void AddRecipient(std::function f) { - InitializeRecipient(MakeAutoFilterDescriptor(std::make_shared>(f))); + InitializeRecipient( + MakeAutoFilterDescriptor(std::make_shared>(f)) + ); } /// diff --git a/autowiring/ContextCreator.h b/autowiring/ContextCreator.h index 4a0b45240..f4449bf9e 100644 --- a/autowiring/ContextCreator.h +++ b/autowiring/ContextCreator.h @@ -41,10 +41,10 @@ class ContextCreator: template void Enumerate(Fn&& fn) { std::lock_guard lk(m_contextLock); - for(auto q = m_contexts.begin(); q != m_contexts.end(); q++) { - auto ctxt = q->second.lock(); + for(const auto& entry : m_contexts) { + auto ctxt = entry.second.lock(); if(ctxt) - if(!fn(q->first, ctxt)) + if(!fn(entry.first, ctxt)) return; } } @@ -56,8 +56,8 @@ class ContextCreator: Container Enumerate(void) { Container container; std::lock_guard lk(m_contextLock); - for(auto q = m_contexts.begin(); q != m_contexts.end(); q++) { - auto ctxt = q->second.lock(); + for(const auto& entry : m_contexts) { + auto ctxt = entry.second.lock(); if(ctxt) container.insert(container.end(), ctxt); } @@ -69,10 +69,10 @@ class ContextCreator: /// std::shared_ptr FindContext(const Key& key) { std::lock_guard lk(m_contextLock); - typename t_mpType::iterator q = m_contexts.find(key); - if(q == m_contexts.end()) + auto entry = m_contexts.find(key); + if(entry == m_contexts.end()) return std::shared_ptr(); - return q->second.lock(); + return entry.second.lock(); } /// @@ -122,7 +122,7 @@ class ContextCreator: /// The contaner could, in fact, have elements in it at the time control is returned to the caller. /// void Clear(bool wait) { - ContextCreatorBase::Clear(wait, m_contexts, [] (typename t_mpType::iterator q) {return q->second.lock();}); + ContextCreatorBase::Clear(wait, m_contexts, [] (const typename t_mpType::value_type& q) {return q.second.lock();}); } /// @@ -240,10 +240,9 @@ class ContextCreator: template void Enumerate(Fn&& fn) { std::lock_guard lk(m_contextLock); - for(auto q = m_contextList.begin(); q != m_contextList.end(); q++) { - auto ctxt = q->lock(); - if(ctxt) - if(!fn(ctxt)) + for(const auto& weak_ctxt : m_contextList) { + auto ctxt = weak_ctxt.lock(); + if(ctxt && !fn(ctxt)) return; } } @@ -264,7 +263,7 @@ class ContextCreator: void Clear(bool wait) { (std::lock_guard)m_contextLock, m_clearSentinal++; - ContextCreatorBase::Clear(wait, m_contextList, [] (typename t_contextList::iterator q) { return q->lock();}); + ContextCreatorBase::Clear(wait, m_contextList, [] (const typename t_contextList::value_type& q) { return q.lock();}); } /// diff --git a/autowiring/ContextCreatorBase.h b/autowiring/ContextCreatorBase.h index 680768354..5c482c0d3 100644 --- a/autowiring/ContextCreatorBase.h +++ b/autowiring/ContextCreatorBase.h @@ -30,8 +30,8 @@ class ContextCreatorBase { if(!wait) { // Trivial signal-clear-return: std::lock_guard lk(m_contextLock); - for(auto q = ctr.begin(); q != ctr.end(); q++) { - auto locked = locker(q); + for(const auto& ctxt : ctr) { + auto locked = locker(ctxt); if(locked) locked->SignalShutdown(); } @@ -44,8 +44,8 @@ class ContextCreatorBase { // Copy out and clear: { std::lock_guard lk(m_contextLock); - for(auto q = ctr.begin(); q != ctr.end(); q++) { - auto locked = locker(q); + for(const auto& ctxt : ctr) { + auto locked = locker(ctxt); if(locked) locked->SignalShutdown(); } @@ -54,8 +54,8 @@ class ContextCreatorBase { } // Signal everyone first, then wait in a second pass: - for(auto q = ctrCopy.begin(); q != ctrCopy.end(); q++) { - auto locked = locker(q); + for(const auto& ctxt : ctrCopy) { + auto locked = locker(ctxt); if(locked) locked->Wait(); } diff --git a/autowiring/ContextMap.h b/autowiring/ContextMap.h index a97c4a663..866be422f 100644 --- a/autowiring/ContextMap.h +++ b/autowiring/ContextMap.h @@ -64,11 +64,10 @@ class ContextMap template void Enumerate(Fn&& fn) { std::lock_guard lk(m_lk); - for(auto q = m_contexts.begin(); q != m_contexts.end(); q++) { - auto ctxt = q->second.lock(); - if(ctxt) - if(!fn(q->first, ctxt)) - return; + for(const auto& entry : m_contexts) { + auto ctxt = entry.second.lock(); + if(ctxt && !fn(entry.first, ctxt)) + return; } } diff --git a/autowiring/EventOutputStream.h b/autowiring/EventOutputStream.h index 7ff54bef6..8fbe1d9de 100644 --- a/autowiring/EventOutputStream.h +++ b/autowiring/EventOutputStream.h @@ -52,9 +52,8 @@ class EventOutputStreamBase { static std::map my_map = local; if(str == "Query") { - for(auto it = my_map.begin(); it != my_map.end(); ++it) - { - if((it->second) == memfn) return it->first; + for(const auto& entry : my_map) { + if((entry.second) == memfn) return entry.first; } return ""; } diff --git a/autowiring/InvokeRelay.h b/autowiring/InvokeRelay.h index 42dbdbdcc..82f68835a 100644 --- a/autowiring/InvokeRelay.h +++ b/autowiring/InvokeRelay.h @@ -83,11 +83,9 @@ class InvokeRelay { // Context not yet started return; - const auto& dq = erp->GetDispatchQueue(); std::lock_guard lk(erp->GetDispatchQueueLock()); - - for(auto q = dq.begin(); q != dq.end(); q++) - (**q).AddExisting(new CurriedInvokeRelay(dynamic_cast(**q), fnPtr, args...)); + for(DispatchQueue* q : erp->GetDispatchQueue()) + q->AddExisting(new CurriedInvokeRelay(dynamic_cast(*q), fnPtr, args...)); } }; diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index 859049e8e..eeb9a395c 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -203,9 +203,8 @@ void BasicThread::Stop(bool graceful) { void BasicThread::ForceCoreThreadReidentify(void) { for(const auto& ctxt : ContextEnumerator(AutoGlobalContext())) { - auto threadListCpy = ctxt->CopyBasicThreadList(); - for(auto q = threadListCpy.begin(); q != threadListCpy.end(); q++) - (**q).SetCurrentThreadName(); + for(const auto& thread : ctxt->CopyBasicThreadList()) + thread->SetCurrentThreadName(); } } diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index e2dfc687d..7a315ef49 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -268,8 +268,8 @@ void CoreContext::FindByTypeUnsafe(AnySharedPointer& reference) const { // Resolve based on iterated dynamic casts for each concrete type: bool assigned = false; - for(auto q = m_concreteTypes.begin(); q != m_concreteTypes.end(); q++) { - if(!reference->try_assign(**q)) + for(const auto& type : m_concreteTypes) { + if(!reference->try_assign(*type)) // No match, try the next entry continue; @@ -295,10 +295,10 @@ std::vector> CoreContext::CopyBasicThreadList(void) // It's safe to enumerate this list from outside of a protective lock because a linked list // has stable iterators, we do not delete entries from the interior of this list, and we only // add entries to the end of the list. - for(auto q = m_threads.begin(); q != m_threads.end(); q++){ - BasicThread* thread = dynamic_cast(*q); + for(CoreRunnable* q : m_threads){ + BasicThread* thread = dynamic_cast(q); if (thread) - retVal.push_back((*thread).GetSelf()); + retVal.push_back(thread->GetSelf()); } return retVal; } @@ -367,8 +367,8 @@ void CoreContext::SignalShutdown(bool wait, ShutdownMode shutdownMode) { // Fill strong lock series in order to ensure proper teardown interleave: childrenInterleave.reserve(m_children.size()); - for(auto q = m_children.begin(); q != m_children.end(); q++) { - auto childContext = q->lock(); + for(const auto& entry : m_children) { + auto childContext = entry.lock(); // Technically, it *is* possible for this weak pointer to be expired, even though // we're holding the lock. This may happen if the context itself is exiting even @@ -379,7 +379,7 @@ void CoreContext::SignalShutdown(bool wait, ShutdownMode shutdownMode) { continue; // Add to the interleave so we can SignalTerminate in a controlled way. - childrenInterleave.push_back(q->lock()); + childrenInterleave.push_back(childContext); } } @@ -389,9 +389,9 @@ void CoreContext::SignalShutdown(bool wait, ShutdownMode shutdownMode) { } // Pass notice to all child threads: - bool graceful = shutdownMode == ShutdownMode::Graceful; - for(t_threadList::iterator q = m_threads.begin(); q != m_threads.end(); ++q) - (*q)->Stop(graceful); + bool graceful = (shutdownMode == ShutdownMode::Graceful); + for(CoreRunnable* runnable : m_threads) + runnable->Stop(graceful); // Signal our condition variable m_stateBlock->m_stateChanged.notify_all(); @@ -401,8 +401,8 @@ void CoreContext::SignalShutdown(bool wait, ShutdownMode shutdownMode) { return; // Wait for the treads to finish before returning. - for (t_threadList::iterator it = m_threads.begin(); it != m_threads.end(); ++it) - (**it).Wait(); + for (CoreRunnable* runnable : m_threads) + runnable->Wait(); } void CoreContext::Wait(void) { @@ -514,16 +514,17 @@ void CoreContext::CancelAutowiringNotification(DeferrableAutowiring* pDeferrable void CoreContext::Dump(std::ostream& os) const { std::lock_guard lk(m_stateBlock->m_lock); - for(auto q = m_typeMemos.begin(); q != m_typeMemos.end(); q++) { - os << q->first.name(); - const void* pObj = q->second.m_value->ptr(); + + for(const auto& entry : m_typeMemos) { + os << entry.first.name(); + const void* pObj = entry.second.m_value->ptr(); if(pObj) os << " 0x" << std::hex << pObj; os << std::endl; } - for(auto q = m_threads.begin(); q != m_threads.end(); q++) { - BasicThread* pThread = dynamic_cast(*q); + for(CoreRunnable* runnable : m_threads) { + BasicThread* pThread = dynamic_cast(runnable); if (!pThread) continue; const char* name = pThread->GetName(); @@ -553,21 +554,20 @@ void CoreContext::UnregisterEventReceiversUnsafe(void) { } void CoreContext::BroadcastContextCreationNotice(const std::type_info& sigil) const { - auto q = m_nameListeners.find(sigil); - if(q != m_nameListeners.end()) { + auto listeners = m_nameListeners.find(sigil); + if(listeners != m_nameListeners.end()) { // Iterate through all listeners: - const auto& list = q->second; - for(auto q = list.begin(); q != list.end(); q++) - (**q).ContextCreated(); + for(BoltBase* bolt : listeners->second) + bolt->ContextCreated(); } // In the case of an anonymous sigil type, we do not notify the all-types // listeners a second time. if(sigil != typeid(void)) { - q = m_nameListeners.find(typeid(void)); - if(q != m_nameListeners.end()) - for(auto cur : q->second) - cur->ContextCreated(); + listeners = m_nameListeners.find(typeid(void)); + if(listeners != m_nameListeners.end()) + for(BoltBase* bolt : listeners->second) + bolt->ContextCreated(); } // Notify the parent next: @@ -625,9 +625,9 @@ void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, cons } // Give children a chance to also update their deferred elements: - for(auto q = m_children.begin(); q != m_children.end(); q++) { + for(const auto& weak_child : m_children) { // Hold reference to prevent this iterator from becoming invalidated: - auto ctxt = q->lock(); + auto ctxt = weak_child.lock(); if(!ctxt) continue; @@ -717,9 +717,9 @@ void CoreContext::UnsnoopEvents(Object* oSnooper, const JunctionBoxEntry void CoreContext::FilterException(void) { bool handled = false; - for(auto q = m_filters.begin(); q != m_filters.end(); q++) + for(ExceptionFilter* filter : m_filters) try { - (*q)->Filter(); + filter->Filter(); handled = true; } catch(...) { // Do nothing @@ -734,6 +734,7 @@ void CoreContext::FilterException(void) { // Parent handled it, we're good to go return; } catch(...) { + // Do nothing } } @@ -745,9 +746,9 @@ void CoreContext::FilterException(void) { void CoreContext::FilterFiringException(const JunctionBoxBase* pProxy, Object* pRecipient) { // Filter in order: for(CoreContext* pCur = this; pCur; pCur = pCur->GetParentContext().get()) - for(auto q = pCur->m_filters.begin(); q != pCur->m_filters.end(); q++) + for(ExceptionFilter* filter : pCur->m_filters) try { - (*q)->Filter(pProxy, pRecipient); + filter->Filter(pProxy, pRecipient); } catch(...) { // Do nothing, filter didn't want to filter this exception } diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index 97695ba7a..54e40211a 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -10,8 +10,8 @@ DispatchQueue::DispatchQueue(void): DispatchQueue::~DispatchQueue(void) { // Wipe out each entry in the queue, we can't call any of them because we're in teardown - for(auto q = m_dispatchQueue.begin(); q != m_dispatchQueue.end(); q++) - delete *q; + for(DispatchThunkBase* thunk : m_dispatchQueue) + delete thunk; while (!m_delayedQueue.empty()) { DispatchThunkDelayed thunk = m_delayedQueue.top(); diff --git a/src/autowiring/TeardownNotifier.cpp b/src/autowiring/TeardownNotifier.cpp index 934b36a35..307113be6 100644 --- a/src/autowiring/TeardownNotifier.cpp +++ b/src/autowiring/TeardownNotifier.cpp @@ -8,8 +8,8 @@ TeardownNotifier::~TeardownNotifier(void) { } void TeardownNotifier::NotifyTeardownListeners(void) { - for(auto q = m_teardownListeners.begin(); q != m_teardownListeners.end(); q++) - (*q)(); + for(const auto& listener : m_teardownListeners) + listener(); m_teardownListeners.clear(); } From 7066f6f45e36c1b95f8b00bed258dfa8ce6dec6d Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 14 Aug 2014 16:45:00 -0700 Subject: [PATCH 3/8] Adding const correctness where applicable --- autowiring/AutoPacket.h | 21 ++++++++++--------- autowiring/Bolt.h | 8 +++---- src/autonet/AutoNetServerImpl.cpp | 6 +++--- src/autotesting/AutowiringEnclosure.cpp | 4 ++-- src/autowiring/AutoPacket.cpp | 10 ++++----- src/autowiring/CoreContext.cpp | 14 ++++++------- src/autowiring/test/ContextEnumeratorTest.cpp | 13 ++++++------ src/autowiring/test/CoreContextTest.cpp | 4 ++-- src/autowiring/test/EventReceiverTest.cpp | 6 +++--- src/autowiring/test/ObjectPoolTest.cpp | 6 +++--- src/autowiring/test/PostConstructTest.cpp | 2 +- 11 files changed, 47 insertions(+), 47 deletions(-) diff --git a/autowiring/AutoPacket.h b/autowiring/AutoPacket.h index 528991dcf..7f635b70c 100644 --- a/autowiring/AutoPacket.h +++ b/autowiring/AutoPacket.h @@ -357,26 +357,27 @@ class AutoPacket: /// template void DecorateImmediate(const T& immed, const Ts&... immeds) { - // These are the things we're going to be working with while we perform immediate decoration: - static const std::type_info* sc_typeInfo [] = {&typeid(T), &typeid(Ts)...}; - const void* pvImmeds [] = {&immed, &immeds...}; - DecorationDisposition* pTypeSubs[1 + sizeof...(Ts)]; - // None of the inputs may be shared pointers--if any of the inputs are shared pointers, they must be attached // to this packet via Decorate, or else dereferenced and used that way. static_assert( !is_any, is_shared_ptr...>::value, "DecorateImmediate must not be used to attach a shared pointer, use Decorate on such a decoration instead" ); + + // These are the things we're going to be working with while we perform immediate decoration: + static const std::type_info* s_argTypes [] = {&typeid(T), &typeid(Ts)...}; + static const size_t s_arity = 1 + sizeof...(Ts); + const void* pvImmeds [] = {&immed, &immeds...}; + DecorationDisposition* pTypeSubs[s_arity]; // Perform standard decoration with a short initialization: { std::lock_guard lk(m_lock); for(size_t i = 0; i <= sizeof...(Ts); i++) { - pTypeSubs[i] = &m_decorations[*sc_typeInfo[i]]; + pTypeSubs[i] = &m_decorations[*s_argTypes[i]]; if(pTypeSubs[i]->wasCheckedOut) { std::stringstream ss; - ss << "Cannot perform immediate decoration with type " << sc_typeInfo[i]->name() + ss << "Cannot perform immediate decoration with type " << s_argTypes[i]->name() << ", the requested decoration already exists"; throw std::runtime_error(ss.str()); } @@ -392,16 +393,16 @@ class AutoPacket: // Pulse satisfaction: MakeAtExit([this, &pTypeSubs] { // Mark entries as unsatisfiable: - for(auto pEntry : pTypeSubs) { + for(DecorationDisposition* pEntry : pTypeSubs) { pEntry->satisfied = false; pEntry->m_pImmediate = nullptr; } // Now trigger a rescan to hit any deferred, unsatisfiable entries: - for(const std::type_info* ti : {&typeid(T), &typeid(Ts)...}) + for(const std::type_info* ti : s_argTypes) MarkUnsatisfiable(*ti); }), - PulseSatisfaction(pTypeSubs, 1 + sizeof...(Ts)); + PulseSatisfaction(pTypeSubs, s_arity); } /// diff --git a/autowiring/Bolt.h b/autowiring/Bolt.h index 173cc9279..9f9f14493 100644 --- a/autowiring/Bolt.h +++ b/autowiring/Bolt.h @@ -19,11 +19,11 @@ class Bolt: { public: const t_TypeInfoVector GetContextSigils(void) const override { - static const std::type_info* sc_types[] = { + static const std::type_info* s_types[] = { &typeid(Sigil)..., nullptr }; - return sc_types; + return s_types; } static_assert(!is_any_same::value, "Can't use 'void' as a sigil type"); @@ -35,7 +35,7 @@ class Bolt<>: { public: const t_TypeInfoVector GetContextSigils(void) const override { - static const std::type_info* sc_types[] = {nullptr}; - return sc_types; + static const std::type_info* s_types[] = {nullptr}; + return s_types; } }; diff --git a/src/autonet/AutoNetServerImpl.cpp b/src/autonet/AutoNetServerImpl.cpp index a1dda502f..a5600f711 100644 --- a/src/autonet/AutoNetServerImpl.cpp +++ b/src/autonet/AutoNetServerImpl.cpp @@ -200,7 +200,7 @@ void AutoNetServerImpl::NewObject(CoreContext& ctxt, const AnySharedPointer& obj // Check if type receives any events { Json::array listenerTypes; - for(auto& event : m_EventTypes) { + for(const auto& event : m_EventTypes) { if(event->IsSameAs(objectPtr.get())) listenerTypes.push_back(demangle(event->Type())); } @@ -240,7 +240,7 @@ void AutoNetServerImpl::HandleSubscribe(websocketpp::connection_hdl hdl) { m_Subscribers.insert(hdl); Json::array types; - for(auto type : m_AllTypes) { + for(const auto& type : m_AllTypes) { types.push_back(type.first); } @@ -248,7 +248,7 @@ void AutoNetServerImpl::HandleSubscribe(websocketpp::connection_hdl hdl) { AutoGlobalContext()->BuildCurrentState(); // Send breakpoint message - for(auto& breakpoint : m_breakpoints) { + for(const auto& breakpoint : m_breakpoints) { SendMessage(hdl, "breakpoint", breakpoint); } } diff --git a/src/autotesting/AutowiringEnclosure.cpp b/src/autotesting/AutowiringEnclosure.cpp index 051f72303..25e439191 100644 --- a/src/autotesting/AutowiringEnclosure.cpp +++ b/src/autotesting/AutowiringEnclosure.cpp @@ -49,8 +49,8 @@ void AutowiringEnclosure::OnTestEnd(const testing::TestInfo& info) { assert(false); } - static const char sc_autothrow[] = "AUTOTHROW_"; - if(!strncmp(sc_autothrow, info.name(), ARRAYCOUNT(sc_autothrow) - 1)) + static const char s_autothrow[] = "AUTOTHROW_"; + if(!strncmp(s_autothrow, info.name(), ARRAYCOUNT(s_autothrow) - 1)) // Throw expected, end here return; diff --git a/src/autowiring/AutoPacket.cpp b/src/autowiring/AutoPacket.cpp index e17757a3e..c0fdb21f9 100644 --- a/src/autowiring/AutoPacket.cpp +++ b/src/autowiring/AutoPacket.cpp @@ -87,7 +87,7 @@ void AutoPacket::MarkUnsatisfiable(const std::type_info& info) { // Update satisfaction inside of lock decoration = &dFind->second; - for(auto& satCounter : decoration->m_subscribers) { + for(const auto& satCounter : decoration->m_subscribers) { if(satCounter.second) // Entry is mandatory, leave it unsatisfaible continue; @@ -115,7 +115,7 @@ void AutoPacket::UpdateSatisfaction(const std::type_info& info) { // Update satisfaction inside of lock decoration = &dFind->second; - for(auto& satCounter : decoration->m_subscribers) + for(const auto& satCounter : decoration->m_subscribers) if(satCounter.first->Decrement(satCounter.second)) callQueue.push_back(satCounter.first); } @@ -159,8 +159,8 @@ void AutoPacket::PulseSatisfaction(DecorationDisposition* pTypeSubs[], size_t nI { std::lock_guard lk(m_lock); for(size_t i = nInfos; i--;) { - for(auto& satCounter : pTypeSubs[i]->m_subscribers) { - auto& cur = satCounter.first; + for(const auto& satCounter : pTypeSubs[i]->m_subscribers) { + SatCounter* cur = satCounter.first; if (satCounter.second) { ++cur->remaining; } @@ -194,7 +194,7 @@ void AutoPacket::Initialize(void) { // Call all subscribers with no required or optional arguments: // NOTE: This may result in decorations that cause other subscribers to be called. - for (auto* call : callCounters) + for (SatCounter* call : callCounters) call->CallAutoFilter(*this); // Initial satisfaction of the AutoPacket: diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index 7a315ef49..e436b8998 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -65,7 +65,7 @@ CoreContext::~CoreContext(void) { UnregisterEventReceiversUnsafe(); // Tell all context members that we're tearing down: - for(auto q : m_contextMembers) + for(ContextMember* q : m_contextMembers) q->NotifyContextTeardown(); } @@ -335,7 +335,7 @@ void CoreContext::Initiate(void) { // Signal our condition variable m_stateBlock->m_stateChanged.notify_all(); - for(auto q : m_threads) + for(CoreRunnable* q : m_threads) q->Start(outstanding); } @@ -470,7 +470,7 @@ void CoreContext::BuildCurrentState(void) { // Recurse on all children std::lock_guard lk(m_stateBlock->m_lock); - for(auto c : m_children) { + for(const auto& c : m_children) { auto cur = c.lock(); if(!cur) continue; @@ -538,8 +538,8 @@ void ShutdownCurrentContext(void) { void CoreContext::UnregisterEventReceiversUnsafe(void) { // Release all event receivers originating from this context: - for(auto q : m_eventReceivers) - m_junctionBoxManager->RemoveEventReceiver(q); + for(const auto& entry : m_eventReceivers) + m_junctionBoxManager->RemoveEventReceiver(entry); // Notify our parent (if we have one) that our event receivers are going away: if(m_pParent) @@ -605,7 +605,7 @@ void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, cons auto top = stk.top(); stk.pop(); - for(auto* pNext = top; pNext; pNext = pNext->GetFlink()) { + for(DeferrableAutowiring* pNext = top; pNext; pNext = pNext->GetFlink()) { pNext->SatisfyAutowiring(value.m_value->shared_ptr()); // See if there's another chain we need to process: @@ -642,7 +642,7 @@ void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, cons lk.unlock(); // Run through everything else and finalize it all: - for(auto cur : satisfiable) + for(const auto& cur : satisfiable) cur.first->Finalize(cur.second); } diff --git a/src/autowiring/test/ContextEnumeratorTest.cpp b/src/autowiring/test/ContextEnumeratorTest.cpp index 21e808720..86167cc86 100644 --- a/src/autowiring/test/ContextEnumeratorTest.cpp +++ b/src/autowiring/test/ContextEnumeratorTest.cpp @@ -89,7 +89,7 @@ TEST_F(ContextEnumeratorTest, VerifyComplexEnumeration) { // Verify there is only one context under the first context int firstCount = 0; - for(auto ctxt : ContextEnumeratorT(firstContext)) { + for(const auto& ctxt : ContextEnumeratorT(firstContext)) { firstCount++; ASSERT_EQ(ctxt, firstNamed); ASSERT_EQ(ctxt->GetParentContext(), firstContext); @@ -98,7 +98,7 @@ TEST_F(ContextEnumeratorTest, VerifyComplexEnumeration) { // Verify there is only one context under the second context int secondCount = 0; - for(auto ctxt : ContextEnumeratorT(secondContext)) { + for(const auto& ctxt : ContextEnumeratorT(secondContext)) { secondCount++; ASSERT_EQ(ctxt, secondNamed); ASSERT_EQ(ctxt->GetParentContext(), secondContext); @@ -106,10 +106,9 @@ TEST_F(ContextEnumeratorTest, VerifyComplexEnumeration) { ASSERT_EQ(secondCount, 1) << "Expected exactly one context in the parent context, found " << secondCount; // Verify global context structure - int globalCount = 0; - for(auto ctxt : ContextEnumeratorT(AutoGlobalContext())) { - globalCount++; - } + auto enumerator = ContextEnumeratorT(AutoGlobalContext()); + int globalCount = std::distance(enumerator.begin(), enumerator.end()); + ASSERT_EQ(globalCount, 2) << "Expected exactly one context in the parent context, found " << globalCount; } @@ -168,7 +167,7 @@ TEST_F(ContextEnumeratorTest, ComplexRemovalInterference) { } // Now verify that nothing we eliminated was enumerated: - for(auto& cur : eliminated) + for(const auto& cur : eliminated) ASSERT_TRUE(cur.expired()) << "Found an element that was iterated after it should have been unreachable"; } diff --git a/src/autowiring/test/CoreContextTest.cpp b/src/autowiring/test/CoreContextTest.cpp index 31190eb2b..0548354c5 100644 --- a/src/autowiring/test/CoreContextTest.cpp +++ b/src/autowiring/test/CoreContextTest.cpp @@ -24,7 +24,7 @@ TEST_F(CoreContextTest, TestEnumerateChildren) { // Enumerate and see what we get back: std::set> allChildren; - for(auto cur : CurrentContextEnumerator()) + for(const auto& cur : CurrentContextEnumerator()) allChildren.insert(cur); // Verify we get exactly four back: @@ -70,7 +70,7 @@ TEST_F(CoreContextTest, TestEarlyLambdaReturn) { // Enumerate, but stop after three: std::vector> allChildren; size_t totalSoFar = 0; - for(auto& ctxt : CurrentContextEnumerator()) { + for(const auto& ctxt : CurrentContextEnumerator()) { if(totalSoFar++ == 3) break; allChildren.push_back(ctxt); diff --git a/src/autowiring/test/EventReceiverTest.cpp b/src/autowiring/test/EventReceiverTest.cpp index 4e97cf217..50ef63947 100644 --- a/src/autowiring/test/EventReceiverTest.cpp +++ b/src/autowiring/test/EventReceiverTest.cpp @@ -99,11 +99,11 @@ TEST_F(EventReceiverTest, NontrivialCopy) { // Accept dispatch delivery: //receiver->AcceptDispatchDelivery(); - static const int sc_numElems = 10; + static const int s_numElems = 10; // Create the vector we're going to copy over: vector ascending; - for(int i = 0; i < sc_numElems; i++) + for(int i = 0; i < s_numElems; i++) ascending.push_back(i); // Deferred fire: @@ -122,7 +122,7 @@ TEST_F(EventReceiverTest, NontrivialCopy) { // Validate our vectors: ASSERT_EQ(10, (int)receiver->m_myVec.size()) << "Receiver was not populated correctly with a vector"; - for(int i = 0; i < sc_numElems; i++) + for(int i = 0; i < s_numElems; i++) EXPECT_EQ(i, ascending[i]) << "Element at offset " << i << " was incorrectly copied"; } diff --git a/src/autowiring/test/ObjectPoolTest.cpp b/src/autowiring/test/ObjectPoolTest.cpp index 9e57c32d7..7374132b0 100644 --- a/src/autowiring/test/ObjectPoolTest.cpp +++ b/src/autowiring/test/ObjectPoolTest.cpp @@ -270,13 +270,13 @@ TEST_F(ObjectPoolTest, MovableObjectPool) { } TEST_F(ObjectPoolTest, MovableObjectPoolAysnc) { - static const size_t sc_count = 10000; + static const size_t s_count = 10000; ObjectPool from; { // Issue a zillion objects from the from pool: std::vector> objs; - for(size_t i = sc_count; i--;) + for(size_t i = s_count; i--;) objs.push_back(from.Wait()); // Make a thread, let it hold these objects while we move its pool: @@ -291,5 +291,5 @@ TEST_F(ObjectPoolTest, MovableObjectPoolAysnc) { AutoCurrentContext()->SignalShutdown(true); // Verify that new pool got all of the objects: - ASSERT_EQ(sc_count, to.GetCached()) << "Object pool move operation did not correctly relay checked out types"; + ASSERT_EQ(s_count, to.GetCached()) << "Object pool move operation did not correctly relay checked out types"; } diff --git a/src/autowiring/test/PostConstructTest.cpp b/src/autowiring/test/PostConstructTest.cpp index e2df5490d..d20a9692d 100644 --- a/src/autowiring/test/PostConstructTest.cpp +++ b/src/autowiring/test/PostConstructTest.cpp @@ -17,7 +17,7 @@ class ContextExposer: public: size_t DeferredCount(void) const { size_t ct = 0; - for(auto& entry : CoreContext::m_typeMemos) + for(const auto& entry : CoreContext::m_typeMemos) for(auto cur = entry.second.pFirst; cur; cur = cur->GetFlink()) ct++; return ct; From 991790f57ae8381099d8e83814a0cb7f2e266b0c Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 14 Aug 2014 16:55:31 -0700 Subject: [PATCH 4/8] Removed unnecessary intermediate variables in MicroBolt --- autowiring/MicroBolt.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/autowiring/MicroBolt.h b/autowiring/MicroBolt.h index b9ee66ab8..1fe892c61 100644 --- a/autowiring/MicroBolt.h +++ b/autowiring/MicroBolt.h @@ -23,10 +23,8 @@ struct MicroBolt: // NOTE: Injection of T into all matching contexts may result in // multiple calls to Inject() if a matching context // is created during traversal. - std::shared_ptr rootContext = CoreContext::CurrentContext(); - auto listSigils = {ContextEnumeratorT(rootContext)...}; - for (auto findSigil : listSigils) - for (auto context : findSigil) + for(auto findSigil : {ContextEnumeratorT(CoreContext::CurrentContext())...}) + for(auto context : findSigil) context->template Inject(); } void ContextCreated(void) override; From 71e95dde41db8416cb3ad43662d897c83465003f Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 14 Aug 2014 19:43:29 -0700 Subject: [PATCH 5/8] Fixing gimpy Visual Studio handling of multi-block statements in range-based for loops --- src/autowiring/CoreContext.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index e436b8998..011de19c1 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -717,13 +717,14 @@ void CoreContext::UnsnoopEvents(Object* oSnooper, const JunctionBoxEntry void CoreContext::FilterException(void) { bool handled = false; - for(ExceptionFilter* filter : m_filters) + for(ExceptionFilter* filter : m_filters) { try { filter->Filter(); handled = true; } catch(...) { // Do nothing } + } // Pass to parent if one exists: if(m_pParent) { @@ -746,12 +747,13 @@ void CoreContext::FilterException(void) { void CoreContext::FilterFiringException(const JunctionBoxBase* pProxy, Object* pRecipient) { // Filter in order: for(CoreContext* pCur = this; pCur; pCur = pCur->GetParentContext().get()) - for(ExceptionFilter* filter : pCur->m_filters) + for(ExceptionFilter* filter : pCur->m_filters) { try { filter->Filter(pProxy, pRecipient); } catch(...) { // Do nothing, filter didn't want to filter this exception } + } } std::shared_ptr CoreContext::GetPacketFactory(void) { From 7de26ebb50669db6bfeba30e6b6cea42ea2b02d9 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 14 Aug 2014 19:44:09 -0700 Subject: [PATCH 6/8] make_unique does not quite work correctly in Visual Studio --- src/autowiring/test/AutoConstructTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/autowiring/test/AutoConstructTest.cpp b/src/autowiring/test/AutoConstructTest.cpp index 95374b7b3..9ae27609f 100644 --- a/src/autowiring/test/AutoConstructTest.cpp +++ b/src/autowiring/test/AutoConstructTest.cpp @@ -43,7 +43,7 @@ TEST_F(AutoConstructTest, CanConstructRvalueCtor) { AutoCreateContext ctxt; CurrentContextPusher pshr(ctxt); - auto forwarded = std::make_unique>(originalPtr); + std::unique_ptr> forwarded{new std::shared_ptr(originalPtr)}; AutoConstruct coami(std::move(forwarded)); // Should have the correct number of references, no more and no less From 5821f32397f576f435f32adf2f01e05118cd46d0 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Fri, 15 Aug 2014 19:27:02 -0700 Subject: [PATCH 7/8] Extra if --- autowiring/ContextCreator.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/autowiring/ContextCreator.h b/autowiring/ContextCreator.h index f4449bf9e..55326c10c 100644 --- a/autowiring/ContextCreator.h +++ b/autowiring/ContextCreator.h @@ -43,9 +43,8 @@ class ContextCreator: std::lock_guard lk(m_contextLock); for(const auto& entry : m_contexts) { auto ctxt = entry.second.lock(); - if(ctxt) - if(!fn(entry.first, ctxt)) - return; + if(ctxt && !fn(entry.first, ctxt)) + return; } } From 852e9d43c7454e06029f2a2fbe91da4145915046 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Fri, 15 Aug 2014 19:40:19 -0700 Subject: [PATCH 8/8] We're iterating over arguments, so use arity --- autowiring/AutoPacket.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autowiring/AutoPacket.h b/autowiring/AutoPacket.h index 7f635b70c..b53f64400 100644 --- a/autowiring/AutoPacket.h +++ b/autowiring/AutoPacket.h @@ -373,7 +373,7 @@ class AutoPacket: // Perform standard decoration with a short initialization: { std::lock_guard lk(m_lock); - for(size_t i = 0; i <= sizeof...(Ts); i++) { + for(size_t i = 0; i < s_arity; i++) { pTypeSubs[i] = &m_decorations[*s_argTypes[i]]; if(pTypeSubs[i]->wasCheckedOut) { std::stringstream ss;