From 04838f189ff564a0016d6b221d97e4ec55de94fe Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 19:46:59 -0700 Subject: [PATCH 01/63] added a typedef for signal registration types --- autowiring/auto_signal.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index 5f7823e32..d18975a60 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -198,8 +198,9 @@ namespace autowiring { const std::shared_ptr> m_relay; public: - internal::signal_node* operator+=(std::function&& fn) { return *m_relay += std::move(fn); } - void operator-=(internal::signal_node* node) { return *m_relay -= node; } + typedef internal::signal_node t_registration; + t_registration* operator+=(std::function&& fn) { return *m_relay += std::move(fn); } + void operator-=(t_registration* node) { return *m_relay -= node; } /// /// Raises the signal and invokes all attached handlers From 4f52556f2567d1329fc2f062df4bf56f9df1ef17 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 20:05:47 -0700 Subject: [PATCH 02/63] Split the AutoConfigParserTest into it's own file. --- src/autowiring/test/AutoConfigParserTest.cpp | 67 ++++++++++++++++++++ src/autowiring/test/AutoConfigTest.cpp | 46 -------------- src/autowiring/test/CMakeLists.txt | 1 + 3 files changed, 68 insertions(+), 46 deletions(-) create mode 100644 src/autowiring/test/AutoConfigParserTest.cpp diff --git a/src/autowiring/test/AutoConfigParserTest.cpp b/src/autowiring/test/AutoConfigParserTest.cpp new file mode 100644 index 000000000..d20bb2444 --- /dev/null +++ b/src/autowiring/test/AutoConfigParserTest.cpp @@ -0,0 +1,67 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include +#include + +class AutoConfigParserTest: + public testing::Test +{}; + +TEST_F(AutoConfigParserTest, ExtractKeyTestWin) { + std::string type1("struct ConfigTypeExtractor"); + const auto key1 = autowiring::ExtractKey(type1); + ASSERT_EQ("Namespace1.XYZ", key1) << "Failed to properly extract Key with a single namespace"; + + std::string type2("struct ConfigTypeExtractor"); + const auto key2 = autowiring::ExtractKey(type2); + ASSERT_EQ("Namespace1.Namespace2.XYZ", key2) << "Failed to properly extract Key with multiple namespaces"; + + std::string type3("struct ConfigTypeExtractor"); + const auto key3 = autowiring::ExtractKey(type3); + ASSERT_EQ("XYZ", key3) << "Failed to properly extract Key with no namespace"; + + std::string type4("struct ConfigTypeExtractor"); + const auto key4 = autowiring::ExtractKey(type4); + ASSERT_EQ("ClassNamespace1.AutoConfigTest.XYZ", key4) << "Failed to properly extract Key with class namespaces"; + + std::string type5("struct ConfigTypeExtractor"); + const auto key5 = autowiring::ExtractKey(type5); + ASSERT_EQ("ClassNamespace1.Base::Nested.XYZ", key5) << "Failed to properly extract Key from nested"; +} + +TEST_F(AutoConfigParserTest, ExtractKeyTestPOSIX) { + std::string type1("ConfigTypeExtractor"); + const auto key1 = autowiring::ExtractKey(type1); + ASSERT_EQ("Namespace1.XYZ", key1) << "Failed to properly extract Key with a single namespace"; + + std::string type2("ConfigTypeExtractor"); + const auto key2 = autowiring::ExtractKey(type2); + ASSERT_EQ("Namespace1.Namespace2.XYZ", key2) << "Failed to properly extract Key with multiple namespaces"; + + std::string type3("ConfigTypeExtractor"); + const auto key3 = autowiring::ExtractKey(type3); + ASSERT_EQ("XYZ", key3) << "Failed to properly extract Key with no namespace"; + + std::string type4("ConfigTypeExtractor"); + const auto key4 = autowiring::ExtractKey(type4); + ASSERT_EQ("ClassNamespace1.AutoConfigTest.XYZ", key4) << "Failed to properly extract Key with class namespaces"; + + std::string type5("ConfigTypeExtractor"); + const auto key5 = autowiring::ExtractKey(type5); + ASSERT_EQ("ClassNamespace1.Base::Nested.XYZ", key5) << "Failed to properly extract Key from nested"; +} + +struct Namespace1 {}; +struct Namespace2 {}; +struct XYZ {}; + +struct MyConfigurableClass { + AutoConfig m_myName; +}; + +TEST_F(AutoConfigParserTest, VerifyCorrectDeconstruction) { + AutoRequired mcc; + + EXPECT_STREQ("Namespace1.XYZ", mcc->m_myName->m_key.c_str()) + << "Configuration variable name was not correctly extracted"; +} diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index d50476c15..5c42b125c 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -2,54 +2,15 @@ #include "stdafx.h" #include #include -#include class AutoConfigTest: public testing::Test {}; -TEST_F(AutoConfigTest, ExtractKeyTestWin) { - std::string type1("struct ConfigTypeExtractor"); - const auto key1 = autowiring::ExtractKey(type1); - ASSERT_EQ("Namespace1.XYZ", key1) << "Failed to properly extract Key with a single namespace"; - std::string type2("struct ConfigTypeExtractor"); - const auto key2 = autowiring::ExtractKey(type2); - ASSERT_EQ("Namespace1.Namespace2.XYZ", key2) << "Failed to properly extract Key with multiple namespaces"; - std::string type3("struct ConfigTypeExtractor"); - const auto key3 = autowiring::ExtractKey(type3); - ASSERT_EQ("XYZ", key3) << "Failed to properly extract Key with no namespace"; - std::string type4("struct ConfigTypeExtractor"); - const auto key4 = autowiring::ExtractKey(type4); - ASSERT_EQ("ClassNamespace1.AutoConfigTest.XYZ", key4) << "Failed to properly extract Key with class namespaces"; - std::string type5("struct ConfigTypeExtractor"); - const auto key5 = autowiring::ExtractKey(type5); - ASSERT_EQ("ClassNamespace1.Base::Nested.XYZ", key5) << "Failed to properly extract Key from nested"; -} - -TEST_F(AutoConfigTest, ExtractKeyTestPOSIX) { - std::string type1("ConfigTypeExtractor"); - const auto key1 = autowiring::ExtractKey(type1); - ASSERT_EQ("Namespace1.XYZ", key1) << "Failed to properly extract Key with a single namespace"; - - std::string type2("ConfigTypeExtractor"); - const auto key2 = autowiring::ExtractKey(type2); - ASSERT_EQ("Namespace1.Namespace2.XYZ", key2) << "Failed to properly extract Key with multiple namespaces"; - - std::string type3("ConfigTypeExtractor"); - const auto key3 = autowiring::ExtractKey(type3); - ASSERT_EQ("XYZ", key3) << "Failed to properly extract Key with no namespace"; - - std::string type4("ConfigTypeExtractor"); - const auto key4 = autowiring::ExtractKey(type4); - ASSERT_EQ("ClassNamespace1.AutoConfigTest.XYZ", key4) << "Failed to properly extract Key with class namespaces"; - - std::string type5("ConfigTypeExtractor"); - const auto key5 = autowiring::ExtractKey(type5); - ASSERT_EQ("ClassNamespace1.Base::Nested.XYZ", key5) << "Failed to properly extract Key from nested"; } struct MyConfigurableClass { @@ -60,13 +21,6 @@ struct MyConfigurableClass2 { AutoConfig m_myName; }; -TEST_F(AutoConfigTest, VerifyCorrectDeconstruction) { - AutoRequired mcc; - - EXPECT_STREQ("Namespace1.XYZ", mcc->m_myName.m_key.c_str()) - << "Configuration variable name was not correctly extracted"; -} - TEST_F(AutoConfigTest, VerifySimpleAssignment) { // Set an attribute in the manager before injecting anything: AutoRequired acm; diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index 56ef33af5..43401e6f6 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -2,6 +2,7 @@ set(AutowiringTest_SRCS AnySharedPointerTest.cpp ArgumentTypeTest.cpp AutoConfigTest.cpp + AutoConfigParserTest.cpp AutoConstructTest.cpp AutoFilterCollapseRulesTest.cpp AutoFilterDiagnosticsTest.cpp From ca0067d78e738f342a847a49a2ec91d0df580fb9 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 20:17:11 -0700 Subject: [PATCH 03/63] Make AutoConfigs hold their own value, remove references to AutoConfigManager from AutoConfig.h. Add some tests --- autowiring/AutoConfig.h | 41 +++++++++----------------- src/autowiring/AutoConfigManager.cpp | 1 + src/autowiring/test/AutoConfigTest.cpp | 14 ++++++++- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index cc7b5c405..3278b1c6e 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -1,7 +1,7 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once #include "Autowired.h" -#include "AutoConfigManager.h" +#include "auto_signal.h" #include "ConfigRegistry.h" #include @@ -20,6 +20,9 @@ class AutoConfigBase // Key used to identify this config value const std::string m_key; + + typedef autowiring::signal t_OnChangedSignal; + t_OnChangedSignal onChangedSignal; }; /// @@ -48,10 +51,6 @@ class AutoConfig: { // Register with config registry (void)RegConfig::r; - - if (!IsConfigured()){ - m_manager->Set(m_key, T(std::forward(arg), std::forward(args)...)); - } } AutoConfig() : @@ -61,33 +60,21 @@ class AutoConfig: (void)RegConfig::r; } -protected: - AutoRequired m_manager; - public: - const T& operator*() const { - return *m_manager->Get(m_key).template as().get(); - } - - const T* operator->(void) const { - return m_manager->Get(m_key)->template as().get(); - } + + operator const T&() const { return m_value; } void operator=(const T& newValue) { - return m_manager->Set(m_key, newValue); + m_value = newValue; } - /// - /// True if this configurable field has been satisfied with a value - /// - bool IsConfigured(void) const { - return m_manager->IsConfigured(m_key); - } - // Add a callback for when this config value changes - void operator+=(std::function&& fx) { - m_manager->AddCallback(m_key, [fx](const AnySharedPointer& val){ - fx(*val.template as().get()); - }); + t_OnChangedSignal::t_registration* operator+=(std::function&& fx) { + return onChangedSignal += [this,fx](){ + fx(m_value); + }; } + +private: + T m_value; }; diff --git a/src/autowiring/AutoConfigManager.cpp b/src/autowiring/AutoConfigManager.cpp index f7c4b3948..16ca12130 100644 --- a/src/autowiring/AutoConfigManager.cpp +++ b/src/autowiring/AutoConfigManager.cpp @@ -1,6 +1,7 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #include "stdafx.h" #include "AutoConfig.h" +#include "AutoConfigManager.h" #include "AnySharedPointer.h" #include diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index 5c42b125c..5e10bf3e8 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -1,16 +1,28 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #include "stdafx.h" #include -#include class AutoConfigTest: public testing::Test {}; +struct Namespace1 {}; +struct Namespace2 {}; +struct XYZ {}; +TEST_F(AutoConfigTest, VerifyBasicAssignment) { + AutoConfig cfg1; + cfg1 = 1; + ASSERT_EQ(cfg1, 1) << "Operator = failed"; + AutoConfig cfg2(2); + ASSERT_EQ(cfg2, 2) << "Default construction failed"; + AutoConfig cfg1a; + ASSERT_EQ(cfg1a, 1) << "Failed to autowire AutoConfig value"; + cfg1a = 3; + ASSERT_EQ(cfg1, 3) << "Failed to autowire AutoConfig value"; } struct MyConfigurableClass { From a05493b7540e41ca2bec4992cf2f10122b9909d2 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 20:26:14 -0700 Subject: [PATCH 04/63] Change AutoConfig to AutoConfigVar, make AutoConfig a wrapper for AutoRequired --- autowiring/AutoConfig.h | 60 ++++++++++++++++---------- src/autowiring/AutoConfig.cpp | 2 +- src/autowiring/test/AutoConfigTest.cpp | 17 +++++--- 3 files changed, 49 insertions(+), 30 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 3278b1c6e..3cdd182a3 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -13,48 +13,36 @@ struct AnySharedPointer; /// /// Utility base type for configuration members /// -class AutoConfigBase +class AutoConfigVarBase { public: - AutoConfigBase(const std::type_info& tiName); + AutoConfigVarBase(const std::type_info& tiName); // Key used to identify this config value const std::string m_key; - typedef autowiring::signal t_OnChangedSignal; + typedef autowiring::signal t_OnChangedSignal; t_OnChangedSignal onChangedSignal; }; -/// -/// Register an attribute with the AutoConfig system. For example -/// -/// AutoConfig m_myVal; -/// defines the key "MyNamespace.MyKey" -/// -/// The Namespace field is optional, so -/// AutoConfig m_myVal; -/// defines the key "MyKey" -/// -/// AutoConfig values can be set from the AutoConfigManager. The string key -/// is used as the identifier for the value -/// template -class AutoConfig: - public AutoConfigBase +class AutoConfigVar: + public AutoConfigVarBase { public: static_assert(sizeof...(TKey) >= 1, "Must provide a key and optionally at least one namespace"); template - AutoConfig(t_Arg &&arg, t_Args&&... args) : - AutoConfigBase(typeid(ConfigTypeExtractor)) + AutoConfigVar(t_Arg &&arg, t_Args&&... args) : + AutoConfigVarBase(typeid(ConfigTypeExtractor)), + m_value(std::forward(arg), std::forward(args)...) { // Register with config registry (void)RegConfig::r; } - AutoConfig() : - AutoConfigBase(typeid(ConfigTypeExtractor)) + AutoConfigVar() : + AutoConfigVarBase(typeid(ConfigTypeExtractor)) { // Register with config registry (void)RegConfig::r; @@ -78,3 +66,31 @@ class AutoConfig: private: T m_value; }; + +/// +/// Register an attribute with the AutoConfig system. For example +/// +/// AutoConfig m_myVal; +/// defines the key "MyNamespace.MyKey" +/// +/// The Namespace field is optional, so +/// AutoConfig m_myVal; +/// defines the key "MyKey" +/// +/// AutoConfig values can be set from the AutoConfigManager. The string key +/// is used as the identifier for the value +/// +template +class AutoConfig : public AutoRequired> { +public: + AutoConfig(const std::shared_ptr& ctxt = CoreContext::CurrentContext()) : + AutoRequired>(ctxt) + { + } + + template + AutoConfig(const std::shared_ptr& ctxt, t_Args&&... args) : + AutoRequired>(ctxt, std::forward(args)...) + { + } +}; \ No newline at end of file diff --git a/src/autowiring/AutoConfig.cpp b/src/autowiring/AutoConfig.cpp index e958ebe5f..3179dab45 100644 --- a/src/autowiring/AutoConfig.cpp +++ b/src/autowiring/AutoConfig.cpp @@ -3,6 +3,6 @@ #include "AutoConfig.h" #include "AutoConfigParser.hpp" -AutoConfigBase::AutoConfigBase(const std::type_info& ti): +AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti) : m_key(autowiring::ExtractKey(ti)) {} diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index 5e10bf3e8..d8c6929a2 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -12,17 +12,20 @@ struct XYZ {}; TEST_F(AutoConfigTest, VerifyBasicAssignment) { AutoConfig cfg1; - cfg1 = 1; - ASSERT_EQ(cfg1, 1) << "Operator = failed"; + *cfg1 = 1; + ASSERT_EQ(*cfg1, 1) << "Operator = failed"; - AutoConfig cfg2(2); - ASSERT_EQ(cfg2, 2) << "Default construction failed"; + AutoConfig cfg2(CoreContext::CurrentContext(), 2); + ASSERT_EQ(*cfg2, 2) << "Default construction failed"; AutoConfig cfg1a; - ASSERT_EQ(cfg1a, 1) << "Failed to autowire AutoConfig value"; + ASSERT_EQ(*cfg1a, 1) << "Failed to autowire AutoConfig value"; - cfg1a = 3; - ASSERT_EQ(cfg1, 3) << "Failed to autowire AutoConfig value"; + *cfg1a = 3; + ASSERT_EQ(*cfg1, 3) << "Failed to autowire AutoConfig value"; + + AutoConfig cfg2a(CoreContext::CurrentContext(), 5); + ASSERT_EQ(*cfg2a, 2) << "Constructor overwrote value when it wasn't supposed to!"; } struct MyConfigurableClass { From 5f1d61f84d75927cda9f02ef450d16f89884c5b7 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 21:00:36 -0700 Subject: [PATCH 05/63] Add generic Get/Set interface to AutoConfigVarBase --- autowiring/AutoConfig.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 3cdd182a3..5bde392c6 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -20,6 +20,10 @@ class AutoConfigVarBase // Key used to identify this config value const std::string m_key; + + // Generic interface functions for getting and setting + virtual void Get(void* pValue) const = 0; + virtual void Set(const void* pValue) = 0; typedef autowiring::signal t_OnChangedSignal; t_OnChangedSignal onChangedSignal; @@ -56,6 +60,9 @@ class AutoConfigVar: m_value = newValue; } + void Get(void* pValue) const override { *reinterpret_cast(pValue) = m_value; } + void Set(const void* pValue) override { m_value = *reinterpret_cast(pValue); } + // Add a callback for when this config value changes t_OnChangedSignal::t_registration* operator+=(std::function&& fx) { return onChangedSignal += [this,fx](){ From 7b9bda352734ee5c46d5baee5daf9b4f1b33eb36 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 20:54:24 -0700 Subject: [PATCH 06/63] Add signal for on changed. --- autowiring/AutoConfig.h | 17 +++++++-- src/autowiring/AutoConfig.cpp | 1 + src/autowiring/test/AutoConfigTest.cpp | 51 +++++++++++++++++++------- 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 5bde392c6..d62b99eec 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -43,6 +43,8 @@ class AutoConfigVar: { // Register with config registry (void)RegConfig::r; + + onChangedSignal(*this); } AutoConfigVar() : @@ -58,6 +60,7 @@ class AutoConfigVar: void operator=(const T& newValue) { m_value = newValue; + onChangedSignal(*this); } void Get(void* pValue) const override { *reinterpret_cast(pValue) = m_value; } @@ -65,11 +68,13 @@ class AutoConfigVar: // Add a callback for when this config value changes t_OnChangedSignal::t_registration* operator+=(std::function&& fx) { - return onChangedSignal += [this,fx](){ - fx(m_value); + return onChangedSignal += [fx](const AutoConfigVarBase& var){ + fx(reinterpret_cast*>(&var)->m_value); }; } + void operator-=(t_OnChangedSignal::t_registration* node) { onChangedSignal -= node; } + private: T m_value; }; @@ -90,14 +95,18 @@ class AutoConfigVar: template class AutoConfig : public AutoRequired> { public: + typedef AutoConfigVar t_Var; + AutoConfig(const std::shared_ptr& ctxt = CoreContext::CurrentContext()) : - AutoRequired>(ctxt) + AutoRequired(ctxt) { } template AutoConfig(const std::shared_ptr& ctxt, t_Args&&... args) : - AutoRequired>(ctxt, std::forward(args)...) + AutoRequired(ctxt, std::forward(args)...) { } + + using AutoRequired::operator*; }; \ No newline at end of file diff --git a/src/autowiring/AutoConfig.cpp b/src/autowiring/AutoConfig.cpp index 3179dab45..cdb3565c4 100644 --- a/src/autowiring/AutoConfig.cpp +++ b/src/autowiring/AutoConfig.cpp @@ -5,4 +5,5 @@ AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti) : m_key(autowiring::ExtractKey(ti)) + onChangedSignal(), {} diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index d8c6929a2..bc5572e91 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -28,22 +28,47 @@ TEST_F(AutoConfigTest, VerifyBasicAssignment) { ASSERT_EQ(*cfg2a, 2) << "Constructor overwrote value when it wasn't supposed to!"; } -struct MyConfigurableClass { - AutoConfig m_myName; -}; +TEST_F(AutoConfigTest, VerifyBasicSignals) { + AutoCurrentContext ctxt; + + int handler_called = 0; + int handler_value_read = 0; + Autowired> autoCfg; + autoCfg(&AutoConfigVar::onChangedSignal) += [&](const AutoConfigVarBase& var) { + ++handler_called; + var.Get(&handler_value_read); + }; -struct MyConfigurableClass2 { - AutoConfig m_myName; -}; + AutoConfig cfg1(CoreContext::CurrentContext(), 1); + ASSERT_EQ(handler_called, 1) << "OnChanged not triggered by construction"; + ASSERT_EQ(handler_value_read, 1) << "Bad value read in OnChanged"; -TEST_F(AutoConfigTest, VerifySimpleAssignment) { - // Set an attribute in the manager before injecting anything: - AutoRequired acm; - acm->Set("Namespace1.XYZ", 323); + *cfg1 = 20; + ASSERT_EQ(handler_called, 2) << "OnChanged not triggered by assignement"; + ASSERT_EQ(handler_value_read, 20) << "Bad value read in OnChanged"; - // Now inject the type which expects this value to be assigned: - AutoRequired mcc; - ASSERT_EQ(323, *mcc->m_myName) << "Configurable type did not receive a value as expected"; + AutoConfig cfg2(CoreContext::CurrentContext()); + *cfg2 = 2; + + ASSERT_EQ(handler_called, 3) << "OnChanged not triggred by assignement from alternate autowired instance"; + ASSERT_EQ(handler_value_read, 2) << "Bad value read in OnChanged"; + + int handler_direct_called = 0; + int handler_direct_value = 0; + auto* registration = *cfg2 += [&](const int& var) { + ++handler_direct_called; + handler_direct_value = var; + }; + + *cfg1 = 30; + ASSERT_EQ(handler_direct_called, 1) << "OnChanged not triggred by indirect +="; + ASSERT_EQ(handler_direct_value, 30) << "Bad value read in OnChanged"; + + *cfg1 -= registration; + + *cfg1 = 123; + ASSERT_EQ(handler_direct_called, 1) << "OnChanged not unsubscribed by indirect -="; + ASSERT_EQ(handler_direct_value, 30) << "Bad value read in OnChanged"; } struct NamespaceRoot; From 3c115e45ca819ecce7eaf10cc770c55a90566924 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 23:09:42 -0700 Subject: [PATCH 07/63] make AutoConfigVar a ContextMember, add IsConfigured --- autowiring/AutoConfig.h | 24 +++++++++++++++++------- src/autowiring/AutoConfig.cpp | 7 +++++-- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index d62b99eec..f231924a6 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -13,10 +13,10 @@ struct AnySharedPointer; /// /// Utility base type for configuration members /// -class AutoConfigVarBase +class AutoConfigVarBase : public ContextMember { public: - AutoConfigVarBase(const std::type_info& tiName); + AutoConfigVarBase(const std::type_info& tiName, bool configured = false); // Key used to identify this config value const std::string m_key; @@ -27,6 +27,11 @@ class AutoConfigVarBase typedef autowiring::signal t_OnChangedSignal; t_OnChangedSignal onChangedSignal; + + bool IsConfigured() const { return m_isConfigured; } + +protected: + bool m_isConfigured; }; template @@ -38,7 +43,7 @@ class AutoConfigVar: template AutoConfigVar(t_Arg &&arg, t_Args&&... args) : - AutoConfigVarBase(typeid(ConfigTypeExtractor)), + AutoConfigVarBase(typeid(ConfigTypeExtractor), true), m_value(std::forward(arg), std::forward(args)...) { // Register with config registry @@ -48,7 +53,8 @@ class AutoConfigVar: } AutoConfigVar() : - AutoConfigVarBase(typeid(ConfigTypeExtractor)) + AutoConfigVarBase(typeid(ConfigTypeExtractor)), + m_value() { // Register with config registry (void)RegConfig::r; @@ -59,12 +65,11 @@ class AutoConfigVar: operator const T&() const { return m_value; } void operator=(const T& newValue) { - m_value = newValue; - onChangedSignal(*this); + SetInternal(newValue); } void Get(void* pValue) const override { *reinterpret_cast(pValue) = m_value; } - void Set(const void* pValue) override { m_value = *reinterpret_cast(pValue); } + void Set(const void* pValue) override { *this = *reinterpret_cast(pValue); } // Add a callback for when this config value changes t_OnChangedSignal::t_registration* operator+=(std::function&& fx) { @@ -77,6 +82,11 @@ class AutoConfigVar: private: T m_value; + void SetInternal(const T& val) { + m_isConfigured = true; + m_value = val; + onChangedSignal(*this); + } }; /// diff --git a/src/autowiring/AutoConfig.cpp b/src/autowiring/AutoConfig.cpp index cdb3565c4..f1809d17a 100644 --- a/src/autowiring/AutoConfig.cpp +++ b/src/autowiring/AutoConfig.cpp @@ -3,7 +3,10 @@ #include "AutoConfig.h" #include "AutoConfigParser.hpp" -AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti) : - m_key(autowiring::ExtractKey(ti)) +AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti, bool configured) : + m_key(autowiring::ExtractKey(ti)), + m_isConfigured(configured), onChangedSignal(), + ContextMember(m_key.c_str()) {} + From a59905225e23734ced119fa521ab2ca44f69e0e4 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 23 Mar 2015 23:38:15 -0700 Subject: [PATCH 08/63] Add inheritance support. --- autowiring/AutoConfig.h | 28 +++++++ src/autowiring/AutoConfig.cpp | 1 + src/autowiring/test/AutoConfigTest.cpp | 89 +++++++++-------------- src/autowiring/test/AutoParameterTest.cpp | 3 +- 4 files changed, 65 insertions(+), 56 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index f231924a6..d5d795078 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -29,9 +29,11 @@ class AutoConfigVarBase : public ContextMember t_OnChangedSignal onChangedSignal; bool IsConfigured() const { return m_isConfigured; } + bool IsInherited() const { return m_parentRegistration != nullptr; } protected: bool m_isConfigured; + t_OnChangedSignal::t_registration* m_parentRegistration; }; template @@ -58,6 +60,25 @@ class AutoConfigVar: { // Register with config registry (void)RegConfig::r; + + const auto ctxt = m_context.lock(); + if (!ctxt) + return; + + //This will wind up being recursive + auto parent = ctxt->GetParentContext(); + if (parent != nullptr) { + auto parentVar = parent->Inject>(); + + //Only copy the value if it's initalized + if (parentVar->IsConfigured()) { + SetInternal(parentVar->m_value); + } + + m_parentRegistration = *parentVar += [this](const T& val){ + SetInternal(val); + }; + } } public: @@ -65,6 +86,12 @@ class AutoConfigVar: operator const T&() const { return m_value; } void operator=(const T& newValue) { + if (m_parentRegistration) { + auto parent_ctxt = m_context.lock()->GetParentContext(); + AutowiredFast> parentVar(parent_ctxt); + *parentVar -= m_parentRegistration; + m_parentRegistration = nullptr; + } SetInternal(newValue); } @@ -82,6 +109,7 @@ class AutoConfigVar: private: T m_value; + void SetInternal(const T& val) { m_isConfigured = true; m_value = val; diff --git a/src/autowiring/AutoConfig.cpp b/src/autowiring/AutoConfig.cpp index f1809d17a..4af78513f 100644 --- a/src/autowiring/AutoConfig.cpp +++ b/src/autowiring/AutoConfig.cpp @@ -6,6 +6,7 @@ AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti, bool configured) : m_key(autowiring::ExtractKey(ti)), m_isConfigured(configured), + m_parentRegistration(nullptr), onChangedSignal(), ContextMember(m_key.c_str()) {} diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index bc5572e91..b18a18025 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -71,66 +71,44 @@ TEST_F(AutoConfigTest, VerifyBasicSignals) { ASSERT_EQ(handler_direct_value, 30) << "Bad value read in OnChanged"; } -struct NamespaceRoot; -struct NamespaceChild; - -TEST_F(AutoConfigTest, VerifyNestedNamespace) { - AutoRequired acm; - acm->Set("NamespaceRoot.NamespaceChild.Namespace1.Namespace2.XYZ", 142); - - AutoConfig cfg; - ASSERT_EQ(142, *cfg); -} - -struct MyBoolClass { - AutoConfig m_bool; -}; - -TEST_F(AutoConfigTest, VerifyBool) { - AutoRequired acm; - AutoRequired clz1; - - acm->Set("bool_space.my_bool", true); - ASSERT_TRUE(*clz1->m_bool); - - acm->SetParsed("bool_space.my_bool", "false"); - ASSERT_FALSE(*clz1->m_bool); -} - -TEST_F(AutoConfigTest, VerifyPostHocAssignment) { - // Inject the configurable type first - AutoRequired mcc; - - // Configuration manager must exist at this point as a consequence of the earlier construction - Autowired acm; - ASSERT_TRUE(acm.IsAutowired()) << "AutoConfig field did not inject a configuration manager into this context as expected"; - - acm->Set("Namespace1.XYZ", 323); - - // Now inject the type which expects this value to be assigned: - ASSERT_EQ(323, *mcc->m_myName) << "Configurable type did not receive a value as expected"; -} - -TEST_F(AutoConfigTest, VerifyRecursiveSearch) { - AutoRequired acm; - acm->Set("Namespace1.XYZ", 1001); +TEST_F(AutoConfigTest, VerifyBasicInheritance) { + AutoConfig cfg_outer; + *cfg_outer = 1001; { - AutoCreateContext ctxt; - CurrentContextPusher pshr(ctxt); - - // Now inject an element here, and verify that it was wired up as expected: - AutoRequired mcc; - ASSERT_TRUE(mcc->m_myName.IsConfigured()) << "A configurable value was not configured as expected"; - ASSERT_EQ(1001, *mcc->m_myName) << "Configurable value obtained from a parent scope did not have the correct value"; - - // This must work as expected--a local context override will rewrite configuration values in the local scope - AutoRequired sub_mcc; - sub_mcc->Set("Namespace1.XYZ", 1002); - ASSERT_EQ(1002, *mcc->m_myName) << "Override of a configurable value in a derived class did not take place as expected"; + AutoCreateContext ctxt_middle; + CurrentContextPusher pshr(ctxt_middle); + AutoConfig cfg_middle; + ASSERT_EQ(*cfg_middle, 1001) << "Configurable value obtained from parent context did not have the correct value"; + + { + AutoCreateContext ctxt_inner; + CurrentContextPusher pshr(ctxt_inner); + + // Now inject an element here, and verify that it was wired up as expected: + AutoConfig cfg_inner; + ASSERT_EQ(1001, *cfg_inner) << "Configurable value obtained from a parent context did not have the correct value"; + + // Now change the parent value.... + *cfg_outer = 1002; + ASSERT_EQ(*cfg_inner, 1002) << "Configuration value did not update when value in a parent was updated"; + + // Now change the value in the intervening context... + *cfg_middle = 1003; + ASSERT_EQ(*cfg_inner, 1003) << "Configuration value did not update when value in a parent was updated"; + + // This must work as expected--a local context override will rewrite configuration values in the local scope + *cfg_inner = 1004; + ASSERT_EQ(*cfg_inner, 1004) << "Override of a configurable value in a derived class did not take place as expected"; + } + ASSERT_EQ(*cfg_middle, 1003) << "A configuration value was not updated when accessed directly."; } + + //Modification of the value in the enclosed context should not effect us. + ASSERT_EQ(1002, *cfg_outer) << "Override of a configurable value in a child scope affected parent"; } +/* struct DefaultName { AutoConfig m_def; }; @@ -406,3 +384,4 @@ TEST_F(AutoConfigTest, ListingConfigs) { auto keys_inner = acm_inner->GetLocalKeys(); ASSERT_EQ(var1_inner->m_myName.m_key, keys_inner[0]) << "Keys listed out of construction order"; } +*/ diff --git a/src/autowiring/test/AutoParameterTest.cpp b/src/autowiring/test/AutoParameterTest.cpp index a06122ebc..310565ac8 100644 --- a/src/autowiring/test/AutoParameterTest.cpp +++ b/src/autowiring/test/AutoParameterTest.cpp @@ -7,7 +7,7 @@ class AutoParameterTest: public testing::Test {}; - +/* struct MyParamClass1 { struct MyIntParam1 { static int Default() { return 15; } @@ -156,3 +156,4 @@ TEST_F(AutoParameterTest, VerifyDefaultMinMaxKey) { ASSERT_TRUE(param.Set(10) && *param == 10) << "Should be able to set values that are valid according to the validation function"; } +*/ \ No newline at end of file From 7421e52f7fe76a19a937177786a7f0796319e08f Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Tue, 24 Mar 2015 19:58:19 -0700 Subject: [PATCH 09/63] Add SetParsed method --- autowiring/AutoConfig.h | 9 ++- autowiring/ConfigRegistry.h | 8 +-- src/autowiring/test/AutoConfigTest.cpp | 87 +++++--------------------- 3 files changed, 26 insertions(+), 78 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index d5d795078..3007eccea 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -24,7 +24,9 @@ class AutoConfigVarBase : public ContextMember // Generic interface functions for getting and setting virtual void Get(void* pValue) const = 0; virtual void Set(const void* pValue) = 0; - + + virtual void SetParsed(const std::string& value) = 0; + typedef autowiring::signal t_OnChangedSignal; t_OnChangedSignal onChangedSignal; @@ -98,6 +100,11 @@ class AutoConfigVar: void Get(void* pValue) const override { *reinterpret_cast(pValue) = m_value; } void Set(const void* pValue) override { *this = *reinterpret_cast(pValue); } + void SetParsed(const std::string& value) override { + auto entry = RegConfig::r; + *this = entry.parseInternal(value); + } + // Add a callback for when this config value changes t_OnChangedSignal::t_registration* operator+=(std::function&& fx) { return onChangedSignal += [fx](const AutoConfigVarBase& var){ diff --git a/autowiring/ConfigRegistry.h b/autowiring/ConfigRegistry.h index 7420b0449..c598e3902 100644 --- a/autowiring/ConfigRegistry.h +++ b/autowiring/ConfigRegistry.h @@ -88,13 +88,13 @@ struct ConfigRegistryEntryT: // Parse string into this ConfigEntry's type. Throw an exception // if no such stream operator exists AnySharedPointer parse(const std::string& str) const override { - return parseInternal(str); + return AnySharedPointer(std::make_shared(std::move(parseInternal(str)))); } public: // Only use if there is a stream operator template - typename std::enable_if::value, AnySharedPointer>::type + typename std::enable_if::value, T>::type parseInternal(const std::string& str) const { std::istringstream ss(str); T val; @@ -102,12 +102,12 @@ struct ConfigRegistryEntryT: if (ss.fail()) autowiring::ThrowFailedTypeParseException(str, typeid(T)); - return AnySharedPointer(std::make_shared(val)); + return val; } // Throw exception if there is no stream operator template - typename std::enable_if::value, AnySharedPointer>::type + typename std::enable_if::value, T>::type parseInternal(const std::string&) const { throw autowiring_error("This type doesn't support stream conversions. Define one if you want this to be parsable"); }; diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index b18a18025..07461cf37 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -108,90 +108,31 @@ TEST_F(AutoConfigTest, VerifyBasicInheritance) { ASSERT_EQ(1002, *cfg_outer) << "Override of a configurable value in a child scope affected parent"; } -/* -struct DefaultName { - AutoConfig m_def; +struct Unparseable { + int v; }; -TEST_F(AutoConfigTest, DefaultNamespace) { - AutoRequired acm; - acm->Set("defaultname1", 123); - - AutoRequired def; - - ASSERT_EQ(123, *def->m_def); -} - TEST_F(AutoConfigTest, VerifyParsedAssignment) { // We must also be able to support implicit string-to-type conversion via the shift operator for this type - AutoRequired acm; + AutoConfig cfg; // Direct assignment to a string should not work, the type isn't a string it's an int - ASSERT_ANY_THROW(acm->Set("Namespace1.XYZ", "327")) << "An attempt to assign a value to an unrelated type did not generate an exception as expected"; - - ASSERT_ANY_THROW(acm->Set("Namespace1.XYZ", 3.0)) << "An attempt to assign a value to an unrelated type did not generate an exception as expected"; + ASSERT_ANY_THROW(cfg->SetParsed("badvalue")) << "An attempt to assign a value to an unparsable string did not throw an exception"; + ASSERT_FALSE(cfg->IsConfigured()) << "A failed parse incorrectly marked the value as configured"; // Assignment to a string type should result in an appropriate coercion to the right value - ASSERT_TRUE(acm->SetParsed("Namespace1.XYZ", "324")); + cfg->SetParsed("324"); + ASSERT_EQ(*cfg, 324); + ASSERT_TRUE(cfg->IsConfigured()) << "Assignement from a parsed value falied to mark the value as configured"; + + AutoConfig noparse; + ASSERT_ANY_THROW(cfg->SetParsed("anyvalue")) << "An attempt to parse a value into an unparsable structure did not throw an exception"; + *noparse = Unparseable{ 20 }; + ASSERT_EQ((*noparse)->v, 20); } - -TEST_F(AutoConfigTest, VerifyDuplicateConfigAssignment) { - AutoRequired acm; - ASSERT_TRUE(acm->SetParsed("Namespace1.XYZ", "324")); - ASSERT_TRUE(acm->SetParsed("Namespace2.XYZ", "1111")); - - AutoRequired clz1; - AutoRequired clz2; - - ASSERT_EQ(324, *clz1->m_myName); - ASSERT_EQ(1111, *clz2->m_myName); -} - -class TypeWithoutAShiftOperator { -public: - int foo; -}; - -struct NoShift { - AutoConfig m_noshift; +/* }; -static_assert(has_stream::value, "Stream operation not detected on a primitive type"); -static_assert(!has_stream::value, "Stream operation detected on a type that should not have supported it"); - -TEST_F(AutoConfigTest, TypeWithoutAShiftOperatorTest) { - AutoRequired noshift; - - AutoRequired mgr; - - // Indirect assignment should cause an exception - ASSERT_ANY_THROW(mgr->Set("MyNoShift", "")) << "Expected a throw in a case where a configurable value was used which cannot be assigned"; - - // Direct assignment should be supported still - TypeWithoutAShiftOperator tasoVal; - tasoVal.foo = 592; - mgr->Set("MyNoShift", tasoVal); - - ASSERT_EQ(592, noshift->m_noshift->foo) << "Value assignment did not result in an update to a non-serializable configuration field"; -} - -TEST_F(AutoConfigTest, Callbacks) { - AutoRequired acm; - AutoRequired mcc; - - acm->Set("Namespace1.XYZ", 4); - - mcc->m_myName += [](int val) { - ASSERT_EQ(val, 42); - }; - - mcc->m_myName += [](int val) { - ASSERT_EQ(val, 42); - }; - - acm->Set("Namespace1.XYZ", 42); -} - TEST_F(AutoConfigTest, NestedContexts) { // Set up contexts and members AutoCurrentContext ctxt_outer; From 8b918d422941268463ec2da9a7135a3c34bacf15 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Tue, 24 Mar 2015 20:34:27 -0700 Subject: [PATCH 10/63] Add tests for complex nested context case. --- autowiring/AutoConfig.h | 1 + src/autowiring/test/AutoConfigTest.cpp | 54 +++++++++++++------------- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 3007eccea..c7e04be70 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -86,6 +86,7 @@ class AutoConfigVar: public: operator const T&() const { return m_value; } + const T* operator->() const { return &m_value; } void operator=(const T& newValue) { if (m_parentRegistration) { diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index 07461cf37..bbcd851c0 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -130,7 +130,9 @@ TEST_F(AutoConfigTest, VerifyParsedAssignment) { *noparse = Unparseable{ 20 }; ASSERT_EQ((*noparse)->v, 20); } -/* + +struct MyConfigurableClass { + AutoConfig cfg; }; TEST_F(AutoConfigTest, NestedContexts) { @@ -148,55 +150,51 @@ TEST_F(AutoConfigTest, NestedContexts) { AutoRequired mcc_inner(ctxt_inner); AutoRequired mcc_leaf(ctxt_leaf); - AutoRequired acm_outer(ctxt_outer); - AutoRequired acm_middle(ctxt_middle); - AutoRequired acm_leaf(ctxt_leaf); - // Set initial value - acm_outer->Set("Namespace1.XYZ", 42); - ASSERT_EQ(42, *mcc_outer->m_myName) << "Config value not set"; - ASSERT_EQ(42, *mcc_middle->m_myName) << "Config value not set in descendant context"; - ASSERT_EQ(42, *mcc_sibling->m_myName) << "Config value not set in descendant context"; - ASSERT_EQ(42, *mcc_inner->m_myName) << "Config value not set in descendant context"; - ASSERT_EQ(42, *mcc_leaf->m_myName) << "Config value not set in desendant context"; - EXPECT_TRUE(acm_middle->IsInherited("Namespace1.XYZ")) << "Inherited key not marked as such"; - EXPECT_TRUE(acm_leaf->IsInherited("Namespace1.XYZ")) << "Inherited key not marked as such"; + *mcc_outer->cfg = 42; + ASSERT_EQ(42, *mcc_outer->cfg) << "Config value not set"; + ASSERT_EQ(42, *mcc_middle->cfg) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_sibling->cfg) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_inner->cfg) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_leaf->cfg) << "Config value not set in desendant context"; + EXPECT_TRUE(mcc_middle->cfg->IsInherited()) << "Inherited key not marked as such"; + EXPECT_TRUE(mcc_leaf->cfg->IsInherited()) << "Inherited key not marked as such"; // Set middle, inner shouldn't be able to be set from outer after this bool callback_hit1 = false; - mcc_inner->m_myName += [&callback_hit1](int) { + *mcc_inner->cfg += [&callback_hit1](int) { callback_hit1 = true; }; - acm_middle->Set("Namespace1.XYZ", 1337); - ASSERT_EQ(42, *mcc_outer->m_myName) << "Config value changed in outer context"; - ASSERT_EQ(42, *mcc_sibling->m_myName) << "Config value set from sibling context"; - ASSERT_EQ(1337, *mcc_middle->m_myName) << "Config value not set"; - ASSERT_EQ(1337, *mcc_inner->m_myName) << "Config value not set in child context"; - ASSERT_EQ(1337, *mcc_leaf->m_myName) << "Config value not set in leaf context"; + *mcc_middle->cfg = 1337; + ASSERT_EQ(42, *mcc_outer->cfg) << "Config value changed in outer context"; + ASSERT_EQ(42, *mcc_sibling->cfg) << "Config value set from sibling context"; + ASSERT_EQ(1337, *mcc_middle->cfg) << "Config value not set"; + ASSERT_EQ(1337, *mcc_inner->cfg) << "Config value not set in child context"; + ASSERT_EQ(1337, *mcc_leaf->cfg) << "Config value not set in leaf context"; ASSERT_TRUE(callback_hit1) << "Callback not hit in inner context"; // Set from outter, inner should be shielded by middle context - mcc_inner->m_myName += [](int) { + *mcc_inner->cfg += [](int) { FAIL() << "This callback should never be hit"; }; // Make sure sibling context is not shielded bool callback_hit2 = false; - mcc_sibling->m_myName += [&callback_hit2](int) { + *mcc_sibling->cfg += [&callback_hit2](int) { callback_hit2 = true; }; // Set from outer, shouldn't effect middle or inner contexts - acm_outer->Set("Namespace1.XYZ", 999); - ASSERT_EQ(999, *mcc_outer->m_myName) << "Config value not set"; - ASSERT_EQ(1337, *mcc_middle->m_myName) << "Config value overwritten when value was set in this context"; - ASSERT_EQ(1337, *mcc_inner->m_myName) << "Config value overwritten when value was set in parent context"; + *mcc_outer->cfg = 999; + ASSERT_EQ(999, *mcc_outer->cfg) << "Config value not set"; + ASSERT_EQ(1337, *mcc_middle->cfg) << "Config value overwritten when value was set in this context"; + ASSERT_EQ(1337, *mcc_inner->cfg) << "Config value overwritten when value was set in parent context"; // Make sure sibling hit - ASSERT_EQ(999, *mcc_sibling->m_myName) << "Value not set on sibling of context where value was previously set"; + ASSERT_EQ(999, *mcc_sibling->cfg) << "Value not set on sibling of context where value was previously set"; ASSERT_TRUE(callback_hit2) << "Callback not called on sibling of context where value was previously set"; } - +/* struct ValidatedKey{ static bool Validate(const int& value) { return value > 5; From 7eba155e5ef9241918f16955f69c752899039c1a Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Tue, 24 Mar 2015 21:13:42 -0700 Subject: [PATCH 11/63] Add Validator functionality --- autowiring/AutoConfig.h | 18 ++++++++++++++++-- autowiring/AutoConfigManager.h | 2 +- autowiring/ConfigRegistry.h | 8 +++++++- src/autowiring/AutoConfigManager.cpp | 21 ++++++++++----------- src/autowiring/ConfigRegistry.cpp | 2 +- src/autowiring/test/AutoConfigTest.cpp | 14 +++++--------- 6 files changed, 40 insertions(+), 25 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index c7e04be70..e98c9a413 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -51,7 +51,13 @@ class AutoConfigVar: m_value(std::forward(arg), std::forward(args)...) { // Register with config registry - (void)RegConfig::r; + auto config = RegConfig::r; + + if (config.m_hasValidator) { + if (!config.validatorInternal()(m_value)) { + throw autowiring_error("Cannot construct AutoConfigVar with a value that fails validation"); + } + } onChangedSignal(*this); } @@ -89,13 +95,13 @@ class AutoConfigVar: const T* operator->() const { return &m_value; } void operator=(const T& newValue) { + SetInternal(newValue); if (m_parentRegistration) { auto parent_ctxt = m_context.lock()->GetParentContext(); AutowiredFast> parentVar(parent_ctxt); *parentVar -= m_parentRegistration; m_parentRegistration = nullptr; } - SetInternal(newValue); } void Get(void* pValue) const override { *reinterpret_cast(pValue) = m_value; } @@ -119,6 +125,14 @@ class AutoConfigVar: T m_value; void SetInternal(const T& val) { + auto config = RegConfig::r; + + if (config.m_hasValidator) { + if (!config.validatorInternal()(val)) { + throw autowiring_error("Validator rejected set for config value"); + } + } + m_isConfigured = true; m_value = val; onChangedSignal(*this); diff --git a/autowiring/AutoConfigManager.h b/autowiring/AutoConfigManager.h index 03a34a5ac..57b20f301 100644 --- a/autowiring/AutoConfigManager.h +++ b/autowiring/AutoConfigManager.h @@ -32,7 +32,7 @@ class AutoConfigManager: static const std::unordered_map s_registry; // map of validators registered for a key - static const std::unordered_map> s_validators; + static const std::unordered_map s_validators; // lock for all members std::mutex m_lock; diff --git a/autowiring/ConfigRegistry.h b/autowiring/ConfigRegistry.h index c598e3902..2f5c461a7 100644 --- a/autowiring/ConfigRegistry.h +++ b/autowiring/ConfigRegistry.h @@ -45,7 +45,7 @@ struct ConfigRegistryEntry { const std::string m_key; // True if a validator was provided - const bool m_has_validator; + const bool m_hasValidator; // Is this key identify this entry? bool is(const std::string& key) const; @@ -112,6 +112,12 @@ struct ConfigRegistryEntryT: throw autowiring_error("This type doesn't support stream conversions. Define one if you want this to be parsable"); }; + std::function validatorInternal(void) { + return[](const T& ptr) { + return CallValidate::Call(ptr); + }; + } + std::function validator(void) const override { return [] (const AnySharedPointer& ptr) { return CallValidate::Call(*ptr.as().get()); diff --git a/src/autowiring/AutoConfigManager.cpp b/src/autowiring/AutoConfigManager.cpp index 16ca12130..ec12c6676 100644 --- a/src/autowiring/AutoConfigManager.cpp +++ b/src/autowiring/AutoConfigManager.cpp @@ -19,12 +19,12 @@ static std::unordered_map FillRegistry( } // Create map of all validators specified -static std::unordered_map> FillValidators(void) { - std::unordered_map> validator; +static std::unordered_map FillValidators(void) { + std::unordered_map validator; for (auto config = g_pFirstConfigEntry; config; config = config->pFlink) { - if (config->m_has_validator) { - validator[config->m_key].push_back(config->validator()); + if (config->m_hasValidator) { + validator[config->m_key] = config->validator(); } } @@ -32,7 +32,7 @@ static std::unordered_map AutoConfigManager::s_registry = FillRegistry(); -const std::unordered_map> AutoConfigManager::s_validators = FillValidators(); +const std::unordered_map AutoConfigManager::s_validators = FillValidators(); AutoConfigManager::AutoConfigManager(void){ // Copy parent's config settings @@ -119,12 +119,11 @@ void AutoConfigManager::AddCallback(t_add_callback&& fx) { void AutoConfigManager::SetRecursive(const std::string& key, AnySharedPointer value) { // Call all validators for this key if (s_validators.count(key)) { - for (auto const& fx : s_validators.find(key)->second) { - if (!fx(value)) { - std::stringstream ss; - ss << "Attempted to set key '" << key << "'which didin't pass validator"; - throw autowiring_error(ss.str()); - } + auto const& fx = s_validators.find(key)->second; + if (!fx(value)) { + std::stringstream ss; + ss << "Attempted to set key '" << key << "'which didin't pass validator"; + throw autowiring_error(ss.str()); } } diff --git a/src/autowiring/ConfigRegistry.cpp b/src/autowiring/ConfigRegistry.cpp index 4e23468fe..973a0e5c2 100644 --- a/src/autowiring/ConfigRegistry.cpp +++ b/src/autowiring/ConfigRegistry.cpp @@ -11,7 +11,7 @@ size_t g_configEntryCount = 0; ConfigRegistryEntry::ConfigRegistryEntry(const std::type_info& tinfo, bool has_validator) : pFlink(g_pFirstConfigEntry), m_key(autowiring::ExtractKey(tinfo)), - m_has_validator(has_validator) + m_hasValidator(has_validator) { g_configEntryCount++; g_pFirstConfigEntry = this; diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index bbcd851c0..a8142d601 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -194,7 +194,7 @@ TEST_F(AutoConfigTest, NestedContexts) { ASSERT_EQ(999, *mcc_sibling->cfg) << "Value not set on sibling of context where value was previously set"; ASSERT_TRUE(callback_hit2) << "Callback not called on sibling of context where value was previously set"; } -/* + struct ValidatedKey{ static bool Validate(const int& value) { return value > 5; @@ -206,22 +206,18 @@ struct MyValidatedClass{ }; TEST_F(AutoConfigTest, Validators) { - AutoRequired acm; - - ASSERT_ANY_THROW(acm->Set("ValidatedKey", 2)) << "AutoConfigManager didn't regect invalid value"; - AutoRequired valid; - acm->Set("ValidatedKey", 42); + *valid->m_config = 42; ASSERT_EQ(42, *valid->m_config) << "Value not set for key"; - ASSERT_ANY_THROW(acm->Set("ValidatedKey", 1)) << "AutoConfigManager didn't regect invalid value"; + ASSERT_ANY_THROW(*valid->m_config = 1) << "AutoConfigManager didn't regect invalid value"; ASSERT_EQ(42, *valid->m_config) << "Value not set for key"; - acm->Set("ValidatedKey", 1337); + *valid->m_config = 1337; ASSERT_EQ(1337, *valid->m_config) << "Value not set for key"; } - +/* TEST_F(AutoConfigTest, DirectAssignemnt) { AutoConfig var; var = 10; From ac99383ecdc40fac9d7a2316445167c73dc8c43b Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Tue, 24 Mar 2015 22:05:26 -0700 Subject: [PATCH 12/63] Added structure construction functionality. --- autowiring/AutoConfig.h | 14 ++-- src/autowiring/test/AutoConfigTest.cpp | 104 +++++-------------------- 2 files changed, 29 insertions(+), 89 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index e98c9a413..5eeb1ccbf 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -157,16 +157,20 @@ class AutoConfig : public AutoRequired> { public: typedef AutoConfigVar t_Var; + using AutoRequired::operator*; + AutoConfig(const std::shared_ptr& ctxt = CoreContext::CurrentContext()) : AutoRequired(ctxt) { } - template - AutoConfig(const std::shared_ptr& ctxt, t_Args&&... args) : - AutoRequired(ctxt, std::forward(args)...) + template + AutoConfig(t_Arg&& arg, t_Args&&... args) : + AutoRequired(CoreContext::CurrentContext(), std::forward(arg), std::forward(args)...) { + if (!(*this)->IsConfigured() || (*this)->IsInherited()) { + **this = T(std::forward(arg), std::forward(args)...); + } } - - using AutoRequired::operator*; + }; \ No newline at end of file diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index a8142d601..4ba914e42 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -15,7 +15,7 @@ TEST_F(AutoConfigTest, VerifyBasicAssignment) { *cfg1 = 1; ASSERT_EQ(*cfg1, 1) << "Operator = failed"; - AutoConfig cfg2(CoreContext::CurrentContext(), 2); + AutoConfig cfg2(2); ASSERT_EQ(*cfg2, 2) << "Default construction failed"; AutoConfig cfg1a; @@ -24,7 +24,7 @@ TEST_F(AutoConfigTest, VerifyBasicAssignment) { *cfg1a = 3; ASSERT_EQ(*cfg1, 3) << "Failed to autowire AutoConfig value"; - AutoConfig cfg2a(CoreContext::CurrentContext(), 5); + AutoConfig cfg2a(5); ASSERT_EQ(*cfg2a, 2) << "Constructor overwrote value when it wasn't supposed to!"; } @@ -39,7 +39,7 @@ TEST_F(AutoConfigTest, VerifyBasicSignals) { var.Get(&handler_value_read); }; - AutoConfig cfg1(CoreContext::CurrentContext(), 1); + AutoConfig cfg1(1); ASSERT_EQ(handler_called, 1) << "OnChanged not triggered by construction"; ASSERT_EQ(handler_value_read, 1) << "Bad value read in OnChanged"; @@ -47,7 +47,7 @@ TEST_F(AutoConfigTest, VerifyBasicSignals) { ASSERT_EQ(handler_called, 2) << "OnChanged not triggered by assignement"; ASSERT_EQ(handler_value_read, 20) << "Bad value read in OnChanged"; - AutoConfig cfg2(CoreContext::CurrentContext()); + AutoConfig cfg2; *cfg2 = 2; ASSERT_EQ(handler_called, 3) << "OnChanged not triggred by assignement from alternate autowired instance"; @@ -217,18 +217,6 @@ TEST_F(AutoConfigTest, Validators) { *valid->m_config = 1337; ASSERT_EQ(1337, *valid->m_config) << "Value not set for key"; } -/* -TEST_F(AutoConfigTest, DirectAssignemnt) { - AutoConfig var; - var = 10; - ASSERT_EQ(10, *var); - - AutoRequired containsVar; - - ASSERT_EQ(10, *var); - ASSERT_EQ(10, *containsVar->m_myName); -} - struct ComplexValue { int a; @@ -237,6 +225,7 @@ struct ComplexValue { ComplexValue(int nA, int nB, int nC) : a(nA), b(nB), c(nC) {} ComplexValue(int repeated) : a(repeated), b(repeated), c(repeated) {} + ComplexValue() {} }; struct MyComplexValueClass { @@ -247,76 +236,23 @@ struct MyComplexValueClass { }; TEST_F(AutoConfigTest, ComplexConstruction){ - AutoRequired mgr; - ASSERT_FALSE(mgr->IsConfigured("Namespace1.MyCxValue")); - + //Default constuction is not allowed if the underlying type does not support it AutoConfig defaultConstructed; - - ASSERT_FALSE(mgr->IsConfigured("Namespace1.MyCxValue")) << "Improperly set config value when default constructing AutoConfig"; + ASSERT_FALSE(defaultConstructed->IsConfigured()) << "non-initalizing constructor should not configure a value!"; AutoConfig fancyConstructed(1, 2, 3); - ASSERT_TRUE(mgr->IsConfigured("Namespace1.MyCxValue")) << "Initializing constructor did not set config value"; - ASSERT_EQ(fancyConstructed->a, 1) << "Initializing constructor did not set config value"; - ASSERT_EQ(fancyConstructed->b, 2) << "Initializing constructor did not set config value"; - ASSERT_EQ(fancyConstructed->c, 3) << "Initializing constructor did not set config value"; - - - AutoConfig fancy2(7); - ASSERT_EQ(fancy2->a, 1) << "Second Initalizing constructor overrode the first!"; - ASSERT_EQ(fancy2->b, 2) << "Second Initalizing constructor overrode the first!"; - ASSERT_EQ(fancy2->c, 3) << "Second Initalizing constructor overrode the first!"; -} - -struct OuterCtxt{}; -struct MiddleCtxt{}; -struct InnerCtxt{}; -TEST_F(AutoConfigTest, ListingConfigs) { - AutoCreateContextT ctxt_outer; - auto ctxt_middle = ctxt_outer->Create(); - auto ctxt_inner = ctxt_middle->Create(); - - AutoRequired acm_outer(ctxt_outer); - AutoRequired acm_inner(ctxt_inner); - - AutoRequired var1_inner(ctxt_inner); - var1_inner->m_myName = 1; - - ASSERT_EQ(0, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; - ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; - - int callback_outer = 0; - acm_outer->AddCallback([&callback_outer](const std::string& key, const AnySharedPointer& ptr) { - ++callback_outer; - }); - - int callback_inner = 0; - acm_inner->AddCallback([&callback_inner](const std::string& key, const AnySharedPointer& ptr) { - ++callback_inner; - }); - - ASSERT_EQ(1, callback_inner) << "Callback not called on existing keys"; - - AutoRequired var1_outer(ctxt_outer); - var1_outer->m_myName = 2; - - ASSERT_EQ(1, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; - ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; - - AutoRequired var2_outer(ctxt_outer); - var2_outer->m_myName = 3; - - ASSERT_EQ(2, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; - ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; - - ASSERT_EQ(2, callback_outer) << "Outer callback called an incorrect number of times"; - ASSERT_EQ(1, callback_inner) << "Inner callback called an incorrect number of times"; - - auto keys_outer = acm_outer->GetLocalKeys(); - ASSERT_EQ(var1_outer->m_myName.m_key, keys_outer[0]) << "Keys listed out of construction order"; - ASSERT_EQ(var2_outer->m_myName.m_key, keys_outer[1]) << "Keys listed out of construction order"; - - auto keys_inner = acm_inner->GetLocalKeys(); - ASSERT_EQ(var1_inner->m_myName.m_key, keys_inner[0]) << "Keys listed out of construction order"; + ASSERT_TRUE(fancyConstructed->IsConfigured()) << "Initializing constructor did not set config value"; + ASSERT_EQ((*fancyConstructed)->a, 1) << "Initializing constructor did not set config value"; + ASSERT_EQ((*fancyConstructed)->b, 2) << "Initializing constructor did not set config value"; + ASSERT_EQ((*fancyConstructed)->c, 3) << "Initializing constructor did not set config value"; + + AutoRequired complex; + ASSERT_EQ((*complex->m_cfg)->a, 1) << "Second Initalizing constructor overrode the first!"; + ASSERT_EQ((*complex->m_cfg)->b, 2) << "Second Initalizing constructor overrode the first!"; + ASSERT_EQ((*complex->m_cfg)->c, 3) << "Second Initalizing constructor overrode the first!"; + + ASSERT_EQ((*complex->m_cfg2)->a, 10) << "Initializing constructor was not called"; + ASSERT_EQ((*complex->m_cfg2)->b, 15) << "Initializing constructor was not called"; + ASSERT_EQ((*complex->m_cfg2)->c, 30) << "Initializing constructor was not called"; } -*/ From ed6e7280f942163cd05917b5828d93dedecaeb35 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 13:53:41 -0700 Subject: [PATCH 13/63] Fix assorted warnings and errors on mac --- autowiring/AutoConfig.h | 4 ++-- autowiring/Autowired.h | 10 +++++++--- src/autowiring/AutoConfig.cpp | 10 ++++++---- src/autowiring/SlotInformation.cpp | 8 ++++---- src/autowiring/test/AnySharedPointerTest.cpp | 2 +- src/autowiring/test/AutoConfigTest.cpp | 2 +- src/autowiring/test/ContextMapTest.cpp | 1 + 7 files changed, 22 insertions(+), 15 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 5eeb1ccbf..029b75c2d 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -76,7 +76,7 @@ class AutoConfigVar: //This will wind up being recursive auto parent = ctxt->GetParentContext(); if (parent != nullptr) { - auto parentVar = parent->Inject>(); + auto parentVar = parent->template Inject>(); //Only copy the value if it's initalized if (parentVar->IsConfigured()) { @@ -109,7 +109,7 @@ class AutoConfigVar: void SetParsed(const std::string& value) override { auto entry = RegConfig::r; - *this = entry.parseInternal(value); + *this = entry.template parseInternal(value); } // Add a callback for when this config value changes diff --git a/autowiring/Autowired.h b/autowiring/Autowired.h index c8a695be6..fbd411eff 100644 --- a/autowiring/Autowired.h +++ b/autowiring/Autowired.h @@ -201,12 +201,16 @@ class Autowired: } }; - template - signal_relay operator()(autowiring::signal T::*handler) { + template + signal_relay operator()(autowiring::signal U::*handler) { + static_assert(std::is_base_of::value, "Cannot reference member of unrelated type"); + + auto handlerActual = static_cast T::*>(handler); + auto ctxt = AutowirableSlot::GetContext(); if (!ctxt) throw std::runtime_error("Attempted to attach a signal to an Autowired field in a context that was already destroyed"); - return {*this, ctxt->RelayForType(handler)}; + return {*this, ctxt->RelayForType(handlerActual)}; } /// diff --git a/src/autowiring/AutoConfig.cpp b/src/autowiring/AutoConfig.cpp index 4af78513f..4b0036bba 100644 --- a/src/autowiring/AutoConfig.cpp +++ b/src/autowiring/AutoConfig.cpp @@ -4,10 +4,12 @@ #include "AutoConfigParser.hpp" AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti, bool configured) : + ContextMember(), m_key(autowiring::ExtractKey(ti)), - m_isConfigured(configured), - m_parentRegistration(nullptr), onChangedSignal(), - ContextMember(m_key.c_str()) -{} + m_isConfigured(configured), + m_parentRegistration(nullptr) +{ + m_name = m_key.c_str(); +} diff --git a/src/autowiring/SlotInformation.cpp b/src/autowiring/SlotInformation.cpp index a6b069df4..9fc6671fa 100644 --- a/src/autowiring/SlotInformation.cpp +++ b/src/autowiring/SlotInformation.cpp @@ -10,12 +10,12 @@ static autowiring::thread_specific_ptr tss([](SlotInformationStackLocation*) {}); SlotInformationStackLocation::SlotInformationStackLocation(SlotInformationStumpBase& stump, const void* pObj, size_t extent) : - prior(*tss), stump(stump), - m_pCur(nullptr), - m_pLastLink(nullptr), pObj(pObj), - extent(extent) + extent(extent), + prior(*tss), + m_pCur(nullptr), + m_pLastLink(nullptr) { tss.reset(this); } diff --git a/src/autowiring/test/AnySharedPointerTest.cpp b/src/autowiring/test/AnySharedPointerTest.cpp index ac74e4e1b..a0da08c52 100644 --- a/src/autowiring/test/AnySharedPointerTest.cpp +++ b/src/autowiring/test/AnySharedPointerTest.cpp @@ -200,7 +200,7 @@ TEST_F(AnySharedPointerTest, CanHoldCoreObject) { } TEST_F(AnySharedPointerTest, CanFastCastToSelf) { - autowiring::fast_pointer_cast_initializer::sc_init; + (void)autowiring::fast_pointer_cast_initializer::sc_init; auto co = std::make_shared(); ASSERT_EQ( diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index 4ba914e42..d68db36a9 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -34,7 +34,7 @@ TEST_F(AutoConfigTest, VerifyBasicSignals) { int handler_called = 0; int handler_value_read = 0; Autowired> autoCfg; - autoCfg(&AutoConfigVar::onChangedSignal) += [&](const AutoConfigVarBase& var) { + autoCfg(&AutoConfigVarBase::onChangedSignal) += [&](const AutoConfigVarBase& var) { ++handler_called; var.Get(&handler_value_read); }; diff --git a/src/autowiring/test/ContextMapTest.cpp b/src/autowiring/test/ContextMapTest.cpp index 268587e93..d8beb7f73 100644 --- a/src/autowiring/test/ContextMapTest.cpp +++ b/src/autowiring/test/ContextMapTest.cpp @@ -251,6 +251,7 @@ TEST_F(ContextMapTest, VerifyRangeBasedEnumeration) { // Internal map in ContextMap ensures entries are enumerated in-order for (auto& cur : mp) { + (void)cur; switch (ct) { case 0: // Release the entry we are presently enumerating From 26200fb8baca3ed3cbf1968874dfdcee5e342dd0 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 15:26:43 -0700 Subject: [PATCH 14/63] Changed AutoConfigManager to AutoConfigListing --- autowiring/AutoConfig.h | 4 +- ...utoConfigManager.h => AutoConfigListing.h} | 55 +-- autowiring/AutoParameter.h | 6 +- src/autowiring/AutoConfigListing.cpp | 116 ++++++ src/autowiring/AutoConfigManager.cpp | 184 --------- src/autowiring/CMakeLists.txt | 4 +- src/autowiring/test/AutoConfigListingTest.cpp | 360 ++++++++++++++++++ src/autowiring/test/AutoConfigTest.cpp | 2 +- src/autowiring/test/AutoParameterTest.cpp | 2 +- src/autowiring/test/CMakeLists.txt | 1 + 10 files changed, 505 insertions(+), 229 deletions(-) rename autowiring/{AutoConfigManager.h => AutoConfigListing.h} (65%) create mode 100644 src/autowiring/AutoConfigListing.cpp delete mode 100644 src/autowiring/AutoConfigManager.cpp create mode 100644 src/autowiring/test/AutoConfigListingTest.cpp diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 029b75c2d..3d3d0aac4 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -149,8 +149,8 @@ class AutoConfigVar: /// AutoConfig m_myVal; /// defines the key "MyKey" /// -/// AutoConfig values can be set from the AutoConfigManager. The string key -/// is used as the identifier for the value +/// AutoConfig values can also be set from the AutoConfigListing in the same context. The string key +/// is used as the identifier for the value. /// template class AutoConfig : public AutoRequired> { diff --git a/autowiring/AutoConfigManager.h b/autowiring/AutoConfigListing.h similarity index 65% rename from autowiring/AutoConfigManager.h rename to autowiring/AutoConfigListing.h index 57b20f301..ce5bf75ca 100644 --- a/autowiring/AutoConfigManager.h +++ b/autowiring/AutoConfigListing.h @@ -10,46 +10,38 @@ #include MUTEX_HEADER #include MEMORY_HEADER -struct AnySharedPointer; +class AutoConfigVarBase; -class AutoConfigManager: +class AutoConfigListing: public ContextMember { public: - AutoConfigManager(); - virtual ~AutoConfigManager(); + AutoConfigListing(); + virtual ~AutoConfigListing(); // Callback function type - typedef std::function t_callback; - - typedef std::function t_add_callback; + typedef std::function t_add_callback; // Validator function type typedef std::function t_validator; private: - // local map of the config registry + // Local map of the config registry static const std::unordered_map s_registry; - // map of validators registered for a key + // Validators for keys that have them. static const std::unordered_map s_validators; - // lock for all members + // Lock for all members std::mutex m_lock; - // Values of AutoConfigs in this context - std::unordered_map m_values; - - // Set of keys for values set from this context - std::unordered_set m_setHere; + // Map of AutoConfigs in this context + std::unordered_map> m_values; - // list of keys for values set from this context in order of creation. + // list of keys set locally from this context in order of creation. std::vector m_orderedKeys; - // map of callbacks registered for a key - std::unordered_map> m_callbacks; - - // list of callbacks registered for keys which exist in this context. + // list of callbacks for keys being added to this context. std::list m_addCallbacks; // Exception throwers: @@ -74,10 +66,10 @@ class AutoConfigManager: /// This method will throw an exception if the specified name cannot be found as a configurable value /// in the application, or if the specified value type does not match the type expected by this field /// - AnySharedPointer& Get(const std::string& key); + std::weak_ptr Get(const std::string& key); /// - /// Assigns the specified value to an AnySharedPointer slot + /// Assigns the specified value to an AutoConfig with a given key. Creates one if it doesn't exist. /// /// /// This method will throw an exception if the specified name cannot be found as a configurable value @@ -92,15 +84,9 @@ class AutoConfigManager: if (!s_registry.find(key)->second->verifyType(typeid(T))) ThrowTypeMismatchException(key, typeid(T)); - // Set value in this AutoConfigManager - SetRecursive(key, AnySharedPointer(std::make_shared(value))); + SetInternal(key, &value); } - /// - /// Overload for c-style string. Converts to std::string - /// - void Set(const std::string& key, const char* value); - /// /// Coerces the string representation of the specified field to the correct value type /// @@ -113,20 +99,17 @@ class AutoConfigManager: bool SetParsed(const std::string& key, const std::string& value); // Add a callback for when key is changed in this context - void AddCallback(const std::string& key, t_callback&& fx); + void AddOnChanged(const std::string& key, std::function&& fx); // Add a callback for when a key is set in this context. Is immediately called on all - // currently existing values in the order they were created + // currently existing values in the order they were created. void AddCallback(t_add_callback&& fx); - // Returns a list of all keys which have been set from this context + // Returns a list of all keys which have been set from this context. const std::vector& GetLocalKeys() const { return m_orderedKeys; } private: - // Handles setting a value recursivly to all child contexts - void SetRecursive(const std::string& key, AnySharedPointer value); - // Set a value in this manager, call callbacks // Must hold m_lock when calling this - void SetInternal(const std::string& key, const AnySharedPointer& value); + void SetInternal(const std::string& key, const void* value); }; diff --git a/autowiring/AutoParameter.h b/autowiring/AutoParameter.h index b5ae2cef8..c8c6480ac 100644 --- a/autowiring/AutoParameter.h +++ b/autowiring/AutoParameter.h @@ -7,7 +7,7 @@ struct AutoParam{}; /// -/// Register an AutoParameter with "AutoParam" namespace in AutoConfigManager. +/// Register an AutoParameter with "AutoParam" namespace in AutoConfigListing. /// In addition to being the lookup string, the Key also implements: /// - static constexpr T Default() /// - (optional) static bool Validate(const T&) @@ -66,7 +66,7 @@ class AutoParameter: /// Base class for providing an easy way to specify just a default value /// /// -/// Because the class names are used as keys to be stored in the AutoConfigManager, these classes cannot be used +/// Because the class names are used as keys to be stored in the AutoConfigListing, these classes cannot be used /// directly and must be subclassed: /// /// struct MyDefaultKey : DefaultKey {}; @@ -88,7 +88,7 @@ struct DefaultKey /// Base class for providing an easy way to default, min and max values /// /// -/// Because the class names are used as keys to be stored in the AutoConfigManager, these classes cannot be used +/// Because the class names are used as keys to be stored in the AutoConfigListing, these classes cannot be used /// directly and must be subclassed: /// /// struct MyDefaultMinMaxKey : DefaultMinMaxKey {}; diff --git a/src/autowiring/AutoConfigListing.cpp b/src/autowiring/AutoConfigListing.cpp new file mode 100644 index 000000000..0fcbda56f --- /dev/null +++ b/src/autowiring/AutoConfigListing.cpp @@ -0,0 +1,116 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include "AutoConfig.h" +#include "AutoConfigListing.h" +#include "AnySharedPointer.h" +#include + +using namespace autowiring; + +// Create map of all config values in the registry +static std::unordered_map FillRegistry(void) { + std::unordered_map registry; + + for (auto config = g_pFirstConfigEntry; config; config = config->pFlink) { + registry[config->m_key] = config; + } + + return registry; +} + +// Create map of all validators specified +static std::unordered_map FillValidators(void) { + std::unordered_map validator; + + for (auto config = g_pFirstConfigEntry; config; config = config->pFlink) { + if (config->m_hasValidator) { + validator[config->m_key] = config->validator(); + } + } + + return validator; +} + +const std::unordered_map AutoConfigListing::s_registry = FillRegistry(); +const std::unordered_map AutoConfigListing::s_validators = FillValidators(); + +AutoConfigListing::AutoConfigListing(void){ + auto ctxt = m_context.lock(); +} + +AutoConfigListing::~AutoConfigListing(void){} + +void AutoConfigListing::ThrowKeyNotFoundException(const std::string& key) const { + std::stringstream ss; + ss << "No configuration found for key '" << key << "'"; + throw autowiring_error(ss.str()); +} + +void AutoConfigListing::ThrowTypeMismatchException(const std::string& key, const std::type_info& ti) const { + std::stringstream ss; + ss << "Attempting to set config '" << key << "' with incorrect type '" + << autowiring::demangle(ti) << "'"; + throw autowiring_error(ss.str()); + +} + +bool AutoConfigListing::IsConfigured(const std::string& key) { + std::lock_guard lk(m_lock); + auto config = m_values[key].lock(); + return config && config->IsConfigured(); +} + +bool AutoConfigListing::IsInherited(const std::string& key) { + std::lock_guard lk(m_lock); + auto config = m_values[key].lock(); + return config && config->IsInherited(); +} + +std::weak_ptr AutoConfigListing::Get(const std::string& key) { + std::lock_guard lk(m_lock); + return m_values[key]; + + // Key not found, throw exception + std::stringstream ss; + ss << "Attepted to get key '" << key << "' which hasn't been set"; + throw autowiring_error(ss.str()); +} + +bool AutoConfigListing::SetParsed(const std::string& key, const std::string& value) { + // Key not found + if (!s_registry.count(key)) { + return false; + } + auto parsedValue = s_registry.find(key)->second->parse(value); + SetInternal(key, &(*parsedValue)); + return true; +} + +void AutoConfigListing::AddOnChanged(const std::string& key, std::function&& fx) { + std::lock_guard lk(m_lock); + auto config = m_values[key].lock(); + if (config) + (*config).onChangedSignal += std::move(fx); +} + +void AutoConfigListing::AddCallback(t_add_callback&& fx) { + // Grab lock until done setting value + std::lock_guard lk(m_lock); + + for (auto& key : m_orderedKeys) { + auto config = m_values[key].lock(); + if ( config ) + fx(*config); + } + + m_addCallbacks.emplace_back(std::move(fx)); +} + +void AutoConfigListing::SetInternal(const std::string& key, const void* value) { + auto config = m_values[key].lock(); + + if (!config) + return; + + config->Set(value); +} diff --git a/src/autowiring/AutoConfigManager.cpp b/src/autowiring/AutoConfigManager.cpp deleted file mode 100644 index ec12c6676..000000000 --- a/src/autowiring/AutoConfigManager.cpp +++ /dev/null @@ -1,184 +0,0 @@ -// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. -#include "stdafx.h" -#include "AutoConfig.h" -#include "AutoConfigManager.h" -#include "AnySharedPointer.h" -#include - -using namespace autowiring; - -// Create map of all config values in the registry -static std::unordered_map FillRegistry(void) { - std::unordered_map registry; - - for (auto config = g_pFirstConfigEntry; config; config = config->pFlink) { - registry[config->m_key] = config; - } - - return registry; -} - -// Create map of all validators specified -static std::unordered_map FillValidators(void) { - std::unordered_map validator; - - for (auto config = g_pFirstConfigEntry; config; config = config->pFlink) { - if (config->m_hasValidator) { - validator[config->m_key] = config->validator(); - } - } - - return validator; -} - -const std::unordered_map AutoConfigManager::s_registry = FillRegistry(); -const std::unordered_map AutoConfigManager::s_validators = FillValidators(); - -AutoConfigManager::AutoConfigManager(void){ - // Copy parent's config settings - auto parent = GetContext()->GetParentContext(); - if (parent) { - AutowiredFast mgmt(parent); - - // Is there AutoConfigManager in an ancestor? - if (mgmt) { - std::lock_guard lk(mgmt->m_lock); - m_values = mgmt->m_values; - } - } -} - -AutoConfigManager::~AutoConfigManager(void){} - -void AutoConfigManager::ThrowKeyNotFoundException(const std::string& key) const { - std::stringstream ss; - ss << "No configuration found for key '" << key << "'"; - throw autowiring_error(ss.str()); -} - -void AutoConfigManager::ThrowTypeMismatchException(const std::string& key, const std::type_info& ti) const { - std::stringstream ss; - ss << "Attempting to set config '" << key << "' with incorrect type '" - << autowiring::demangle(ti) << "'"; - throw autowiring_error(ss.str()); - -} - -bool AutoConfigManager::IsConfigured(const std::string& key) { - std::lock_guard lk(m_lock); - return !!m_values.count(key); -} - -bool AutoConfigManager::IsInherited(const std::string& key) { - std::lock_guard lk(m_lock); - return m_values.count(key) && !m_setHere.count(key); -} - -AnySharedPointer& AutoConfigManager::Get(const std::string& key) { - std::lock_guard lk(m_lock); - - if (m_values.count(key)) { - return m_values[key]; - } - - // Key not found, throw exception - std::stringstream ss; - ss << "Attepted to get key '" << key << "' which hasn't been set"; - throw autowiring_error(ss.str()); -} - -void AutoConfigManager::Set(const std::string& key, const char* value) { - Set(key, std::string(value)); -} - -bool AutoConfigManager::SetParsed(const std::string& key, const std::string& value) { - // Key not found - if (!s_registry.count(key)) { - return false; - } - - SetRecursive(key, s_registry.find(key)->second->parse(value)); - return true; -} - -void AutoConfigManager::AddCallback(const std::string& key, t_callback&& fx) { - std::lock_guard lk(m_lock); - m_callbacks[key].push_back(fx); -} - -void AutoConfigManager::AddCallback(t_add_callback&& fx) { - // Grab lock until done setting value - std::lock_guard lk(m_lock); - - for (auto& key : m_orderedKeys) - fx(key, m_values[key]); - - m_addCallbacks.emplace_back(std::move(fx)); -} - -void AutoConfigManager::SetRecursive(const std::string& key, AnySharedPointer value) { - // Call all validators for this key - if (s_validators.count(key)) { - auto const& fx = s_validators.find(key)->second; - if (!fx(value)) { - std::stringstream ss; - ss << "Attempted to set key '" << key << "'which didin't pass validator"; - throw autowiring_error(ss.str()); - } - } - - // Grab lock until done setting value - std::unique_lock lk(m_lock); - - // Actually set the value in this manager - SetInternal(key, value); - - // Mark key set from this manager - auto inserted = m_setHere.insert(key); - if (inserted.second) { - m_orderedKeys.push_back(key); - for (auto& callback : m_addCallbacks) { - lk.unlock(); - callback(key, value); - lk.lock(); - } - } - - // Enumerate descendant contexts - auto enumerator = ContextEnumerator(GetContext()); - auto ctxt = enumerator.begin(); - - // Skip current context - ++ctxt; - - // Recursivly set values in desendent contexts - while (ctxt != enumerator.end()) { - - // Make sure we get the AutoConfigManager from "ctxt" - std::shared_ptr mgmt; - (*ctxt)->FindByType(mgmt); - if (mgmt) { - std::lock_guard descendant_lk(mgmt->m_lock); - - // Check if value was set from this context - // If so, stop recursing down this branch, continue to next sibling - if (mgmt->m_setHere.count(key)){ - ctxt.NextSibling(); - continue; - } - - //Actaully set the value - mgmt->SetInternal(key, value); - } - // Continue to next context - ++ctxt; - } -} - -void AutoConfigManager::SetInternal(const std::string& key, const AnySharedPointer& value) { - m_values[key] = value; - - // Call callbacks for this key - for (const auto& cb : m_callbacks[key]) - cb(value); -} diff --git a/src/autowiring/CMakeLists.txt b/src/autowiring/CMakeLists.txt index c9134e3bc..50838c646 100644 --- a/src/autowiring/CMakeLists.txt +++ b/src/autowiring/CMakeLists.txt @@ -20,8 +20,8 @@ set(Autowiring_SRCS auto_signal.cpp AutoConfig.cpp AutoConfig.h - AutoConfigManager.cpp - AutoConfigManager.h + AutoConfigListing.cpp + AutoConfigListing.h AutoConfigParser.cpp AutoConfigParser.hpp AutoFilterDescriptor.h diff --git a/src/autowiring/test/AutoConfigListingTest.cpp b/src/autowiring/test/AutoConfigListingTest.cpp new file mode 100644 index 000000000..0552a0593 --- /dev/null +++ b/src/autowiring/test/AutoConfigListingTest.cpp @@ -0,0 +1,360 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include +#include +#include + +class AutoConfigListingTest : + public testing::Test +{}; + +struct Namespace1; +struct Namespace2; +struct XYZ; + +TEST_F(AutoConfigListingTest, VerifySimpleAssignment) { + // Set an attribute in the manager before injecting anything: + AutoRequired acm; + acm->Set("Namespace1.XYZ", 323); + + // Now inject the type which expects this value to be assigned: + AutoConfig cfg; + ASSERT_EQ(323, *cfg) << "Configurable type did not receive a value as expected"; +} +/* +struct NamespaceRoot; +struct NamespaceChild; + +TEST_F(AutoConfigListingTest, VerifyNestedNamespace) { + AutoRequired acm; + acm->Set("NamespaceRoot.NamespaceChild.Namespace1.Namespace2.XYZ", 142); + + AutoConfig cfg; + ASSERT_EQ(142, *cfg); +} + +struct MyBoolClass { + AutoConfig m_bool; +}; + +TEST_F(AutoConfigListingTest, VerifyBool) { + AutoRequired acm; + AutoRequired clz1; + + acm->Set("bool_space.my_bool", true); + ASSERT_TRUE(*clz1->m_bool); + + acm->SetParsed("bool_space.my_bool", "false"); + ASSERT_FALSE(*clz1->m_bool); +} + +TEST_F(AutoConfigListingTest, VerifyPostHocAssignment) { + // Inject the configurable type first + AutoRequired mcc; + + // Configuration manager must exist at this point as a consequence of the earlier construction + Autowired acm; + ASSERT_TRUE(acm.IsAutowired()) << "AutoConfig field did not inject a configuration manager into this context as expected"; + + acm->Set("Namespace1.XYZ", 323); + + // Now inject the type which expects this value to be assigned: + ASSERT_EQ(323, *mcc->m_myName) << "Configurable type did not receive a value as expected"; +} + +TEST_F(AutoConfigListingTest, VerifyRecursiveSearch) { + AutoRequired acm; + acm->Set("Namespace1.XYZ", 1001); + + { + AutoCreateContext ctxt; + CurrentContextPusher pshr(ctxt); + + // Now inject an element here, and verify that it was wired up as expected: + AutoRequired mcc; + ASSERT_TRUE(mcc->m_myName->IsConfigured()) << "A configurable value was not configured as expected"; + ASSERT_EQ(1001, *mcc->m_myName) << "Configurable value obtained from a parent scope did not have the correct value"; + + // This must work as expected--a local context override will rewrite configuration values in the local scope + AutoRequired sub_mcc; + sub_mcc->Set("Namespace1.XYZ", 1002); + ASSERT_EQ(1002, *mcc->m_myName) << "Override of a configurable value in a derived class did not take place as expected"; + } +} + +struct DefaultName { + AutoConfig m_def; +}; + +TEST_F(AutoConfigListingTest, DefaultNamespace) { + AutoRequired acm; + acm->Set("defaultname1", 123); + + AutoRequired def; + + ASSERT_EQ(123, *def->m_def); +} + +TEST_F(AutoConfigListingTest, VerifyParsedAssignment) { + // We must also be able to support implicit string-to-type conversion via the shift operator for this type + AutoRequired acm; + + // Direct assignment to a string should not work, the type isn't a string it's an int + ASSERT_ANY_THROW(acm->Set("Namespace1.XYZ", "327")) << "An attempt to assign a value to an unrelated type did not generate an exception as expected"; + + ASSERT_ANY_THROW(acm->Set("Namespace1.XYZ", 3.0)) << "An attempt to assign a value to an unrelated type did not generate an exception as expected"; + + // Assignment to a string type should result in an appropriate coercion to the right value + ASSERT_TRUE(acm->SetParsed("Namespace1.XYZ", "324")); +} + +TEST_F(AutoConfigListingTest, VerifyDuplicateConfigAssignment) { + AutoRequired acm; + ASSERT_TRUE(acm->SetParsed("Namespace1.XYZ", "324")); + ASSERT_TRUE(acm->SetParsed("Namespace2.XYZ", "1111")); + + AutoRequired clz1; + AutoRequired clz2; + + ASSERT_EQ(324, *clz1->m_myName); + ASSERT_EQ(1111, *clz2->m_myName); +} + +class TypeWithoutAShiftOperator { +public: + int foo; +}; + +struct NoShift { + AutoConfig m_noshift; +}; + +static_assert(has_stream::value, "Stream operation not detected on a primitive type"); +static_assert(!has_stream::value, "Stream operation detected on a type that should not have supported it"); + +TEST_F(AutoConfigListingTest, TypeWithoutAShiftOperatorTest) { + AutoRequired noshift; + + AutoRequired mgr; + + // Indirect assignment should cause an exception + ASSERT_ANY_THROW(mgr->Set("MyNoShift", "")) << "Expected a throw in a case where a configurable value was used which cannot be assigned"; + + // Direct assignment should be supported still + TypeWithoutAShiftOperator tasoVal; + tasoVal.foo = 592; + mgr->Set("MyNoShift", tasoVal); + + ASSERT_EQ(592, noshift->m_noshift->foo) << "Value assignment did not result in an update to a non-serializable configuration field"; +} + +TEST_F(AutoConfigListingTest, Callbacks) { + AutoRequired acm; + AutoRequired mcc; + + acm->Set("Namespace1.XYZ", 4); + + mcc->m_myName += [](int val) { + ASSERT_EQ(val, 42); + }; + + mcc->m_myName += [](int val) { + ASSERT_EQ(val, 42); + }; + + acm->Set("Namespace1.XYZ", 42); +} + +TEST_F(AutoConfigListingTest, NestedContexts) { + // Set up contexts and members + AutoCurrentContext ctxt_outer; + std::shared_ptr ctxt_middle = ctxt_outer->Create(); + std::shared_ptr ctxt_sibling = ctxt_outer->Create(); + std::shared_ptr ctxt_inner = ctxt_middle->Create(); + std::shared_ptr ctxt_no_manager = ctxt_inner->Create(); + std::shared_ptr ctxt_leaf = ctxt_no_manager->Create(); + + AutoRequired mcc_outer(ctxt_outer); + AutoRequired mcc_middle(ctxt_middle); + AutoRequired mcc_sibling(ctxt_sibling); + AutoRequired mcc_inner(ctxt_inner); + AutoRequired mcc_leaf(ctxt_leaf); + + AutoRequired acm_outer(ctxt_outer); + AutoRequired acm_middle(ctxt_middle); + AutoRequired acm_leaf(ctxt_leaf); + + // Set initial value + acm_outer->Set("Namespace1.XYZ", 42); + ASSERT_EQ(42, *mcc_outer->m_myName) << "Config value not set"; + ASSERT_EQ(42, *mcc_middle->m_myName) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_sibling->m_myName) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_inner->m_myName) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_leaf->m_myName) << "Config value not set in desendant context"; + EXPECT_TRUE(acm_middle->IsInherited("Namespace1.XYZ")) << "Inherited key not marked as such"; + EXPECT_TRUE(acm_leaf->IsInherited("Namespace1.XYZ")) << "Inherited key not marked as such"; + + // Set middle, inner shouldn't be able to be set from outer after this + bool callback_hit1 = false; + mcc_inner->m_myName += [&callback_hit1](int) { + callback_hit1 = true; + }; + acm_middle->Set("Namespace1.XYZ", 1337); + ASSERT_EQ(42, *mcc_outer->m_myName) << "Config value changed in outer context"; + ASSERT_EQ(42, *mcc_sibling->m_myName) << "Config value set from sibling context"; + ASSERT_EQ(1337, *mcc_middle->m_myName) << "Config value not set"; + ASSERT_EQ(1337, *mcc_inner->m_myName) << "Config value not set in child context"; + ASSERT_EQ(1337, *mcc_leaf->m_myName) << "Config value not set in leaf context"; + ASSERT_TRUE(callback_hit1) << "Callback not hit in inner context"; + + // Set from outter, inner should be shielded by middle context + mcc_inner->m_myName += [](int) { + FAIL() << "This callback should never be hit"; + }; + + // Make sure sibling context is not shielded + bool callback_hit2 = false; + mcc_sibling->m_myName += [&callback_hit2](int) { + callback_hit2 = true; + }; + + // Set from outer, shouldn't effect middle or inner contexts + acm_outer->Set("Namespace1.XYZ", 999); + ASSERT_EQ(999, *mcc_outer->m_myName) << "Config value not set"; + ASSERT_EQ(1337, *mcc_middle->m_myName) << "Config value overwritten when value was set in this context"; + ASSERT_EQ(1337, *mcc_inner->m_myName) << "Config value overwritten when value was set in parent context"; + + // Make sure sibling hit + ASSERT_EQ(999, *mcc_sibling->m_myName) << "Value not set on sibling of context where value was previously set"; + ASSERT_TRUE(callback_hit2) << "Callback not called on sibling of context where value was previously set"; +} + +struct ValidatedKey{ + static bool Validate(const int& value) { + return value > 5; + } +}; + +struct MyValidatedClass{ + AutoConfig m_config; +}; + +TEST_F(AutoConfigListingTest, Validators) { + AutoRequired acm; + + ASSERT_ANY_THROW(acm->Set("ValidatedKey", 2)) << "AutoConfigListing didn't regect invalid value"; + + AutoRequired valid; + + acm->Set("ValidatedKey", 42); + ASSERT_EQ(42, *valid->m_config) << "Value not set for key"; + + ASSERT_ANY_THROW(acm->Set("ValidatedKey", 1)) << "AutoConfigListing didn't regect invalid value"; + ASSERT_EQ(42, *valid->m_config) << "Value not set for key"; + + acm->Set("ValidatedKey", 1337); + ASSERT_EQ(1337, *valid->m_config) << "Value not set for key"; +} + +TEST_F(AutoConfigListingTest, DirectAssignemnt) { + AutoConfig var; + var = 10; + ASSERT_EQ(10, *var); + + AutoRequired containsVar; + + ASSERT_EQ(10, *var); + ASSERT_EQ(10, *containsVar->m_myName); +} + + +struct ComplexValue { + int a; + int b; + int c; + + ComplexValue(int nA, int nB, int nC) : a(nA), b(nB), c(nC) {} + ComplexValue(int repeated) : a(repeated), b(repeated), c(repeated) {} +}; + +struct MyComplexValueClass { + AutoConfig m_cfg; + AutoConfig m_cfg2 = ComplexValue{ 10, 15, 30 }; + + MyComplexValueClass() : m_cfg(ComplexValue{ 2, 20, 20 }) {} +}; + +TEST_F(AutoConfigListingTest, ComplexConstruction){ + AutoRequired mgr; + ASSERT_FALSE(mgr->IsConfigured("Namespace1.MyCxValue")); + + AutoConfig defaultConstructed; + + ASSERT_FALSE(mgr->IsConfigured("Namespace1.MyCxValue")) << "Improperly set config value when default constructing AutoConfig"; + + AutoConfig fancyConstructed(1, 2, 3); + + ASSERT_TRUE(mgr->IsConfigured("Namespace1.MyCxValue")) << "Initializing constructor did not set config value"; + ASSERT_EQ(fancyConstructed->a, 1) << "Initializing constructor did not set config value"; + ASSERT_EQ(fancyConstructed->b, 2) << "Initializing constructor did not set config value"; + ASSERT_EQ(fancyConstructed->c, 3) << "Initializing constructor did not set config value"; + + + AutoConfig fancy2(7); + ASSERT_EQ(fancy2->a, 1) << "Second Initalizing constructor overrode the first!"; + ASSERT_EQ(fancy2->b, 2) << "Second Initalizing constructor overrode the first!"; + ASSERT_EQ(fancy2->c, 3) << "Second Initalizing constructor overrode the first!"; +} + +struct OuterCtxt{}; +struct MiddleCtxt{}; +struct InnerCtxt{}; +TEST_F(AutoConfigListingTest, ListingConfigs) { + AutoCreateContextT ctxt_outer; + auto ctxt_middle = ctxt_outer->Create(); + auto ctxt_inner = ctxt_middle->Create(); + + AutoRequired acm_outer(ctxt_outer); + AutoRequired acm_inner(ctxt_inner); + + AutoRequired var1_inner(ctxt_inner); + var1_inner->m_myName = 1; + + ASSERT_EQ(0, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; + ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; + + int callback_outer = 0; + acm_outer->AddCallback([&callback_outer](const std::string& key, const AnySharedPointer& ptr) { + ++callback_outer; + }); + + int callback_inner = 0; + acm_inner->AddCallback([&callback_inner](const std::string& key, const AnySharedPointer& ptr) { + ++callback_inner; + }); + + ASSERT_EQ(1, callback_inner) << "Callback not called on existing keys"; + + AutoRequired var1_outer(ctxt_outer); + var1_outer->m_myName = 2; + + ASSERT_EQ(1, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; + ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; + + AutoRequired var2_outer(ctxt_outer); + var2_outer->m_myName = 3; + + ASSERT_EQ(2, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; + ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; + + ASSERT_EQ(2, callback_outer) << "Outer callback called an incorrect number of times"; + ASSERT_EQ(1, callback_inner) << "Inner callback called an incorrect number of times"; + + auto keys_outer = acm_outer->GetLocalKeys(); + ASSERT_EQ(var1_outer->m_myName.m_key, keys_outer[0]) << "Keys listed out of construction order"; + ASSERT_EQ(var2_outer->m_myName.m_key, keys_outer[1]) << "Keys listed out of construction order"; + + auto keys_inner = acm_inner->GetLocalKeys(); + ASSERT_EQ(var1_inner->m_myName.m_key, keys_inner[0]) << "Keys listed out of construction order"; +} +*/ \ No newline at end of file diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index d68db36a9..ba737c5cc 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -211,7 +211,7 @@ TEST_F(AutoConfigTest, Validators) { *valid->m_config = 42; ASSERT_EQ(42, *valid->m_config) << "Value not set for key"; - ASSERT_ANY_THROW(*valid->m_config = 1) << "AutoConfigManager didn't regect invalid value"; + ASSERT_ANY_THROW(*valid->m_config = 1) << "AutoConfig validator didn't reject invalid value"; ASSERT_EQ(42, *valid->m_config) << "Value not set for key"; *valid->m_config = 1337; diff --git a/src/autowiring/test/AutoParameterTest.cpp b/src/autowiring/test/AutoParameterTest.cpp index 310565ac8..432400ee7 100644 --- a/src/autowiring/test/AutoParameterTest.cpp +++ b/src/autowiring/test/AutoParameterTest.cpp @@ -114,7 +114,7 @@ struct MyParamClass4 { }; TEST_F(AutoParameterTest, VerifyInvalidPreconfiguredValue) { - AutoRequired acm; + AutoRequired acm; ASSERT_ANY_THROW(acm->Set("AutoParam.MyParamClass4::MyIntParam4", 0)); AutoRequired my4; diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index 43401e6f6..6c5a1753c 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -2,6 +2,7 @@ set(AutowiringTest_SRCS AnySharedPointerTest.cpp ArgumentTypeTest.cpp AutoConfigTest.cpp + AutoConfigListingTest.cpp AutoConfigParserTest.cpp AutoConstructTest.cpp AutoFilterCollapseRulesTest.cpp From cbed6780b5a6acfb8944e0661118954dd5fed66e Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 17:18:04 -0700 Subject: [PATCH 15/63] Split AutoConfig into 2 files. --- autowiring/AutoConfig.h | 42 ++++--------------- autowiring/AutoConfigBase.h | 37 ++++++++++++++++ autowiring/ConfigRegistry.h | 4 +- .../{AutoConfig.cpp => AutoConfigBase.cpp} | 2 +- src/autowiring/CMakeLists.txt | 3 +- 5 files changed, 50 insertions(+), 38 deletions(-) create mode 100644 autowiring/AutoConfigBase.h rename src/autowiring/{AutoConfig.cpp => AutoConfigBase.cpp} (92%) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 3d3d0aac4..ac2faff42 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -1,49 +1,23 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once +#include "AutoConfigBase.h" #include "Autowired.h" -#include "auto_signal.h" #include "ConfigRegistry.h" -#include -#include TYPE_INDEX_HEADER - -struct AnySharedPointer; - -/// \internal /// -/// Utility base type for configuration members -/// -class AutoConfigVarBase : public ContextMember -{ -public: - AutoConfigVarBase(const std::type_info& tiName, bool configured = false); - - // Key used to identify this config value - const std::string m_key; - - // Generic interface functions for getting and setting - virtual void Get(void* pValue) const = 0; - virtual void Set(const void* pValue) = 0; - - virtual void SetParsed(const std::string& value) = 0; - - typedef autowiring::signal t_OnChangedSignal; - t_OnChangedSignal onChangedSignal; - - bool IsConfigured() const { return m_isConfigured; } - bool IsInherited() const { return m_parentRegistration != nullptr; } - -protected: - bool m_isConfigured; - t_OnChangedSignal::t_registration* m_parentRegistration; -}; +/// The type underlying the AutoConfig System. +/// Represents a unique type created by the combination of the type and a set of sigils. +/// Responsible for tracking changes to the underlying value and triggering signals, +/// making sure values are inherited correctly from enclosing contexts, and providing +/// a primitive polymorphic get/set interface (void*) +/// template class AutoConfigVar: public AutoConfigVarBase { public: - static_assert(sizeof...(TKey) >= 1, "Must provide a key and optionally at least one namespace"); + static_assert(sizeof...(TKey) >= 1, "Must provide a key and optionally a set of namespaces"); template AutoConfigVar(t_Arg &&arg, t_Args&&... args) : diff --git a/autowiring/AutoConfigBase.h b/autowiring/AutoConfigBase.h new file mode 100644 index 000000000..88dfd3cb4 --- /dev/null +++ b/autowiring/AutoConfigBase.h @@ -0,0 +1,37 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#pragma once +#include "ContextMember.h" +#include "auto_signal.h" + +#include +#include +#include TYPE_INDEX_HEADER + +struct AnySharedPointer; + +/// \internal +/// +/// Utility base type & interface for configuration members +/// +class AutoConfigVarBase : public ContextMember +{ +public: + AutoConfigVarBase(const std::type_info& tiName, bool configured = false); + + // Key used to identify this config value + const std::string m_key; + + bool IsConfigured() const { return m_isConfigured; } + bool IsInherited() const { return m_parentRegistration != nullptr; } + + typedef autowiring::signal t_OnChangedSignal; + t_OnChangedSignal onChangedSignal; + + virtual void Get(void* pValue) const = 0; + virtual void Set(const void* pValue) = 0; + virtual void SetParsed(const std::string& value) = 0; + +protected: + bool m_isConfigured; + t_OnChangedSignal::t_registration* m_parentRegistration; +}; diff --git a/autowiring/ConfigRegistry.h b/autowiring/ConfigRegistry.h index 2f5c461a7..ad101ad8c 100644 --- a/autowiring/ConfigRegistry.h +++ b/autowiring/ConfigRegistry.h @@ -1,6 +1,6 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once -#include "AnySharedPointer.h" +#include "AutoConfigBase.h" #include "autowiring_error.h" #include "has_validate.h" #include @@ -34,7 +34,7 @@ struct get_last{ typedef T last; }; -// Stores information about an AutoConfig entry +// Stores information about an AutoConfigVar type struct ConfigRegistryEntry { ConfigRegistryEntry(const std::type_info& ti, bool has_validator); diff --git a/src/autowiring/AutoConfig.cpp b/src/autowiring/AutoConfigBase.cpp similarity index 92% rename from src/autowiring/AutoConfig.cpp rename to src/autowiring/AutoConfigBase.cpp index 4b0036bba..80017ce25 100644 --- a/src/autowiring/AutoConfig.cpp +++ b/src/autowiring/AutoConfigBase.cpp @@ -1,6 +1,6 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #include "stdafx.h" -#include "AutoConfig.h" +#include "AutoConfigBase.h" #include "AutoConfigParser.hpp" AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti, bool configured) : diff --git a/src/autowiring/CMakeLists.txt b/src/autowiring/CMakeLists.txt index 50838c646..1c2bdc550 100644 --- a/src/autowiring/CMakeLists.txt +++ b/src/autowiring/CMakeLists.txt @@ -18,8 +18,9 @@ set(Autowiring_SRCS auto_future.h auto_signal.h auto_signal.cpp - AutoConfig.cpp AutoConfig.h + AutoConfigBase.cpp + AutoConfigBase.h AutoConfigListing.cpp AutoConfigListing.h AutoConfigParser.cpp From d62d38e469ac5c7b0eaf429946f1c1fca95e8d49 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 18:00:52 -0700 Subject: [PATCH 16/63] Make ConfigRegistryEntries able to inject their corresponding types. --- autowiring/AutoConfig.h | 31 +++++++++++++++++--------- autowiring/ConfigRegistry.h | 33 +++++++++------------------- src/autowiring/AutoConfigListing.cpp | 19 ++++++++++++++-- src/autowiring/ConfigRegistry.cpp | 9 +++----- 4 files changed, 51 insertions(+), 41 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index ac2faff42..6e712345b 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -25,10 +25,10 @@ class AutoConfigVar: m_value(std::forward(arg), std::forward(args)...) { // Register with config registry - auto config = RegConfig::r; + (void)AutoConfigVar::RegistryEntry; - if (config.m_hasValidator) { - if (!config.validatorInternal()(m_value)) { + if (RegistryEntry.m_hasValidator) { + if (!RegistryEntry.validatorInternal()(m_value)) { throw autowiring_error("Cannot construct AutoConfigVar with a value that fails validation"); } } @@ -41,7 +41,7 @@ class AutoConfigVar: m_value() { // Register with config registry - (void)RegConfig::r; + (void)AutoConfigVar::RegistryEntry; const auto ctxt = m_context.lock(); if (!ctxt) @@ -82,8 +82,7 @@ class AutoConfigVar: void Set(const void* pValue) override { *this = *reinterpret_cast(pValue); } void SetParsed(const std::string& value) override { - auto entry = RegConfig::r; - *this = entry.template parseInternal(value); + *this = RegistryEntry.parseInternal(value); } // Add a callback for when this config value changes @@ -99,10 +98,8 @@ class AutoConfigVar: T m_value; void SetInternal(const T& val) { - auto config = RegConfig::r; - - if (config.m_hasValidator) { - if (!config.validatorInternal()(val)) { + if (RegistryEntry.m_hasValidator) { + if (!RegistryEntry.validatorInternal()(val)) { throw autowiring_error("Validator rejected set for config value"); } } @@ -111,8 +108,22 @@ class AutoConfigVar: m_value = val; onChangedSignal(*this); } + +public: + static std::shared_ptr Inject(const std::shared_ptr& ctxt, const void* value) { + if (!value) + return ctxt->Inject>(); + else + return ctxt->Inject>(*reinterpret_cast(value)); + } + + static const ConfigRegistryEntryT RegistryEntry; }; +template +const ConfigRegistryEntryT AutoConfigVar::RegistryEntry(&AutoConfigVar::Inject); + + /// /// Register an attribute with the AutoConfig system. For example /// diff --git a/autowiring/ConfigRegistry.h b/autowiring/ConfigRegistry.h index ad101ad8c..40c3e0190 100644 --- a/autowiring/ConfigRegistry.h +++ b/autowiring/ConfigRegistry.h @@ -8,6 +8,7 @@ #include FUNCTIONAL_HEADER #include MEMORY_HEADER +#include "AnySharedPointer.h" // Check if 'T' has a valid stream conversion operator template struct has_stream { @@ -36,7 +37,9 @@ struct get_last{ // Stores information about an AutoConfigVar type struct ConfigRegistryEntry { - ConfigRegistryEntry(const std::type_info& ti, bool has_validator); + typedef std::function(const std::shared_ptr&, const void*)> injector_t; + + ConfigRegistryEntry(const std::type_info& ti, bool has_validator, injector_t&& inject); // Next entry in the list: const ConfigRegistryEntry* const pFlink; @@ -47,9 +50,9 @@ struct ConfigRegistryEntry { // True if a validator was provided const bool m_hasValidator; - // Is this key identify this entry? - bool is(const std::string& key) const; - + // Construct an instance of the AutoConfigVar with this key. + const injector_t m_injector; + // Verify 'ti' is the same type as this entry's value virtual bool verifyType(const std::type_info& ti) const = 0; @@ -77,8 +80,8 @@ struct ConfigRegistryEntryT: // The "key" proper, without the namespace typedef typename get_last::last t_key; - ConfigRegistryEntryT(void): - ConfigRegistryEntry(typeid(ConfigTypeExtractor), has_validate::value) + ConfigRegistryEntryT(injector_t&& constructor) : + ConfigRegistryEntry(typeid(ConfigTypeExtractor), has_validate::value, std::move(constructor)) {} bool verifyType(const std::type_info& ti) const override { @@ -112,7 +115,7 @@ struct ConfigRegistryEntryT: throw autowiring_error("This type doesn't support stream conversions. Define one if you want this to be parsable"); }; - std::function validatorInternal(void) { + std::function validatorInternal(void) const { return[](const T& ptr) { return CallValidate::Call(ptr); }; @@ -128,19 +131,3 @@ struct ConfigRegistryEntryT: extern const ConfigRegistryEntry* g_pFirstConfigEntry; extern size_t g_confgiEntryCount; -/// -/// Adds the specified type to the universal type registry -/// -/// -/// Any instance of this type registry parameterized on type T will be added to the -/// global static type registry, and this registry is computed at link time. -/// -template -class RegConfig -{ -public: - static const ConfigRegistryEntryT r; -}; - -template -const ConfigRegistryEntryT RegConfig::r; diff --git a/src/autowiring/AutoConfigListing.cpp b/src/autowiring/AutoConfigListing.cpp index 0fcbda56f..e27032f0a 100644 --- a/src/autowiring/AutoConfigListing.cpp +++ b/src/autowiring/AutoConfigListing.cpp @@ -81,7 +81,9 @@ bool AutoConfigListing::SetParsed(const std::string& key, const std::string& val if (!s_registry.count(key)) { return false; } + auto parsedValue = s_registry.find(key)->second->parse(value); + SetInternal(key, &(*parsedValue)); return true; } @@ -107,10 +109,23 @@ void AutoConfigListing::AddCallback(t_add_callback&& fx) { } void AutoConfigListing::SetInternal(const std::string& key, const void* value) { + std::unique_lock lk(m_lock); auto config = m_values[key].lock(); - if (!config) - return; + if (!config) { + auto entry = s_registry.find(key); + if (entry == s_registry.end()) { + ThrowKeyNotFoundException(key); + return; + } + + auto ctxt = m_context.lock(); + lk.unlock(); + config = entry->second->m_injector(ctxt, value); + lk.lock(); + m_values[key] = config; + } + lk.unlock(); config->Set(value); } diff --git a/src/autowiring/ConfigRegistry.cpp b/src/autowiring/ConfigRegistry.cpp index 973a0e5c2..0fd1c417b 100644 --- a/src/autowiring/ConfigRegistry.cpp +++ b/src/autowiring/ConfigRegistry.cpp @@ -8,19 +8,16 @@ const ConfigRegistryEntry* g_pFirstConfigEntry = nullptr; size_t g_configEntryCount = 0; -ConfigRegistryEntry::ConfigRegistryEntry(const std::type_info& tinfo, bool has_validator) : +ConfigRegistryEntry::ConfigRegistryEntry(const std::type_info& tinfo, bool hasValidator, injector_t&& inject) : pFlink(g_pFirstConfigEntry), m_key(autowiring::ExtractKey(tinfo)), - m_hasValidator(has_validator) + m_hasValidator(hasValidator), + m_injector(std::move(inject)) { g_configEntryCount++; g_pFirstConfigEntry = this; } -bool ConfigRegistryEntry::is(const std::string& key) const { - return m_key == key; -} - void autowiring::ThrowFailedTypeParseException(const std::string& str, const std::type_info& ti) { std::stringstream msg; msg << "Failed to parse '" << str << "' as type '" << autowiring::demangle(ti) << "'"; From f57a8fc8536aa434948ff19fa95dd4d246611b29 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 18:57:11 -0700 Subject: [PATCH 17/63] Add a test of some undesireable behavior --- src/autowiring/test/AutoConfigTest.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/autowiring/test/AutoConfigTest.cpp b/src/autowiring/test/AutoConfigTest.cpp index ba737c5cc..dd933e04e 100644 --- a/src/autowiring/test/AutoConfigTest.cpp +++ b/src/autowiring/test/AutoConfigTest.cpp @@ -28,6 +28,19 @@ TEST_F(AutoConfigTest, VerifyBasicAssignment) { ASSERT_EQ(*cfg2a, 2) << "Constructor overwrote value when it wasn't supposed to!"; } +//Ideally, this shouldn't even compile. As it stands, it should be at the very least an exception. +TEST_F(AutoConfigTest, DISABLED_VerifySingleUnderlyingType){ + AutoConfig cfg1; + bool threw = false; + try { + AutoConfig cfg2; + } + catch(...){ + threw = true; + } + ASSERT_TRUE(threw) << "Constructing an AutoConfig with the same name and a different type failed to cause an exception"; +} + TEST_F(AutoConfigTest, VerifyBasicSignals) { AutoCurrentContext ctxt; From 07ddf146147b7439ebb63acd8b90c100e50d12f6 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 19:44:47 -0700 Subject: [PATCH 18/63] Make Get return shared_ptr instead of weak pointers. --- autowiring/AutoConfigListing.h | 2 +- src/autowiring/AutoConfigListing.cpp | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/autowiring/AutoConfigListing.h b/autowiring/AutoConfigListing.h index ce5bf75ca..6631197da 100644 --- a/autowiring/AutoConfigListing.h +++ b/autowiring/AutoConfigListing.h @@ -66,7 +66,7 @@ class AutoConfigListing: /// This method will throw an exception if the specified name cannot be found as a configurable value /// in the application, or if the specified value type does not match the type expected by this field /// - std::weak_ptr Get(const std::string& key); + std::shared_ptr Get(const std::string& key); /// /// Assigns the specified value to an AutoConfig with a given key. Creates one if it doesn't exist. diff --git a/src/autowiring/AutoConfigListing.cpp b/src/autowiring/AutoConfigListing.cpp index e27032f0a..73cd3f3ef 100644 --- a/src/autowiring/AutoConfigListing.cpp +++ b/src/autowiring/AutoConfigListing.cpp @@ -66,10 +66,13 @@ bool AutoConfigListing::IsInherited(const std::string& key) { return config && config->IsInherited(); } -std::weak_ptr AutoConfigListing::Get(const std::string& key) { +std::shared_ptr AutoConfigListing::Get(const std::string& key) { std::lock_guard lk(m_lock); - return m_values[key]; - + + auto found = m_values.find(key); + if (found != m_values.end()) + return found->second.lock(); + // Key not found, throw exception std::stringstream ss; ss << "Attepted to get key '" << key << "' which hasn't been set"; From b5fd2c01d1c86985d3d385e9034186865b939d35 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 19:45:46 -0700 Subject: [PATCH 19/63] revise parse() syntax --- autowiring/ConfigRegistry.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/autowiring/ConfigRegistry.h b/autowiring/ConfigRegistry.h index 40c3e0190..2729e9fa8 100644 --- a/autowiring/ConfigRegistry.h +++ b/autowiring/ConfigRegistry.h @@ -56,9 +56,9 @@ struct ConfigRegistryEntry { // Verify 'ti' is the same type as this entry's value virtual bool verifyType(const std::type_info& ti) const = 0; - // Parse a string into this entrie's value type. + // Parse a string into this entry's value type. // Type must have operator>> T defined - virtual AnySharedPointer parse(const std::string&) const = 0; + virtual void parse(AutoConfigVarBase&, const std::string&) const = 0; // Returns function which validates this input. The validator function is // defined as KEY::Validate(const T&) where KEY is the type identifing this entry @@ -90,8 +90,8 @@ struct ConfigRegistryEntryT: // Parse string into this ConfigEntry's type. Throw an exception // if no such stream operator exists - AnySharedPointer parse(const std::string& str) const override { - return AnySharedPointer(std::make_shared(std::move(parseInternal(str)))); + void parse(AutoConfigVarBase& var, const std::string& str) const override { + var.SetParsed(str); } public: From 8c27196e23b4d261ab494a089e11de19d2201a24 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 19:46:56 -0700 Subject: [PATCH 20/63] reduce use of [] on m_values, make onAdded callbacks use the signal system. --- autowiring/AutoConfigListing.h | 7 +-- src/autowiring/AutoConfigListing.cpp | 66 ++++++++++++++-------------- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/autowiring/AutoConfigListing.h b/autowiring/AutoConfigListing.h index 6631197da..8a8301487 100644 --- a/autowiring/AutoConfigListing.h +++ b/autowiring/AutoConfigListing.h @@ -41,9 +41,6 @@ class AutoConfigListing: // list of keys set locally from this context in order of creation. std::vector m_orderedKeys; - // list of callbacks for keys being added to this context. - std::list m_addCallbacks; - // Exception throwers: void ThrowKeyNotFoundException(const std::string& key) const; void ThrowTypeMismatchException(const std::string& key, const std::type_info& ti) const; @@ -112,4 +109,8 @@ class AutoConfigListing: // Set a value in this manager, call callbacks // Must hold m_lock when calling this void SetInternal(const std::string& key, const void* value); + + std::shared_ptr GetOrConstruct(const std::string& key, const void* value); + + autowiring::signal m_onAddedSignal; }; diff --git a/src/autowiring/AutoConfigListing.cpp b/src/autowiring/AutoConfigListing.cpp index 73cd3f3ef..bdbc09897 100644 --- a/src/autowiring/AutoConfigListing.cpp +++ b/src/autowiring/AutoConfigListing.cpp @@ -56,14 +56,16 @@ void AutoConfigListing::ThrowTypeMismatchException(const std::string& key, const bool AutoConfigListing::IsConfigured(const std::string& key) { std::lock_guard lk(m_lock); - auto config = m_values[key].lock(); - return config && config->IsConfigured(); + auto found = m_values.find(key); + + return found != m_values.end() && found->second.lock()->IsConfigured(); } bool AutoConfigListing::IsInherited(const std::string& key) { std::lock_guard lk(m_lock); - auto config = m_values[key].lock(); - return config && config->IsInherited(); + auto found = m_values.find(key); + + return found != m_values.end() && found->second.lock()->IsConfigured(); } std::shared_ptr AutoConfigListing::Get(const std::string& key) { @@ -80,26 +82,17 @@ std::shared_ptr AutoConfigListing::Get(const std::string& key } bool AutoConfigListing::SetParsed(const std::string& key, const std::string& value) { - // Key not found - if (!s_registry.count(key)) { - return false; - } - - auto parsedValue = s_registry.find(key)->second->parse(value); - - SetInternal(key, &(*parsedValue)); + auto config = GetOrConstruct(key, nullptr); + config->SetParsed(value); return true; } void AutoConfigListing::AddOnChanged(const std::string& key, std::function&& fx) { - std::lock_guard lk(m_lock); - auto config = m_values[key].lock(); - if (config) - (*config).onChangedSignal += std::move(fx); + auto config = GetOrConstruct(key, nullptr); + (*config).onChangedSignal += std::move(fx); } void AutoConfigListing::AddCallback(t_add_callback&& fx) { - // Grab lock until done setting value std::lock_guard lk(m_lock); for (auto& key : m_orderedKeys) { @@ -108,27 +101,32 @@ void AutoConfigListing::AddCallback(t_add_callback&& fx) { fx(*config); } - m_addCallbacks.emplace_back(std::move(fx)); + m_onAddedSignal += std::move(fx); } void AutoConfigListing::SetInternal(const std::string& key, const void* value) { - std::unique_lock lk(m_lock); - auto config = m_values[key].lock(); - - if (!config) { - auto entry = s_registry.find(key); - if (entry == s_registry.end()) { - ThrowKeyNotFoundException(key); - return; - } + auto config = GetOrConstruct(key, value); + config->Set(value); +} + +std::shared_ptr AutoConfigListing::GetOrConstruct(const std::string& key, const void* value) { + std::unique_lock lk(m_lock); + + auto found = m_values.find(key); + if (found != m_values.end()) + return found->second.lock(); - auto ctxt = m_context.lock(); - lk.unlock(); - config = entry->second->m_injector(ctxt, value); - lk.lock(); - m_values[key] = config; + auto entry = s_registry.find(key); + if (entry == s_registry.end()) { + ThrowKeyNotFoundException(key); + return nullptr; } + auto ctxt = m_context.lock(); lk.unlock(); - config->Set(value); -} + auto newValue = entry->second->m_injector(ctxt, value); + lk.lock(); + + m_values.emplace(key, newValue); + return newValue; +} \ No newline at end of file From e58d64214e6aa93d683af9fbd69cf91df5ccdc83 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 20:02:19 -0700 Subject: [PATCH 21/63] Make AutoConfigVars register themselves with the AutoConfigListing in the same context. --- autowiring/AutoConfigBase.h | 1 + autowiring/AutoConfigListing.h | 3 +++ src/autowiring/AutoConfigBase.cpp | 7 +++++++ src/autowiring/AutoConfigListing.cpp | 10 ++++++++-- src/autowiring/test/AutoConfigListingTest.cpp | 19 ++++--------------- 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/autowiring/AutoConfigBase.h b/autowiring/AutoConfigBase.h index 88dfd3cb4..7e62475e9 100644 --- a/autowiring/AutoConfigBase.h +++ b/autowiring/AutoConfigBase.h @@ -17,6 +17,7 @@ class AutoConfigVarBase : public ContextMember { public: AutoConfigVarBase(const std::type_info& tiName, bool configured = false); + void AutoInit(); // Key used to identify this config value const std::string m_key; diff --git a/autowiring/AutoConfigListing.h b/autowiring/AutoConfigListing.h index 8a8301487..1339c1502 100644 --- a/autowiring/AutoConfigListing.h +++ b/autowiring/AutoConfigListing.h @@ -113,4 +113,7 @@ class AutoConfigListing: std::shared_ptr GetOrConstruct(const std::string& key, const void* value); autowiring::signal m_onAddedSignal; + + friend class AutoConfigVarBase; + void NotifyConfigAdded(const std::shared_ptr& cfg); }; diff --git a/src/autowiring/AutoConfigBase.cpp b/src/autowiring/AutoConfigBase.cpp index 80017ce25..6c6f603f6 100644 --- a/src/autowiring/AutoConfigBase.cpp +++ b/src/autowiring/AutoConfigBase.cpp @@ -2,6 +2,8 @@ #include "stdafx.h" #include "AutoConfigBase.h" #include "AutoConfigParser.hpp" +#include "AutoConfigListing.h" +#include "CoreContext.h" AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti, bool configured) : ContextMember(), @@ -13,3 +15,8 @@ AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti, bool configured) m_name = m_key.c_str(); } +void AutoConfigVarBase::AutoInit() { + auto ctxt = m_context.lock(); + auto listing = ctxt->Inject(); + listing->NotifyConfigAdded(GetSelf()); +} diff --git a/src/autowiring/AutoConfigListing.cpp b/src/autowiring/AutoConfigListing.cpp index bdbc09897..fbc032985 100644 --- a/src/autowiring/AutoConfigListing.cpp +++ b/src/autowiring/AutoConfigListing.cpp @@ -1,8 +1,9 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #include "stdafx.h" -#include "AutoConfig.h" +#include "AutoConfigBase.h" #include "AutoConfigListing.h" -#include "AnySharedPointer.h" +#include "AutoConfigParser.hpp" +#include "demangle.h" #include using namespace autowiring; @@ -129,4 +130,9 @@ std::shared_ptr AutoConfigListing::GetOrConstruct(const std:: m_values.emplace(key, newValue); return newValue; +} + +void AutoConfigListing::NotifyConfigAdded(const std::shared_ptr& config){ + std::lock_guard lk(m_lock); + m_values.emplace(config->m_key, config); } \ No newline at end of file diff --git a/src/autowiring/test/AutoConfigListingTest.cpp b/src/autowiring/test/AutoConfigListingTest.cpp index 0552a0593..b3a32ecd9 100644 --- a/src/autowiring/test/AutoConfigListingTest.cpp +++ b/src/autowiring/test/AutoConfigListingTest.cpp @@ -21,7 +21,7 @@ TEST_F(AutoConfigListingTest, VerifySimpleAssignment) { AutoConfig cfg; ASSERT_EQ(323, *cfg) << "Configurable type did not receive a value as expected"; } -/* + struct NamespaceRoot; struct NamespaceChild; @@ -37,20 +37,9 @@ struct MyBoolClass { AutoConfig m_bool; }; -TEST_F(AutoConfigListingTest, VerifyBool) { - AutoRequired acm; - AutoRequired clz1; - - acm->Set("bool_space.my_bool", true); - ASSERT_TRUE(*clz1->m_bool); - - acm->SetParsed("bool_space.my_bool", "false"); - ASSERT_FALSE(*clz1->m_bool); -} - TEST_F(AutoConfigListingTest, VerifyPostHocAssignment) { // Inject the configurable type first - AutoRequired mcc; + AutoConfig cfg; // Configuration manager must exist at this point as a consequence of the earlier construction Autowired acm; @@ -59,9 +48,9 @@ TEST_F(AutoConfigListingTest, VerifyPostHocAssignment) { acm->Set("Namespace1.XYZ", 323); // Now inject the type which expects this value to be assigned: - ASSERT_EQ(323, *mcc->m_myName) << "Configurable type did not receive a value as expected"; + ASSERT_EQ(323, *cfg) << "Configurable type did not receive a value as expected"; } - +/* TEST_F(AutoConfigListingTest, VerifyRecursiveSearch) { AutoRequired acm; acm->Set("Namespace1.XYZ", 1001); From 8109ba3cb0899abfe5166dc28b9730337636bdaf Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 20:21:55 -0700 Subject: [PATCH 22/63] fix typedef names to be more in-line with naming conventions, make AddCallback return a registration. --- autowiring/AutoConfig.h | 4 ++-- autowiring/AutoConfigBase.h | 2 +- autowiring/AutoConfigListing.h | 10 +++++----- autowiring/auto_signal.h | 8 +++++--- src/autowiring/AutoConfigListing.cpp | 4 ++-- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index 6e712345b..bb9ca36a1 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -86,13 +86,13 @@ class AutoConfigVar: } // Add a callback for when this config value changes - t_OnChangedSignal::t_registration* operator+=(std::function&& fx) { + t_OnChangedSignal::registration_t* operator+=(std::function&& fx) { return onChangedSignal += [fx](const AutoConfigVarBase& var){ fx(reinterpret_cast*>(&var)->m_value); }; } - void operator-=(t_OnChangedSignal::t_registration* node) { onChangedSignal -= node; } + void operator-=(t_OnChangedSignal::registration_t* node) { onChangedSignal -= node; } private: T m_value; diff --git a/autowiring/AutoConfigBase.h b/autowiring/AutoConfigBase.h index 7e62475e9..d917dfc9d 100644 --- a/autowiring/AutoConfigBase.h +++ b/autowiring/AutoConfigBase.h @@ -34,5 +34,5 @@ class AutoConfigVarBase : public ContextMember protected: bool m_isConfigured; - t_OnChangedSignal::t_registration* m_parentRegistration; + t_OnChangedSignal::registration_t* m_parentRegistration; }; diff --git a/autowiring/AutoConfigListing.h b/autowiring/AutoConfigListing.h index 1339c1502..bbef06bb7 100644 --- a/autowiring/AutoConfigListing.h +++ b/autowiring/AutoConfigListing.h @@ -19,9 +19,9 @@ class AutoConfigListing: AutoConfigListing(); virtual ~AutoConfigListing(); - // Callback function type - typedef std::function t_add_callback; - + // Callback signal type + typedef autowiring::signal onAddSignal_t; + // Validator function type typedef std::function t_validator; @@ -100,7 +100,7 @@ class AutoConfigListing: // Add a callback for when a key is set in this context. Is immediately called on all // currently existing values in the order they were created. - void AddCallback(t_add_callback&& fx); + onAddSignal_t::registration_t* AddCallback(onAddSignal_t::function_t&& fx); // Returns a list of all keys which have been set from this context. const std::vector& GetLocalKeys() const { return m_orderedKeys; } @@ -112,7 +112,7 @@ class AutoConfigListing: std::shared_ptr GetOrConstruct(const std::string& key, const void* value); - autowiring::signal m_onAddedSignal; + onAddSignal_t m_onAddedSignal; friend class AutoConfigVarBase; void NotifyConfigAdded(const std::shared_ptr& cfg); diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index d18975a60..34d3a81ee 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -198,9 +198,11 @@ namespace autowiring { const std::shared_ptr> m_relay; public: - typedef internal::signal_node t_registration; - t_registration* operator+=(std::function&& fn) { return *m_relay += std::move(fn); } - void operator-=(t_registration* node) { return *m_relay -= node; } + typedef internal::signal_node registration_t; + typedef std::function function_t; + + registration_t* operator+=(function_t&& fn) { return *m_relay += std::move(fn); } + void operator-=(registration_t* node) { return *m_relay -= node; } /// /// Raises the signal and invokes all attached handlers diff --git a/src/autowiring/AutoConfigListing.cpp b/src/autowiring/AutoConfigListing.cpp index fbc032985..6db950907 100644 --- a/src/autowiring/AutoConfigListing.cpp +++ b/src/autowiring/AutoConfigListing.cpp @@ -93,7 +93,7 @@ void AutoConfigListing::AddOnChanged(const std::string& key, std::function lk(m_lock); for (auto& key : m_orderedKeys) { @@ -102,7 +102,7 @@ void AutoConfigListing::AddCallback(t_add_callback&& fx) { fx(*config); } - m_onAddedSignal += std::move(fx); + return m_onAddedSignal += std::move(fx); } void AutoConfigListing::SetInternal(const std::string& key, const void* value) { From 1bcd134c466f1a3e097bb1f4c7fda1490d9a24e3 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Wed, 25 Mar 2015 20:22:10 -0700 Subject: [PATCH 23/63] Fix the remainder of the AutoConfigListingTests --- src/autowiring/test/AutoConfigListingTest.cpp | 136 ++++++------------ 1 file changed, 46 insertions(+), 90 deletions(-) diff --git a/src/autowiring/test/AutoConfigListingTest.cpp b/src/autowiring/test/AutoConfigListingTest.cpp index b3a32ecd9..93c2d557c 100644 --- a/src/autowiring/test/AutoConfigListingTest.cpp +++ b/src/autowiring/test/AutoConfigListingTest.cpp @@ -50,7 +50,7 @@ TEST_F(AutoConfigListingTest, VerifyPostHocAssignment) { // Now inject the type which expects this value to be assigned: ASSERT_EQ(323, *cfg) << "Configurable type did not receive a value as expected"; } -/* + TEST_F(AutoConfigListingTest, VerifyRecursiveSearch) { AutoRequired acm; acm->Set("Namespace1.XYZ", 1001); @@ -60,14 +60,14 @@ TEST_F(AutoConfigListingTest, VerifyRecursiveSearch) { CurrentContextPusher pshr(ctxt); // Now inject an element here, and verify that it was wired up as expected: - AutoRequired mcc; - ASSERT_TRUE(mcc->m_myName->IsConfigured()) << "A configurable value was not configured as expected"; - ASSERT_EQ(1001, *mcc->m_myName) << "Configurable value obtained from a parent scope did not have the correct value"; + AutoConfig cfg; + ASSERT_TRUE(cfg->IsConfigured()) << "A configurable value was not configured as expected"; + ASSERT_EQ(*cfg, 1001) << "Configurable value obtained from a parent scope did not have the correct value"; // This must work as expected--a local context override will rewrite configuration values in the local scope AutoRequired sub_mcc; sub_mcc->Set("Namespace1.XYZ", 1002); - ASSERT_EQ(1002, *mcc->m_myName) << "Override of a configurable value in a derived class did not take place as expected"; + ASSERT_EQ(1002, *cfg) << "Override of a configurable value in a derived class did not take place as expected"; } } @@ -81,7 +81,7 @@ TEST_F(AutoConfigListingTest, DefaultNamespace) { AutoRequired def; - ASSERT_EQ(123, *def->m_def); + ASSERT_EQ(*def->m_def, 123); } TEST_F(AutoConfigListingTest, VerifyParsedAssignment) { @@ -102,11 +102,11 @@ TEST_F(AutoConfigListingTest, VerifyDuplicateConfigAssignment) { ASSERT_TRUE(acm->SetParsed("Namespace1.XYZ", "324")); ASSERT_TRUE(acm->SetParsed("Namespace2.XYZ", "1111")); - AutoRequired clz1; - AutoRequired clz2; + AutoConfig clz1; + AutoConfig clz2; - ASSERT_EQ(324, *clz1->m_myName); - ASSERT_EQ(1111, *clz2->m_myName); + ASSERT_EQ(*clz1, 324); + ASSERT_EQ(*clz2, 1111); } class TypeWithoutAShiftOperator { @@ -134,26 +134,33 @@ TEST_F(AutoConfigListingTest, TypeWithoutAShiftOperatorTest) { tasoVal.foo = 592; mgr->Set("MyNoShift", tasoVal); - ASSERT_EQ(592, noshift->m_noshift->foo) << "Value assignment did not result in an update to a non-serializable configuration field"; + ASSERT_EQ((*noshift->m_noshift)->foo, 592) << "Value assignment did not result in an update to a non-serializable configuration field"; } TEST_F(AutoConfigListingTest, Callbacks) { AutoRequired acm; - AutoRequired mcc; + AutoConfig cfg; acm->Set("Namespace1.XYZ", 4); - mcc->m_myName += [](int val) { + *cfg += [](int val) { ASSERT_EQ(val, 42); }; - mcc->m_myName += [](int val) { + *cfg += [](int val) { ASSERT_EQ(val, 42); }; acm->Set("Namespace1.XYZ", 42); } +struct MyConfigurableClass { + AutoConfig cfg; +}; +struct MyConfigurableClass2 { + AutoConfig cfg; +}; + TEST_F(AutoConfigListingTest, NestedContexts) { // Set up contexts and members AutoCurrentContext ctxt_outer; @@ -175,46 +182,46 @@ TEST_F(AutoConfigListingTest, NestedContexts) { // Set initial value acm_outer->Set("Namespace1.XYZ", 42); - ASSERT_EQ(42, *mcc_outer->m_myName) << "Config value not set"; - ASSERT_EQ(42, *mcc_middle->m_myName) << "Config value not set in descendant context"; - ASSERT_EQ(42, *mcc_sibling->m_myName) << "Config value not set in descendant context"; - ASSERT_EQ(42, *mcc_inner->m_myName) << "Config value not set in descendant context"; - ASSERT_EQ(42, *mcc_leaf->m_myName) << "Config value not set in desendant context"; + ASSERT_EQ(42, *mcc_outer->cfg) << "Config value not set"; + ASSERT_EQ(42, *mcc_middle->cfg) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_sibling->cfg) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_inner->cfg) << "Config value not set in descendant context"; + ASSERT_EQ(42, *mcc_leaf->cfg) << "Config value not set in desendant context"; EXPECT_TRUE(acm_middle->IsInherited("Namespace1.XYZ")) << "Inherited key not marked as such"; EXPECT_TRUE(acm_leaf->IsInherited("Namespace1.XYZ")) << "Inherited key not marked as such"; // Set middle, inner shouldn't be able to be set from outer after this bool callback_hit1 = false; - mcc_inner->m_myName += [&callback_hit1](int) { + *mcc_inner->cfg += [&callback_hit1](int) { callback_hit1 = true; }; acm_middle->Set("Namespace1.XYZ", 1337); - ASSERT_EQ(42, *mcc_outer->m_myName) << "Config value changed in outer context"; - ASSERT_EQ(42, *mcc_sibling->m_myName) << "Config value set from sibling context"; - ASSERT_EQ(1337, *mcc_middle->m_myName) << "Config value not set"; - ASSERT_EQ(1337, *mcc_inner->m_myName) << "Config value not set in child context"; - ASSERT_EQ(1337, *mcc_leaf->m_myName) << "Config value not set in leaf context"; + ASSERT_EQ(42, *mcc_outer->cfg) << "Config value changed in outer context"; + ASSERT_EQ(42, *mcc_sibling->cfg) << "Config value set from sibling context"; + ASSERT_EQ(1337, *mcc_middle->cfg) << "Config value not set"; + ASSERT_EQ(1337, *mcc_inner->cfg) << "Config value not set in child context"; + ASSERT_EQ(1337, *mcc_leaf->cfg) << "Config value not set in leaf context"; ASSERT_TRUE(callback_hit1) << "Callback not hit in inner context"; // Set from outter, inner should be shielded by middle context - mcc_inner->m_myName += [](int) { + *mcc_inner->cfg += [](int) { FAIL() << "This callback should never be hit"; }; // Make sure sibling context is not shielded bool callback_hit2 = false; - mcc_sibling->m_myName += [&callback_hit2](int) { + *mcc_sibling->cfg += [&callback_hit2](int) { callback_hit2 = true; }; // Set from outer, shouldn't effect middle or inner contexts acm_outer->Set("Namespace1.XYZ", 999); - ASSERT_EQ(999, *mcc_outer->m_myName) << "Config value not set"; - ASSERT_EQ(1337, *mcc_middle->m_myName) << "Config value overwritten when value was set in this context"; - ASSERT_EQ(1337, *mcc_inner->m_myName) << "Config value overwritten when value was set in parent context"; + ASSERT_EQ(999, *mcc_outer->cfg) << "Config value not set"; + ASSERT_EQ(1337, *mcc_middle->cfg) << "Config value overwritten when value was set in this context"; + ASSERT_EQ(1337, *mcc_inner->cfg) << "Config value overwritten when value was set in parent context"; // Make sure sibling hit - ASSERT_EQ(999, *mcc_sibling->m_myName) << "Value not set on sibling of context where value was previously set"; + ASSERT_EQ(999, *mcc_sibling->cfg) << "Value not set on sibling of context where value was previously set"; ASSERT_TRUE(callback_hit2) << "Callback not called on sibling of context where value was previously set"; } @@ -245,56 +252,6 @@ TEST_F(AutoConfigListingTest, Validators) { ASSERT_EQ(1337, *valid->m_config) << "Value not set for key"; } -TEST_F(AutoConfigListingTest, DirectAssignemnt) { - AutoConfig var; - var = 10; - ASSERT_EQ(10, *var); - - AutoRequired containsVar; - - ASSERT_EQ(10, *var); - ASSERT_EQ(10, *containsVar->m_myName); -} - - -struct ComplexValue { - int a; - int b; - int c; - - ComplexValue(int nA, int nB, int nC) : a(nA), b(nB), c(nC) {} - ComplexValue(int repeated) : a(repeated), b(repeated), c(repeated) {} -}; - -struct MyComplexValueClass { - AutoConfig m_cfg; - AutoConfig m_cfg2 = ComplexValue{ 10, 15, 30 }; - - MyComplexValueClass() : m_cfg(ComplexValue{ 2, 20, 20 }) {} -}; - -TEST_F(AutoConfigListingTest, ComplexConstruction){ - AutoRequired mgr; - ASSERT_FALSE(mgr->IsConfigured("Namespace1.MyCxValue")); - - AutoConfig defaultConstructed; - - ASSERT_FALSE(mgr->IsConfigured("Namespace1.MyCxValue")) << "Improperly set config value when default constructing AutoConfig"; - - AutoConfig fancyConstructed(1, 2, 3); - - ASSERT_TRUE(mgr->IsConfigured("Namespace1.MyCxValue")) << "Initializing constructor did not set config value"; - ASSERT_EQ(fancyConstructed->a, 1) << "Initializing constructor did not set config value"; - ASSERT_EQ(fancyConstructed->b, 2) << "Initializing constructor did not set config value"; - ASSERT_EQ(fancyConstructed->c, 3) << "Initializing constructor did not set config value"; - - - AutoConfig fancy2(7); - ASSERT_EQ(fancy2->a, 1) << "Second Initalizing constructor overrode the first!"; - ASSERT_EQ(fancy2->b, 2) << "Second Initalizing constructor overrode the first!"; - ASSERT_EQ(fancy2->c, 3) << "Second Initalizing constructor overrode the first!"; -} - struct OuterCtxt{}; struct MiddleCtxt{}; struct InnerCtxt{}; @@ -307,31 +264,31 @@ TEST_F(AutoConfigListingTest, ListingConfigs) { AutoRequired acm_inner(ctxt_inner); AutoRequired var1_inner(ctxt_inner); - var1_inner->m_myName = 1; + *var1_inner->cfg = 1; ASSERT_EQ(0, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; int callback_outer = 0; - acm_outer->AddCallback([&callback_outer](const std::string& key, const AnySharedPointer& ptr) { + acm_outer->AddCallback([&callback_outer](const AutoConfigVarBase& ptr) { ++callback_outer; }); int callback_inner = 0; - acm_inner->AddCallback([&callback_inner](const std::string& key, const AnySharedPointer& ptr) { + acm_inner->AddCallback([&callback_inner](const AutoConfigVarBase& ptr) { ++callback_inner; }); ASSERT_EQ(1, callback_inner) << "Callback not called on existing keys"; AutoRequired var1_outer(ctxt_outer); - var1_outer->m_myName = 2; + *var1_outer->cfg = 2; ASSERT_EQ(1, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; AutoRequired var2_outer(ctxt_outer); - var2_outer->m_myName = 3; + var2_outer->cfg = 3; ASSERT_EQ(2, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; @@ -340,10 +297,9 @@ TEST_F(AutoConfigListingTest, ListingConfigs) { ASSERT_EQ(1, callback_inner) << "Inner callback called an incorrect number of times"; auto keys_outer = acm_outer->GetLocalKeys(); - ASSERT_EQ(var1_outer->m_myName.m_key, keys_outer[0]) << "Keys listed out of construction order"; - ASSERT_EQ(var2_outer->m_myName.m_key, keys_outer[1]) << "Keys listed out of construction order"; + ASSERT_EQ(var1_outer->cfg->m_key, keys_outer[0]) << "Keys listed out of construction order"; + ASSERT_EQ(var2_outer->cfg->m_key, keys_outer[1]) << "Keys listed out of construction order"; auto keys_inner = acm_inner->GetLocalKeys(); - ASSERT_EQ(var1_inner->m_myName.m_key, keys_inner[0]) << "Keys listed out of construction order"; + ASSERT_EQ(var1_inner->cfg->m_key, keys_inner[0]) << "Keys listed out of construction order"; } -*/ \ No newline at end of file From dd72aaecc1572695086297328cc97e1f1f2841cb Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Thu, 26 Mar 2015 11:39:18 -0700 Subject: [PATCH 24/63] Fix minor compiler error on mac --- autowiring/AutoConfig.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index bb9ca36a1..b6634a3e0 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -82,7 +82,7 @@ class AutoConfigVar: void Set(const void* pValue) override { *this = *reinterpret_cast(pValue); } void SetParsed(const std::string& value) override { - *this = RegistryEntry.parseInternal(value); + *this = RegistryEntry.template parseInternal(value); } // Add a callback for when this config value changes From 4035c722a0f9956a731405206b40b75c08d031fc Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Thu, 26 Mar 2015 15:08:30 -0700 Subject: [PATCH 25/63] Fix Bad include --- autowiring/AutoConfigBase.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autowiring/AutoConfigBase.h b/autowiring/AutoConfigBase.h index d917dfc9d..63102d443 100644 --- a/autowiring/AutoConfigBase.h +++ b/autowiring/AutoConfigBase.h @@ -4,7 +4,7 @@ #include "auto_signal.h" #include -#include +#include "C++11/cpp11.h" #include TYPE_INDEX_HEADER struct AnySharedPointer; From 3568cf23027135b71873ab4eddb7dcbae1ed2619 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Thu, 26 Mar 2015 16:57:53 -0700 Subject: [PATCH 26/63] Update listing test to deal with some more edge cases --- src/autowiring/test/AutoConfigListingTest.cpp | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/autowiring/test/AutoConfigListingTest.cpp b/src/autowiring/test/AutoConfigListingTest.cpp index 93c2d557c..b6de2242c 100644 --- a/src/autowiring/test/AutoConfigListingTest.cpp +++ b/src/autowiring/test/AutoConfigListingTest.cpp @@ -263,12 +263,6 @@ TEST_F(AutoConfigListingTest, ListingConfigs) { AutoRequired acm_outer(ctxt_outer); AutoRequired acm_inner(ctxt_inner); - AutoRequired var1_inner(ctxt_inner); - *var1_inner->cfg = 1; - - ASSERT_EQ(0, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; - ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; - int callback_outer = 0; acm_outer->AddCallback([&callback_outer](const AutoConfigVarBase& ptr) { ++callback_outer; @@ -279,23 +273,37 @@ TEST_F(AutoConfigListingTest, ListingConfigs) { ++callback_inner; }); + AutoRequired var1_inner(ctxt_inner); + *var1_inner->cfg = 1; + + ASSERT_EQ(0, callback_outer) << "OnAdded fired incorrectly"; + ASSERT_EQ(1, callback_inner) << "Callback fired incorrectly"; + ASSERT_EQ(0, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; + ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; + ASSERT_EQ(1, callback_inner) << "Callback not called on existing keys"; - AutoRequired var1_outer(ctxt_outer); + AutoRequired var1_outer(ctxt_outer); *var1_outer->cfg = 2; ASSERT_EQ(1, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; - AutoRequired var2_outer(ctxt_outer); - var2_outer->cfg = 3; + AutoRequired var2_outer(ctxt_outer); + AutoRequired var2_inner(ctxt_inner); - ASSERT_EQ(2, acm_outer->GetLocalKeys().size()) << "Incorrect number of keys found in outer context"; - ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of keys found in inner context"; + ASSERT_EQ(1, acm_outer->GetLocalKeys().size()) << "Constructing an uninitialized config incremented the local count"; + + *var2_outer->cfg = 3; + + ASSERT_EQ(2, acm_outer->GetLocalKeys().size()) << "Incorrect number of local keys found in outer context"; + ASSERT_EQ(1, acm_inner->GetLocalKeys().size()) << "Incorrect number of local keys found in inner context"; ASSERT_EQ(2, callback_outer) << "Outer callback called an incorrect number of times"; ASSERT_EQ(1, callback_inner) << "Inner callback called an incorrect number of times"; + ASSERT_EQ(*var2_inner->cfg, *var2_outer->cfg) << "Value did not get set in child context"; + auto keys_outer = acm_outer->GetLocalKeys(); ASSERT_EQ(var1_outer->cfg->m_key, keys_outer[0]) << "Keys listed out of construction order"; ASSERT_EQ(var2_outer->cfg->m_key, keys_outer[1]) << "Keys listed out of construction order"; From bacadc99b193fcb4a5f6a039da09e76884eba872 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Thu, 26 Mar 2015 17:00:22 -0700 Subject: [PATCH 27/63] Refactor & fix m_orderedKeys not being set properly --- autowiring/AutoConfig.h | 30 ++++++++++++++++++++++------ autowiring/AutoConfigBase.h | 5 +++++ autowiring/AutoConfigListing.h | 1 + src/autowiring/AutoConfigBase.cpp | 12 ++++++++++- src/autowiring/AutoConfigListing.cpp | 9 ++++++++- 5 files changed, 49 insertions(+), 8 deletions(-) diff --git a/autowiring/AutoConfig.h b/autowiring/AutoConfig.h index b6634a3e0..11625ba41 100644 --- a/autowiring/AutoConfig.h +++ b/autowiring/AutoConfig.h @@ -52,12 +52,15 @@ class AutoConfigVar: if (parent != nullptr) { auto parentVar = parent->template Inject>(); - //Only copy the value if it's initalized + //Only copy the value if it's initalized. Base::AutoInit will take care of + //the various listing notifications. if (parentVar->IsConfigured()) { - SetInternal(parentVar->m_value); + m_isConfigured = true; + m_value = parentVar->m_value; } m_parentRegistration = *parentVar += [this](const T& val){ + RunValidation(val); SetInternal(val); }; } @@ -69,13 +72,17 @@ class AutoConfigVar: const T* operator->() const { return &m_value; } void operator=(const T& newValue) { - SetInternal(newValue); + RunValidation(newValue); + if (m_parentRegistration) { auto parent_ctxt = m_context.lock()->GetParentContext(); AutowiredFast> parentVar(parent_ctxt); *parentVar -= m_parentRegistration; m_parentRegistration = nullptr; + OnSetLocally(); } + + SetInternal(newValue); } void Get(void* pValue) const override { *reinterpret_cast(pValue) = m_value; } @@ -97,13 +104,15 @@ class AutoConfigVar: private: T m_value; - void SetInternal(const T& val) { + void RunValidation(const T& val) { if (RegistryEntry.m_hasValidator) { if (!RegistryEntry.validatorInternal()(val)) { throw autowiring_error("Validator rejected set for config value"); } } + } + void SetInternal(const T& val) { m_isConfigured = true; m_value = val; onChangedSignal(*this); @@ -149,11 +158,20 @@ class AutoConfig : public AutoRequired> { { } + AutoConfig(T&& initialValue, const std::shared_ptr& ctxt = CoreContext::CurrentContext()) : + AutoRequired(ctxt, std::move(initialValue)) + { + if (!(*this)->IsLocal()) { + **this = std::move(initialValue); + } + } + template - AutoConfig(t_Arg&& arg, t_Args&&... args) : + explicit AutoConfig(t_Arg&& arg, t_Args&&... args) : AutoRequired(CoreContext::CurrentContext(), std::forward(arg), std::forward(args)...) { - if (!(*this)->IsConfigured() || (*this)->IsInherited()) { + //If we wind up being a reference to an existing value, we may still want to set it... + if (!(*this)->IsLocal()) { **this = T(std::forward(arg), std::forward(args)...); } } diff --git a/autowiring/AutoConfigBase.h b/autowiring/AutoConfigBase.h index 63102d443..0fbb06dfe 100644 --- a/autowiring/AutoConfigBase.h +++ b/autowiring/AutoConfigBase.h @@ -22,8 +22,11 @@ class AutoConfigVarBase : public ContextMember // Key used to identify this config value const std::string m_key; + // True if this config was set at all bool IsConfigured() const { return m_isConfigured; } bool IsInherited() const { return m_parentRegistration != nullptr; } + //True if the config was set from within this context (isn't inherited) + bool IsLocal() const { return IsConfigured() && !IsInherited(); } typedef autowiring::signal t_OnChangedSignal; t_OnChangedSignal onChangedSignal; @@ -33,6 +36,8 @@ class AutoConfigVarBase : public ContextMember virtual void SetParsed(const std::string& value) = 0; protected: + void OnSetLocally(); + bool m_isConfigured; t_OnChangedSignal::registration_t* m_parentRegistration; }; diff --git a/autowiring/AutoConfigListing.h b/autowiring/AutoConfigListing.h index bbef06bb7..e143644cd 100644 --- a/autowiring/AutoConfigListing.h +++ b/autowiring/AutoConfigListing.h @@ -116,4 +116,5 @@ class AutoConfigListing: friend class AutoConfigVarBase; void NotifyConfigAdded(const std::shared_ptr& cfg); + void NotifySetLocally(const std::shared_ptr& cfg); }; diff --git a/src/autowiring/AutoConfigBase.cpp b/src/autowiring/AutoConfigBase.cpp index 6c6f603f6..bd855f32f 100644 --- a/src/autowiring/AutoConfigBase.cpp +++ b/src/autowiring/AutoConfigBase.cpp @@ -18,5 +18,15 @@ AutoConfigVarBase::AutoConfigVarBase(const std::type_info& ti, bool configured) void AutoConfigVarBase::AutoInit() { auto ctxt = m_context.lock(); auto listing = ctxt->Inject(); - listing->NotifyConfigAdded(GetSelf()); + auto self = GetSelf(); + + listing->NotifyConfigAdded(self); + if (!IsInherited()) + listing->NotifySetLocally(self); } + +void AutoConfigVarBase::OnSetLocally() { + auto ctxt = m_context.lock(); + auto listing = ctxt->Inject(); //get or set + listing->NotifySetLocally(GetSelf()); +} \ No newline at end of file diff --git a/src/autowiring/AutoConfigListing.cpp b/src/autowiring/AutoConfigListing.cpp index 6db950907..068bb28d3 100644 --- a/src/autowiring/AutoConfigListing.cpp +++ b/src/autowiring/AutoConfigListing.cpp @@ -128,11 +128,18 @@ std::shared_ptr AutoConfigListing::GetOrConstruct(const std:: auto newValue = entry->second->m_injector(ctxt, value); lk.lock(); - m_values.emplace(key, newValue); return newValue; } void AutoConfigListing::NotifyConfigAdded(const std::shared_ptr& config){ std::lock_guard lk(m_lock); m_values.emplace(config->m_key, config); +} + +void AutoConfigListing::NotifySetLocally(const std::shared_ptr& config) { + { + std::lock_guard lk(m_lock); + m_orderedKeys.push_back(config->m_key); + } + m_onAddedSignal(*config); } \ No newline at end of file From 4b1dff5e3c52fd113b5b3ee6732e30e81b90d674 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 26 Mar 2015 17:26:07 -0700 Subject: [PATCH 28/63] Bump autowiring version number --- Doxyfile | 2 +- publicDoxyfile.conf | 2 +- version.cmake | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Doxyfile b/Doxyfile index e73312580..eb0d4511c 100644 --- a/Doxyfile +++ b/Doxyfile @@ -32,7 +32,7 @@ PROJECT_NAME = Autowiring # This could be handy for archiving the generated documentation or # if some version control system is used. -PROJECT_NUMBER = "0.5.1" +PROJECT_NUMBER = "0.5.2" # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer diff --git a/publicDoxyfile.conf b/publicDoxyfile.conf index 1388a2fd6..36894ba8b 100644 --- a/publicDoxyfile.conf +++ b/publicDoxyfile.conf @@ -38,7 +38,7 @@ PROJECT_NAME = Autowiring # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 0.5.1 +PROJECT_NUMBER = 0.5.2 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/version.cmake b/version.cmake index fbbe55b7b..2084c19e0 100644 --- a/version.cmake +++ b/version.cmake @@ -1 +1 @@ -set(autowiring_VERSION 0.5.1) +set(autowiring_VERSION 0.5.2) From 000abe60625bde8a51941314c5f87437df9d7006 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 26 Mar 2015 18:06:28 -0700 Subject: [PATCH 29/63] Enhance documentation and fix teardown listener behavior for BasicThread Add documentation to describe exactly when teardown listeners are raised for `BasicThread`. Presently, the documentation for this method leaves things rather open-ended. Also change NotifyTeardownListeners so it is invoked when the owning type is still a `BasicThread`, and only invoke `NotifyTeardownListeners` after the `BasicThread` has transitioned to the stopped state. --- autowiring/BasicThread.h | 20 ++++++++++++++++++++ autowiring/ContextMember.h | 4 +++- src/autowiring/BasicThread.cpp | 10 ++++++---- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/autowiring/BasicThread.h b/autowiring/BasicThread.h index d9a11a686..b53e89cd4 100644 --- a/autowiring/BasicThread.h +++ b/autowiring/BasicThread.h @@ -232,6 +232,26 @@ class BasicThread: /// bool IsCompleted(void) const; + /// + /// Adds a function object which will be called when this BasicThread stops running or is destroyed + /// + /// + /// The listener is invoked before the destruction of this BasicThread and also immediately after the + /// BasicThread instance has transitioned to the Stopped state. + /// + /// Users who attach a teardown listener MUST NOT attempt to invoke any methods not defined by + /// BasicThread, and MUST NOT attempt to invoke any non-final virtual functions available here. The + /// object itself may be partially destroyed by the time the listener is invoked, and virtual methods + /// may not have the expected behavior. + /// + /// It is guaranteed to be safe to call any non-virtual method defined by BasicThread from a teardown + /// listener. + /// + template + void AddTeardownListener(Fx&& listener) { + return TeardownNotifier::AddTeardownListener(std::forward(listener)); + } + /// /// Begins thread execution. /// diff --git a/autowiring/ContextMember.h b/autowiring/ContextMember.h index a949b2100..56d280f26 100644 --- a/autowiring/ContextMember.h +++ b/autowiring/ContextMember.h @@ -38,7 +38,9 @@ class ContextMember: /// /// A context may be destroyed if and only if none of its members are running and none of /// them may enter a runnable state. This happens when the last pointer to ContextMember - /// is lost. Resource cleanup must be started at this point. + /// is lost. Resource cleanup must be started at this point. Context members are deemed + /// to be unable to enter a running state if they were not signalled to enter this state + /// before the last shared pointer to their outer CoreContext is released. /// /// For contexts containing strictly heirarchial objects, implementors of this method do /// not need to do anything. If, however, there are circular references anywhere in the diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index 687c0c936..b4e85894a 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -17,7 +17,9 @@ BasicThread::BasicThread(const char* pName): m_priority(ThreadPriority::Default) {} -BasicThread::~BasicThread(void){} +BasicThread::~BasicThread(void) { + NotifyTeardownListeners(); +} std::mutex& BasicThread::GetLock(void) { return m_state->m_lock; @@ -64,13 +66,13 @@ void BasicThread::DoRunLoopCleanup(std::shared_ptr&& ctxt, std::sha // need to hold a reference to. auto state = m_state; - // Perform a manual notification of teardown listeners - NotifyTeardownListeners(); - // Transition to stopped state. Synchronization not required, transitions are all one-way m_stop = true; m_running = false; + // Perform a manual notification of teardown listeners + NotifyTeardownListeners(); + // Tell our CoreRunnable parent that we're done to ensure that our reference count will be cleared. Stop(false); From c2a98fb665bc7546a51df86a3b0b21b0051da040 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 27 Mar 2015 16:30:23 -0700 Subject: [PATCH 30/63] Add remove() to signal_node_base, Make signal_relay a signal_node_base, Modify -= to check and ensure the node is part of the list --- autowiring/auto_signal.h | 69 +++++++++++++++----------- src/autowiring/test/AutoSignalTest.cpp | 20 ++++++++ 2 files changed, 60 insertions(+), 29 deletions(-) diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index 34d3a81ee..04640a72d 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -32,6 +32,21 @@ namespace autowiring { // Forward and backward linkages: signal_node_base* pFlink; signal_node_base* pBlink; + + void remove(){ + // Clear linkage + if (this->pBlink) + this->pBlink->pFlink = this->pFlink; + if (this->pFlink) + this->pFlink->pBlink = this->pBlink; + + // If we're the head pointer, throw an exception + if (!this->pBlink) + throw std::runtime_error("Cannot remove the head of the list"); + + // Fully unlinked, delete + delete this; + } }; // Holds a reference to one of the signal holders @@ -46,6 +61,8 @@ namespace autowiring { {} const std::function fn; + + using signal_node_base::remove; }; @@ -117,40 +134,34 @@ namespace autowiring { /// /// Stores a signal /// - struct signal_relay + /// + /// This is functionally a sentinal head of the linked list. + /// + struct signal_relay : + internal::signal_node_base { - public: - signal_relay(void) : - pHead(nullptr) - {} + signal_relay(void) {} ~signal_relay(void) { // Standard linked list cleaup internal::signal_node_base* next = nullptr; - for (auto cur = pHead; cur; cur = next) { + for (auto cur = pFlink; cur; cur = next) { next = cur->pFlink; delete cur; } } - protected: - // First entry on the list: - internal::signal_node_base* pHead; + /// + /// Searches the list for this node and removes it, or throws if it is not found. + /// + void operator-=(signal_node_base* node) { + signal_node_base* cur; + for (cur = pFlink; cur != nullptr; cur = cur->pFlink) { + if (cur == node) + return node->remove(); + } - public: - void operator-=(internal::signal_node_base* rhs) { - // Clear linkage - if (rhs->pBlink) - rhs->pBlink->pFlink = rhs->pFlink; - if (rhs->pFlink) - rhs->pFlink->pBlink = rhs->pBlink; - - // If we're the head pointer then unlink - if (rhs == pHead) - pHead = rhs->pFlink; - - // Fully unlinked, delete - delete rhs; + throw std::runtime_error("Attempted to remove node which is not part of this list."); } }; @@ -162,19 +173,19 @@ namespace autowiring { signal_relay { internal::signal_node* GetHead(void) const { - return static_cast*>(pHead); + return static_cast*>(pFlink); } /// /// Attaches the specified handler to this signal /// internal::signal_node* operator+=(std::function&& fn) { - // Standard singly linked list insert: auto retVal = new internal::signal_node(std::move(fn)); - retVal->pFlink = pHead; - if (pHead) - pHead->pBlink = retVal; - pHead = retVal; + retVal->pBlink = this; + retVal->pFlink = pFlink; + if (pFlink) + pFlink->pBlink = retVal; + pFlink = retVal; return retVal; } }; diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index 7cba85b91..396c27142 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -122,3 +122,23 @@ TEST_F(AutoSignalTest, RaiseASignalWithinASlotTest) { ASSERT_TRUE(handler_called1) << "Handler 1 was not called on a stack-allocated signal"; ASSERT_TRUE(handler_called2) << "Handler 2 was not called on a stack-allocated signal"; } + +TEST_F(AutoSignalTest, NodeRemoval) { + autowiring::signal signal1; + autowiring::signal signal2; + + bool handler_called1 = false; + bool handler_called2 = false; + + auto* registration1 = signal1 += [&] { handler_called1 = true; }; + auto* registration2 = signal2 += [&] { handler_called2 = true; }; + + ASSERT_ANY_THROW(signal1 -= registration2) << "Removing a registration from a different signal than it was registered to failed to throw an exception"; + + registration1->remove(); + signal1(); + signal2(); + + ASSERT_FALSE(handler_called1) << "Handler1 was called after being unregistered"; + ASSERT_TRUE(handler_called2) << "Handler2 was removed after an invalid -= operation"; +} \ No newline at end of file From f5ee0721bd086dd3487d5193e5c361603d5fa214 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 27 Mar 2015 17:00:49 -0700 Subject: [PATCH 31/63] add the ability to append to a registration object, add tests for call ordering --- autowiring/auto_signal.h | 25 +++++++++++------ src/autowiring/test/AutoSignalTest.cpp | 38 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index 04640a72d..09d1aee5a 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -33,6 +33,9 @@ namespace autowiring { signal_node_base* pFlink; signal_node_base* pBlink; + /// + /// Removes this node from the list it's in, or throws if it's the head. + /// void remove(){ // Clear linkage if (this->pBlink) @@ -63,8 +66,20 @@ namespace autowiring { const std::function fn; using signal_node_base::remove; - }; + /// + /// Appends the specified handler to this list of nodes. + /// + internal::signal_node* operator+=(std::function&& fn) { + auto retVal = new internal::signal_node(std::move(fn)); + retVal->pBlink = this; + retVal->pFlink = pFlink; + if (pFlink) + pFlink->pBlink = retVal; + pFlink = retVal; + return retVal; + } + }; struct signal_registration_base { signal_registration_base(void); @@ -180,13 +195,7 @@ namespace autowiring { /// Attaches the specified handler to this signal /// internal::signal_node* operator+=(std::function&& fn) { - auto retVal = new internal::signal_node(std::move(fn)); - retVal->pBlink = this; - retVal->pFlink = pFlink; - if (pFlink) - pFlink->pBlink = retVal; - pFlink = retVal; - return retVal; + return *reinterpret_cast*>(this) += std::move(fn); } }; diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index 396c27142..a7256f40e 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -141,4 +141,42 @@ TEST_F(AutoSignalTest, NodeRemoval) { ASSERT_FALSE(handler_called1) << "Handler1 was called after being unregistered"; ASSERT_TRUE(handler_called2) << "Handler2 was removed after an invalid -= operation"; +} + +TEST_F(AutoSignalTest, CallOrdering) { + autowiring::signal signal1; + + bool handler_called1 = false; + bool handler_called2 = false; + + //handlers are inserted at the begining, so this will be called last. + auto* registration1 = signal1 += [&] { + ASSERT_TRUE(handler_called2); + handler_called1 = true; + }; + auto* registration2 = signal1 += [&] { handler_called2 = true; }; + + signal1(); + + ASSERT_TRUE(handler_called1) << "Handler1 was called after being unregistered"; + ASSERT_TRUE(handler_called2) << "Handler2 was removed after an invalid -= operation"; +} + +TEST_F(AutoSignalTest, CallInsertion) { + autowiring::signal signal1; + + bool handler_called1 = false; + bool handler_called2 = false; + + auto* registration1 = signal1 += [&] { handler_called1 = true; }; + + //when += to a registration object, the new one is appended. + auto* registration2 = *registration1 += [&] { + ASSERT_TRUE(handler_called1); + handler_called2 = true; }; + + signal1(); + + ASSERT_TRUE(handler_called1) << "Handler1 was called after being unregistered"; + ASSERT_TRUE(handler_called2) << "Handler2 was removed after an invalid -= operation"; } \ No newline at end of file From 9eb482e808f94008d0de1037cfd636c204f97122 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 27 Mar 2015 18:48:11 -0700 Subject: [PATCH 32/63] Added support for self-referencing handler functions --- autowiring/auto_signal.h | 55 ++++++++++++++++++++------ src/autowiring/test/AutoSignalTest.cpp | 49 +++++++++++++++++++++-- 2 files changed, 88 insertions(+), 16 deletions(-) diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index 09d1aee5a..ca8c1cccb 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -50,11 +50,19 @@ namespace autowiring { // Fully unlinked, delete delete this; } + + void insert(signal_node_base* node) { + node->pBlink = this; + node->pFlink = pFlink; + if (pFlink) + pFlink->pBlink = node; + pFlink = node; + } }; // Holds a reference to one of the signal holders template - struct signal_node: + struct signal_node : signal_node_base { signal_node(const signal_node& rhs) = delete; @@ -62,23 +70,42 @@ namespace autowiring { signal_node(std::function&& fn) : fn(std::move(fn)) {} + + signal_node(std::function&& newFn) : + fn([this, newFn](Args... args){ newFn(this, args...); }) + {} const std::function fn; using signal_node_base::remove; + //Same Args + template + signal_node* Register(t_Fn newFn, void(t_Fn::*pfn) (Args...) const){ + auto retVal = new signal_node(std::forward(newFn)); + insert(retVal); + return retVal; + } + + //Additional First arg is a signal_node_base* + template + signal_node* Register(t_Fn newFn, void(t_Fn::*pfn) (signal_node_base*, Args...) const){ + auto retVal = new signal_node(std::forward(newFn)); + insert(retVal); + return retVal; + } + /// /// Appends the specified handler to this list of nodes. /// - internal::signal_node* operator+=(std::function&& fn) { - auto retVal = new internal::signal_node(std::move(fn)); - retVal->pBlink = this; - retVal->pFlink = pFlink; - if (pFlink) - pFlink->pBlink = retVal; - pFlink = retVal; - return retVal; + template + signal_node* operator+=(t_Fn fn) { + return this->Register(std::forward(fn), &t_Fn::operator()); } + private: + signal_node() {} + + }; struct signal_registration_base { @@ -194,9 +221,11 @@ namespace autowiring { /// /// Attaches the specified handler to this signal /// - internal::signal_node* operator+=(std::function&& fn) { - return *reinterpret_cast*>(this) += std::move(fn); + template + internal::signal_node* operator+=(t_Fn fn) { + return *reinterpret_cast*>(this) += std::forward(fn); } + }; /// @@ -221,7 +250,9 @@ namespace autowiring { typedef internal::signal_node registration_t; typedef std::function function_t; - registration_t* operator+=(function_t&& fn) { return *m_relay += std::move(fn); } + template + registration_t* operator+=(t_Fn && fn) { return *m_relay += std::move(fn); } + void operator-=(registration_t* node) { return *m_relay -= node; } /// diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index a7256f40e..66a1d6e3e 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -150,11 +150,11 @@ TEST_F(AutoSignalTest, CallOrdering) { bool handler_called2 = false; //handlers are inserted at the begining, so this will be called last. - auto* registration1 = signal1 += [&] { + signal1 += [&] { ASSERT_TRUE(handler_called2); handler_called1 = true; }; - auto* registration2 = signal1 += [&] { handler_called2 = true; }; + signal1 += [&] { handler_called2 = true; }; signal1(); @@ -168,10 +168,10 @@ TEST_F(AutoSignalTest, CallInsertion) { bool handler_called1 = false; bool handler_called2 = false; - auto* registration1 = signal1 += [&] { handler_called1 = true; }; + auto* registration1 = signal1 += [&](void) { handler_called1 = true; }; //when += to a registration object, the new one is appended. - auto* registration2 = *registration1 += [&] { + *registration1 += [&](void) { ASSERT_TRUE(handler_called1); handler_called2 = true; }; @@ -179,4 +179,45 @@ TEST_F(AutoSignalTest, CallInsertion) { ASSERT_TRUE(handler_called1) << "Handler1 was called after being unregistered"; ASSERT_TRUE(handler_called2) << "Handler2 was removed after an invalid -= operation"; +} + +TEST_F(AutoSignalTest, SelfReferencingCall) { + typedef autowiring::signal signal_t; + signal_t signal1; + + bool handler_called1 = false; + int magic_number = 123; + + //The main test is just if this thing will compile + signal_t::registration_t* registration1 = + signal1 += [&](autowiring::internal::signal_node_base* reg, int magic) { + ASSERT_EQ(magic, magic_number); + ASSERT_EQ(registration1, reg); + handler_called1 = true; + }; + + signal1(magic_number); + + ASSERT_TRUE(handler_called1) << "Handler was not called!"; +} + +TEST_F(AutoSignalTest, SelfRemovingCall) { + typedef autowiring::signal signal_t; + signal_t signal1; + + int handler_called1 = 0; + int magic_number = 123; + + signal_t::registration_t* registration1 = + signal1 += [&](autowiring::internal::signal_node_base* reg, int magic) { + ASSERT_EQ(magic, magic_number); + ASSERT_EQ(registration1, reg); + ++handler_called1; + reg->remove(); + }; + + signal1(magic_number); + signal1(magic_number); + + ASSERT_EQ(handler_called1, 1) << "Handler was unable to remove itself!"; } \ No newline at end of file From 4c7f17858da6a9771a2919c62aa3148cfda12377 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Sat, 28 Mar 2015 19:14:45 -0700 Subject: [PATCH 33/63] Move 'Remove' functionality to the node destructor --- autowiring/auto_signal.h | 56 +++++++------------------- src/autowiring/test/AutoSignalTest.cpp | 4 +- 2 files changed, 17 insertions(+), 43 deletions(-) diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index ca8c1cccb..f2fc56739 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -27,30 +27,21 @@ namespace autowiring { pBlink(nullptr) {} - virtual ~signal_node_base(void) {} - - // Forward and backward linkages: - signal_node_base* pFlink; - signal_node_base* pBlink; - /// /// Removes this node from the list it's in, or throws if it's the head. /// - void remove(){ - // Clear linkage + virtual ~signal_node_base(void) { + // Clear linkage if (this->pBlink) this->pBlink->pFlink = this->pFlink; if (this->pFlink) this->pFlink->pBlink = this->pBlink; - - // If we're the head pointer, throw an exception - if (!this->pBlink) - throw std::runtime_error("Cannot remove the head of the list"); - - // Fully unlinked, delete - delete this; } + // Forward and backward linkages: + signal_node_base* pFlink; + signal_node_base* pBlink; + void insert(signal_node_base* node) { node->pBlink = this; node->pFlink = pFlink; @@ -71,41 +62,22 @@ namespace autowiring { fn(std::move(fn)) {} + //Functions where the first argument is a signal_node_base are also ok. signal_node(std::function&& newFn) : fn([this, newFn](Args... args){ newFn(this, args...); }) {} const std::function fn; - using signal_node_base::remove; - - //Same Args - template - signal_node* Register(t_Fn newFn, void(t_Fn::*pfn) (Args...) const){ - auto retVal = new signal_node(std::forward(newFn)); - insert(retVal); - return retVal; - } - - //Additional First arg is a signal_node_base* - template - signal_node* Register(t_Fn newFn, void(t_Fn::*pfn) (signal_node_base*, Args...) const){ - auto retVal = new signal_node(std::forward(newFn)); - insert(retVal); - return retVal; - } - /// /// Appends the specified handler to this list of nodes. /// template signal_node* operator+=(t_Fn fn) { - return this->Register(std::forward(fn), &t_Fn::operator()); + auto retVal = new signal_node(std::forward(fn)); + insert(retVal); + return retVal; } - private: - signal_node() {} - - }; struct signal_registration_base { @@ -184,7 +156,7 @@ namespace autowiring { { signal_relay(void) {} - ~signal_relay(void) { + ~signal_relay(void) override { // Standard linked list cleaup internal::signal_node_base* next = nullptr; for (auto cur = pFlink; cur; cur = next) { @@ -199,8 +171,10 @@ namespace autowiring { void operator-=(signal_node_base* node) { signal_node_base* cur; for (cur = pFlink; cur != nullptr; cur = cur->pFlink) { - if (cur == node) - return node->remove(); + if (cur == node){ + delete node; + return; + } } throw std::runtime_error("Attempted to remove node which is not part of this list."); diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index 66a1d6e3e..e3eb48b7b 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -135,7 +135,7 @@ TEST_F(AutoSignalTest, NodeRemoval) { ASSERT_ANY_THROW(signal1 -= registration2) << "Removing a registration from a different signal than it was registered to failed to throw an exception"; - registration1->remove(); + delete registration1; signal1(); signal2(); @@ -213,7 +213,7 @@ TEST_F(AutoSignalTest, SelfRemovingCall) { ASSERT_EQ(magic, magic_number); ASSERT_EQ(registration1, reg); ++handler_called1; - reg->remove(); + delete reg; }; signal1(magic_number); From e390447b5660637ae1db074862c1f6cf2289b6a9 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 30 Mar 2015 00:22:04 -0700 Subject: [PATCH 34/63] Make += able to accept signaL_nodes instead of just signal_node_bases, test in-lambda appending. --- autowiring/auto_signal.h | 8 ++++---- src/autowiring/test/AutoSignalTest.cpp | 21 ++++++++++++++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index f2fc56739..a7ad03212 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -62,11 +62,11 @@ namespace autowiring { fn(std::move(fn)) {} - //Functions where the first argument is a signal_node_base are also ok. - signal_node(std::function&& newFn) : - fn([this, newFn](Args... args){ newFn(this, args...); }) + //Functions where the first argument is a signal_node<...> or base type are also ok. + signal_node(std::function*, Args...)>&& newFn) : + fn([this, newFn](Args... args){ newFn(this, args...); }) {} - + const std::function fn; /// diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index e3eb48b7b..cd9fcf9ec 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -201,11 +201,14 @@ TEST_F(AutoSignalTest, SelfReferencingCall) { ASSERT_TRUE(handler_called1) << "Handler was not called!"; } -TEST_F(AutoSignalTest, SelfRemovingCall) { +TEST_F(AutoSignalTest, SelfModifyingCall) { typedef autowiring::signal signal_t; signal_t signal1; int handler_called1 = 0; + int handler_called2 = 0; + int handler_called3 = 0; + int magic_number = 123; signal_t::registration_t* registration1 = @@ -213,6 +216,20 @@ TEST_F(AutoSignalTest, SelfRemovingCall) { ASSERT_EQ(magic, magic_number); ASSERT_EQ(registration1, reg); ++handler_called1; + + delete reg; + }; + + signal_t::registration_t* registration2 = signal1 += [&](signal_t::registration_t* reg, int magic) { + ASSERT_EQ(magic, magic_number); + ASSERT_EQ(registration2, reg); + ++handler_called2; + + //+= is an append operation, so this function will be evaluated immediately after the current one. + *reg += [&](int magic) { + ++handler_called3; + }; + delete reg; }; @@ -220,4 +237,6 @@ TEST_F(AutoSignalTest, SelfRemovingCall) { signal1(magic_number); ASSERT_EQ(handler_called1, 1) << "Handler was unable to remove itself!"; + ASSERT_EQ(handler_called2, 1) << "Specific handler was unable to remove itself"; + ASSERT_EQ(handler_called3, 2) << "Handler was unable to append to itself."; } \ No newline at end of file From 816c014438708a9b839e28172f76158696dae99e Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 30 Mar 2015 11:21:12 -0700 Subject: [PATCH 35/63] Preparatory refactor of Snoop to AddSnooper Fixes #469. The Snoop method is misleading. We cannot rename this method because it is too extensively used, but we can start a deprecation policy and get it fixed eventually. --- autowiring/CoreContext.h | 70 ++++++++++++++++++++++++------- src/autowiring/CoreContext.cpp | 4 +- src/autowiring/test/SnoopTest.cpp | 34 +++++++-------- 3 files changed, 73 insertions(+), 35 deletions(-) diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index a828b81f1..3ef0505f5 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -981,13 +981,20 @@ class CoreContext: /// The recipient of the event void FilterFiringException(const JunctionBoxBase* pProxy, CoreObject* pRecipient); + /// Identical to RemoveSnooper + void DEPRECATED(Snoop(const CoreObjectDescriptor& traits), "Use AddSnooper instead") { return AddSnooper(traits); } + template + void DEPRECATED(Snoop(const std::shared_ptr& pSnooper), "Use AddSnooper instead"); + template + void DEPRECATED(Snoop(const Autowired& snooper), "Use AddSnooper instead"); + /// - /// Runtime version of Snoop + /// Runtime version of AddSnooper /// - void Snoop(const CoreObjectDescriptor& traits); + void AddSnooper(const CoreObjectDescriptor& traits); /// - /// Registers the specified event receiver to receive messages broadcast within this context. + /// Registers the specified event receiver to receive messages from this context /// /// /// This enables the passed event receiver to snoop events that are broadcast from a @@ -998,16 +1005,16 @@ class CoreContext: /// broadcast in THIS context will be forwarded to the snooper. /// template - void Snoop(const std::shared_ptr& pSnooper) { - Snoop(CoreObjectDescriptor(pSnooper, (T*)nullptr)); + void AddSnooper(const std::shared_ptr& pSnooper) { + AddSnooper(CoreObjectDescriptor(pSnooper, (T*)nullptr)); } /// /// Resolution overload /// template - void Snoop(const Autowired& snooper) { - Snoop( + void AddSnooper(const Autowired& snooper) { + AddSnooper( CoreObjectDescriptor( static_cast&>(snooper), (T*)nullptr @@ -1015,28 +1022,35 @@ class CoreContext: ); } + /// Identical to RemoveSnooper + void DEPRECATED(Unsnoop(const CoreObjectDescriptor& traits), "Use RemoveSnooper instead") { return RemoveSnooper(traits); } + template + void DEPRECATED(Unsnoop(const std::shared_ptr& pSnooper), "Use RemoveSnooper instead"); + template + void DEPRECATED(Unsnoop(const Autowired& snooper), "Use RemoveSnooper instead"); + /// - /// Runtime version of Unsnoop + /// Runtime version of RemoveSnooper /// - void Unsnoop(const CoreObjectDescriptor& traits); + void RemoveSnooper(const CoreObjectDescriptor& traits); /// - /// Unregisters an event receiver previously registered to receive snooped events + /// Unregisters a snooper previously registered to receive snooped events /// /// - /// It is an error to call this method without a prior call to Snoop + /// It is an error to call this method without a prior call to AddSnooper /// template - void Unsnoop(const std::shared_ptr& pSnooper) { - Unsnoop(CoreObjectDescriptor(pSnooper, (T*)nullptr)); + void RemoveSnooper(const std::shared_ptr& pSnooper) { + RemoveSnooper(CoreObjectDescriptor(pSnooper, (T*)nullptr)); } /// - /// Resolution overload + /// Resolution overload of RemoveSnooper /// template - void Unsnoop(const Autowired& snooper) { - Unsnoop( + void RemoveSnooper(const Autowired& snooper) { + RemoveSnooper( CoreObjectDescriptor( static_cast&>(snooper), (T*)nullptr @@ -1272,6 +1286,30 @@ void CoreContext::AutoRequireMicroBolt(void) { Inject>(); } +template +void CoreContext::Snoop(const std::shared_ptr& pSnooper) +{ + return AddSnooper(pSnooper); +} + +template +void CoreContext::Snoop(const Autowired& snooper) +{ + return AddSnooper(snooper); +} + +template +void CoreContext::Unsnoop(const std::shared_ptr& pSnooper) +{ + return RemoveSnooper(pSnooper); +} + +template +void CoreContext::Unsnoop(const Autowired& snooper) +{ + return RemoveSnooper(snooper); +} + template class CoreContext::AutoFactory { diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index 6db150a79..72909cb32 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -1019,7 +1019,7 @@ void CoreContext::FilterFiringException(const JunctionBoxBase* pProxy, CoreObjec } } -void CoreContext::Snoop(const CoreObjectDescriptor& traits) { +void CoreContext::AddSnooper(const CoreObjectDescriptor& traits) { // Add to collections of snoopers InsertSnooper(traits.value); @@ -1032,7 +1032,7 @@ void CoreContext::Snoop(const CoreObjectDescriptor& traits) { AddPacketSubscriber(traits.subscriber); } -void CoreContext::Unsnoop(const CoreObjectDescriptor& traits) { +void CoreContext::RemoveSnooper(const CoreObjectDescriptor& traits) { RemoveSnooper(traits.value); // Cleanup if its an EventReceiver diff --git a/src/autowiring/test/SnoopTest.cpp b/src/autowiring/test/SnoopTest.cpp index 473019e97..1c995d685 100644 --- a/src/autowiring/test/SnoopTest.cpp +++ b/src/autowiring/test/SnoopTest.cpp @@ -59,7 +59,7 @@ class RemovesSelf: virtual void ZeroArgs(void){ counter++; AutoCurrentContext ctxt; - ctxt->Unsnoop(GetSelf()); + ctxt->RemoveSnooper(GetSelf()); } public: RemovesSelf(): @@ -83,7 +83,7 @@ TEST_F(SnoopTest, VerifySimpleSnoop) { AutoRequired childMember; // Snoop - child->Snoop(parentMember); + child->AddSnooper(parentMember); // Now fire an event from the child: AutoFired firer; @@ -111,8 +111,8 @@ TEST_F(SnoopTest, VerifyUnsnoop) { AutoRequired childMember; // Snoop, unsnoop: - snoopy->Snoop(parentMember); - snoopy->Unsnoop(parentMember); + snoopy->AddSnooper(parentMember); + snoopy->RemoveSnooper(parentMember); snoopy->Initiate(); // Fire one event: @@ -136,7 +136,7 @@ TEST_F(SnoopTest, AmbiguousReciept) { { AutoCreateContext subCtxt; - subCtxt->Snoop(parent); + subCtxt->AddSnooper(parent); subCtxt->Initiate(); // Verify that simple firing _here_ causes transmission as expected: @@ -178,7 +178,7 @@ TEST_F(SnoopTest, AvoidDoubleReciept) { childMember = child->Inject(); // Snoop - child->Snoop(parentMember); + child->AddSnooper(parentMember); } // Now fire an event from the parent: @@ -193,7 +193,7 @@ TEST_F(SnoopTest, AvoidDoubleReciept) { // Test sibling context AutoRequired sibMember(sibCtxt); AutoRequired alsoInParent; - child->Snoop(sibMember); + child->AddSnooper(sibMember); firer(&UpBroadcastListener::SimpleCall)(); @@ -203,7 +203,7 @@ TEST_F(SnoopTest, AvoidDoubleReciept) { ASSERT_EQ(1, sibMember->m_callCount) << "Sibling context member didn't receive message"; // Make sure unsnoop cleans up everything - child->Unsnoop(sibMember); + child->RemoveSnooper(sibMember); firer(&UpBroadcastListener::SimpleCall)(); ASSERT_EQ(3, childMember->m_callCount) << "Message not received by another member of the same context"; @@ -236,8 +236,8 @@ TEST_F(SnoopTest, MultiSnoop) { member->m_callCount = 0; // Snoop both. Invocation in an uninitialized context should not cause any handlers to be raised. - ctxt1->Snoop(member); - ctxt2->Snoop(member); + ctxt1->AddSnooper(member); + ctxt2->AddSnooper(member); base->Invoke(&UpBroadcastListener::SimpleCall)(); ctxt1->Invoke(&UpBroadcastListener::SimpleCall)(); ctxt2->Invoke(&UpBroadcastListener::SimpleCall)(); @@ -248,7 +248,7 @@ TEST_F(SnoopTest, MultiSnoop) { member2->m_callCount = 0; // Unsnoop one - ctxt2->Unsnoop(member); + ctxt2->RemoveSnooper(member); base->Invoke(&UpBroadcastListener::SimpleCall)(); ctxt1->Invoke(&UpBroadcastListener::SimpleCall)(); ctxt2->Invoke(&UpBroadcastListener::SimpleCall)(); @@ -265,7 +265,7 @@ TEST_F(SnoopTest, AntiCyclicRemoval) { CurrentContextPusher pshr(snoopy); snoopy->Initiate(); - snoopy->Snoop(removeself); + snoopy->AddSnooper(removeself); ASSERT_EQ(0, removeself->counter); @@ -289,7 +289,7 @@ TEST_F(SnoopTest, SimplePackets) { // Add filter to tracking AutoRequired filter(Tracking); AutoRequired detachedFilter(Tracking); - Pipeline->Snoop(filter); + Pipeline->AddSnooper(filter); ASSERT_FALSE(!!filter->m_called) << "Filter called prematurely"; ASSERT_FALSE(detachedFilter->m_called) << "Filter called prematurely"; @@ -308,7 +308,7 @@ TEST_F(SnoopTest, SimplePackets) { //reset filter->m_called = false; - Pipeline->Unsnoop(filter); + Pipeline->RemoveSnooper(filter); auto packet2 = factory->NewPacket(); packet2->Decorate(Decoration<0>()); packet2->Decorate(Decoration<1>()); @@ -321,7 +321,7 @@ TEST_F(SnoopTest, CanSnoopAutowired) { // Now autowire what we injected and verify we can snoop this directly Autowired so; - ctxt->Snoop(so); + ctxt->AddSnooper(so); } TEST_F(SnoopTest, RuntimeSnoopCall) { @@ -332,7 +332,7 @@ TEST_F(SnoopTest, RuntimeSnoopCall) { CoreObjectDescriptor traits(x); // Try to snoop and verify that the snooped member gets an event: - ctxt->Snoop(x); + ctxt->AddSnooper(x); AutoFired ubl; ubl(&UpBroadcastListener::SimpleCall)(); @@ -341,7 +341,7 @@ TEST_F(SnoopTest, RuntimeSnoopCall) { ASSERT_EQ(1UL, x->m_callCount) << "Call count to a child member was incorrect"; // And the alternative variant next: - ctxt->Unsnoop(x); + ctxt->RemoveSnooper(x); ubl(&UpBroadcastListener::SimpleCall)(); ASSERT_EQ(1UL, x->m_callCount) << "Snoop method was invoked after the related type was removed"; } \ No newline at end of file From 9444c1e97cf46613b5e3890bc0a5d7d454c5596f Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 30 Mar 2015 12:59:05 -0700 Subject: [PATCH 36/63] AddSubscriber must return the added AutoFilterDescriptor This is intended to make it simpler to unregister filters added to an AutoPacketFactory, especially when the registration occurs via a lambda function. Also add more tests to confirm that this behavior functions as expected. --- autowiring/AutoPacketFactory.h | 35 ++++++++++++++----- src/autowiring/AutoPacketFactory.cpp | 7 +++- src/autowiring/test/AutoPacketFactoryTest.cpp | 18 ++++++++-- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/autowiring/AutoPacketFactory.h b/autowiring/AutoPacketFactory.h index e40b30042..b9178ff18 100644 --- a/autowiring/AutoPacketFactory.h +++ b/autowiring/AutoPacketFactory.h @@ -89,32 +89,51 @@ class AutoPacketFactory: /// /// Registers the passed subscriber, if it defines a method called AutoFilter /// + /// The descriptor for the AutoFilter to be added + /// rhs /// /// This method is idempotent /// - void AddSubscriber(const AutoFilterDescriptor& rhs); + const AutoFilterDescriptor& AddSubscriber(const AutoFilterDescriptor& rhs); /// /// Convenience override of AddSubscriber /// + /// A shared pointer to a type which has an AutoFilter routine on it + /// + /// For this call to be valid, T::AutoFilter must be defined and must be a compliant AutoFilter + /// template - void AddSubscriber(const std::shared_ptr& rhs) { - AddSubscriber(AutoFilterDescriptor(rhs)); + AutoFilterDescriptor AddSubscriber(const std::shared_ptr& rhs) { + return AddSubscriber(AutoFilterDescriptor(rhs)); } + /// + /// Removes the designated AutoFilter from this factory + /// + /// The AutoFilter to be removed + /// + /// This method will not retroactively modify packets that have already been issued with the specified + /// AutoFilter on it. Only packets that are issued after this method returns will lack the presence of + /// the autoFilter described by the parameter. + /// + void RemoveSubscriber(const AutoFilterDescriptor& autoFilter); + /// /// Convienance overload of operator+= to add a subscriber from a lambda /// + /// + /// This method provides a way to attach a lambda function directly to the factory + /// template - void operator+=(Fx&& fx) { - AddSubscriber(AutoFilterDescriptor(std::forward(fx))); + AutoFilterDescriptor operator+= (Fx&& fx) { + return AddSubscriber(AutoFilterDescriptor(std::forward(fx))); } /// - /// Removes the designated AutoFilter from this factory + /// Overloaded counterpart to RemoveSubscriber /// - /// The AutoFilter to be removed - void RemoveSubscriber(const AutoFilterDescriptor& autoFilter); + void operator-=(const AutoFilterDescriptor& desc); protected: /// diff --git a/src/autowiring/AutoPacketFactory.cpp b/src/autowiring/AutoPacketFactory.cpp index 54994596e..a4bb4f064 100644 --- a/src/autowiring/AutoPacketFactory.cpp +++ b/src/autowiring/AutoPacketFactory.cpp @@ -144,9 +144,10 @@ void AutoPacketFactory::Clear(void) { Stop(false); } -void AutoPacketFactory::AddSubscriber(const AutoFilterDescriptor& rhs) { +const AutoFilterDescriptor& AutoPacketFactory::AddSubscriber(const AutoFilterDescriptor& rhs) { std::lock_guard lk(m_lock); m_autoFilters.insert(rhs); + return rhs; } void AutoPacketFactory::RemoveSubscriber(const AutoFilterDescriptor& autoFilter) { @@ -155,6 +156,10 @@ void AutoPacketFactory::RemoveSubscriber(const AutoFilterDescriptor& autoFilter) m_autoFilters.erase(autoFilter); } +void AutoPacketFactory::operator-=(const AutoFilterDescriptor& desc) { + RemoveSubscriber(desc); +} + AutoFilterDescriptor AutoPacketFactory::GetTypeDescriptorUnsafe(const std::type_info* nodeType) { //ASSUME: type_info uniquely specifies descriptor for (auto& af : m_autoFilters) diff --git a/src/autowiring/test/AutoPacketFactoryTest.cpp b/src/autowiring/test/AutoPacketFactoryTest.cpp index aa63468cb..4b3e50ca3 100644 --- a/src/autowiring/test/AutoPacketFactoryTest.cpp +++ b/src/autowiring/test/AutoPacketFactoryTest.cpp @@ -238,9 +238,8 @@ TEST_F(AutoPacketFactoryTest, MultiDecorateTest) { } TEST_F(AutoPacketFactoryTest, MultiPostHocIntroductionTest) { - AutoCurrentContext ctxt; - ctxt->Initiate(); - AutoRequired factory(ctxt); + AutoCurrentContext()->Initiate(); + AutoRequired factory; int called = 0; @@ -255,4 +254,17 @@ TEST_F(AutoPacketFactoryTest, MultiPostHocIntroductionTest) { }; ASSERT_EQ(3, called) << "Not all lambda functions were called as expected"; +} + +TEST_F(AutoPacketFactoryTest, CanRemoveAddedLambda) { + AutoCurrentContext()->Initiate(); + AutoRequired factory; + + auto desc = *factory += [](int&){}; + auto packet1 = factory->NewPacket(); + *factory -= desc; + auto packet2 = factory->NewPacket(); + + ASSERT_TRUE(packet1->Has()) << "First packet did not posess expected decoration"; + ASSERT_FALSE(packet2->Has()) << "Decoration present even after all filters were removed from a factory"; } \ No newline at end of file From 74218d44032a58c885fbb8bfd0e9cea9c9b4d410 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 30 Mar 2015 13:18:14 -0700 Subject: [PATCH 37/63] Remove unlink step from destructor, restore the remove function(). change insert to insert_after, add more comments. --- autowiring/auto_signal.h | 42 ++++++++++++++++---------- src/autowiring/test/AutoSignalTest.cpp | 13 ++++---- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index a7ad03212..22f4ad786 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -27,28 +27,37 @@ namespace autowiring { pBlink(nullptr) {} - /// - /// Removes this node from the list it's in, or throws if it's the head. - /// - virtual ~signal_node_base(void) { - // Clear linkage - if (this->pBlink) - this->pBlink->pFlink = this->pFlink; - if (this->pFlink) - this->pFlink->pBlink = this->pBlink; - } + virtual ~signal_node_base(void) {} // Forward and backward linkages: signal_node_base* pFlink; signal_node_base* pBlink; - void insert(signal_node_base* node) { + /// + /// Inserts a node after the current one + /// + void insert_after(signal_node_base* node) { node->pBlink = this; node->pFlink = pFlink; if (pFlink) pFlink->pBlink = node; pFlink = node; } + + /// + /// Removes this node from the list it's in. + /// + /// + /// If you call this function, you are assuming responsibility for the memory and + /// are expected to call delete on the node. + /// + void remove() { + // Clear linkage + if (this->pBlink) + this->pBlink->pFlink = this->pFlink; + if (this->pFlink) + this->pFlink->pBlink = this->pBlink; + } }; // Holds a reference to one of the signal holders @@ -64,7 +73,7 @@ namespace autowiring { //Functions where the first argument is a signal_node<...> or base type are also ok. signal_node(std::function*, Args...)>&& newFn) : - fn([this, newFn](Args... args){ newFn(this, args...); }) + fn([this, newFn](Args... args){ newFn(this, args...); }) {} const std::function fn; @@ -75,7 +84,7 @@ namespace autowiring { template signal_node* operator+=(t_Fn fn) { auto retVal = new signal_node(std::forward(fn)); - insert(retVal); + insert_after(retVal); return retVal; } }; @@ -161,17 +170,18 @@ namespace autowiring { internal::signal_node_base* next = nullptr; for (auto cur = pFlink; cur; cur = next) { next = cur->pFlink; - delete cur; + delete cur; //don't bother unlinking.. } } /// - /// Searches the list for this node and removes it, or throws if it is not found. + /// Searches the list for this node and deletes it, or throws if it is not found. /// void operator-=(signal_node_base* node) { signal_node_base* cur; for (cur = pFlink; cur != nullptr; cur = cur->pFlink) { if (cur == node){ + node->remove(); delete node; return; } @@ -225,7 +235,7 @@ namespace autowiring { typedef std::function function_t; template - registration_t* operator+=(t_Fn && fn) { return *m_relay += std::move(fn); } + registration_t* operator+=(t_Fn fn) { return *m_relay += std::forward(fn); } void operator-=(registration_t* node) { return *m_relay -= node; } diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index cd9fcf9ec..9125fa36a 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -190,11 +190,11 @@ TEST_F(AutoSignalTest, SelfReferencingCall) { //The main test is just if this thing will compile signal_t::registration_t* registration1 = - signal1 += [&](autowiring::internal::signal_node_base* reg, int magic) { - ASSERT_EQ(magic, magic_number); - ASSERT_EQ(registration1, reg); - handler_called1 = true; - }; + signal1 += [&](autowiring::internal::signal_node_base* reg, int magic) { + ASSERT_EQ(magic, magic_number); + ASSERT_EQ(registration1, reg); + handler_called1 = true; + }; signal1(magic_number); @@ -216,7 +216,7 @@ TEST_F(AutoSignalTest, SelfModifyingCall) { ASSERT_EQ(magic, magic_number); ASSERT_EQ(registration1, reg); ++handler_called1; - + reg->remove(); delete reg; }; @@ -230,6 +230,7 @@ TEST_F(AutoSignalTest, SelfModifyingCall) { ++handler_called3; }; + reg->remove(); delete reg; }; From 3b1d0e563197542b26c252e60e0c1a989dbefa55 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 30 Mar 2015 16:27:07 -0700 Subject: [PATCH 38/63] Fix potential issues in self-removal & insertion ordering, errors on MSVC --- autowiring/Decompose.h | 3 +++ autowiring/auto_signal.h | 35 ++++++++++++++++++++------ src/autowiring/test/AutoSignalTest.cpp | 20 ++++++++------- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/autowiring/Decompose.h b/autowiring/Decompose.h index 242a40dd7..1edec3c7e 100644 --- a/autowiring/Decompose.h +++ b/autowiring/Decompose.h @@ -1,12 +1,15 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once #include "is_any.h" +#include #include template struct TemplatePack { static const int N = sizeof...(Ts); + typedef std::tuple t_args; + /// /// An array of type T, parameterized by the bound function's arguments /// diff --git a/autowiring/auto_signal.h b/autowiring/auto_signal.h index 22f4ad786..2aa329bcc 100644 --- a/autowiring/auto_signal.h +++ b/autowiring/auto_signal.h @@ -1,5 +1,7 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once +#include "Decompose.h" + #include #include #include @@ -51,12 +53,18 @@ namespace autowiring { /// If you call this function, you are assuming responsibility for the memory and /// are expected to call delete on the node. /// - void remove() { + /// + /// A pointer to itself for easier chaining of operations. + /// + signal_node_base* remove() { // Clear linkage if (this->pBlink) this->pBlink->pFlink = this->pFlink; if (this->pFlink) this->pFlink->pBlink = this->pBlink; + + this->pBlink = this->pFlink = nullptr; + return this; } }; @@ -82,8 +90,17 @@ namespace autowiring { /// Appends the specified handler to this list of nodes. /// template - signal_node* operator+=(t_Fn fn) { - auto retVal = new signal_node(std::forward(fn)); + typename std::enable_if::N == sizeof...(Args), signal_node*>::type + operator+=(t_Fn fn) { + auto retVal = new signal_node(std::function(std::forward(fn))); + insert_after(retVal); + return retVal; + } + + template + typename std::enable_if::N == sizeof...(Args) + 1, signal_node*>::type + operator+=(t_Fn fn) { + auto retVal = new signal_node(std::function*,Args...)>(std::forward(fn))); insert_after(retVal); return retVal; } @@ -243,12 +260,14 @@ namespace autowiring { /// Raises the signal and invokes all attached handlers /// void operator()(Args... args) const { - for ( - auto cur = m_relay->GetHead(); - cur; - cur = static_cast(cur->pFlink) - ) + auto cur = m_relay->GetHead(); + while(cur) { + //Grab the next pointer before we evaluate incase the current node is deleted by it's + //function. + auto next = static_cast(cur->pFlink); cur->fn(args...); + cur = next; + } } }; } diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index 9125fa36a..fbe946125 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -135,6 +135,7 @@ TEST_F(AutoSignalTest, NodeRemoval) { ASSERT_ANY_THROW(signal1 -= registration2) << "Removing a registration from a different signal than it was registered to failed to throw an exception"; + registration1->remove(); delete registration1; signal1(); signal2(); @@ -216,22 +217,23 @@ TEST_F(AutoSignalTest, SelfModifyingCall) { ASSERT_EQ(magic, magic_number); ASSERT_EQ(registration1, reg); ++handler_called1; - reg->remove(); - delete reg; + delete reg->remove(); }; + auto lambda3 = [&](int magic) { + ++handler_called3; + }; + signal_t::registration_t* registration2 = signal1 += [&](signal_t::registration_t* reg, int magic) { ASSERT_EQ(magic, magic_number); ASSERT_EQ(registration2, reg); ++handler_called2; - //+= is an append operation, so this function will be evaluated immediately after the current one. - *reg += [&](int magic) { - ++handler_called3; - }; + //+= is an append operation, but because when we're traveling the list and we grab the next pointer + //*before* the function get's called, this append won't be picked up until the 2nd pass. + *reg += std::move(lambda3); - reg->remove(); - delete reg; + delete reg->remove(); }; signal1(magic_number); @@ -239,5 +241,5 @@ TEST_F(AutoSignalTest, SelfModifyingCall) { ASSERT_EQ(handler_called1, 1) << "Handler was unable to remove itself!"; ASSERT_EQ(handler_called2, 1) << "Specific handler was unable to remove itself"; - ASSERT_EQ(handler_called3, 2) << "Handler was unable to append to itself."; + ASSERT_EQ(handler_called3, 1) << "Handler was unable to append to itself or was called prematurely."; } \ No newline at end of file From dceec57267d2f168d9e43b7b04b6cd75931e1efc Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Mon, 30 Mar 2015 17:12:01 -0700 Subject: [PATCH 39/63] Fix missing header --- autowiring/is_any.h | 1 + 1 file changed, 1 insertion(+) diff --git a/autowiring/is_any.h b/autowiring/is_any.h index 8bdcb5a09..fda7bcc44 100644 --- a/autowiring/is_any.h +++ b/autowiring/is_any.h @@ -1,5 +1,6 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once +#include "C++11/cpp11.h" #include TYPE_TRAITS_HEADER /// From aa5f87f85c02dd194ce6851b5d8ad335125ed381 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 30 Mar 2015 23:53:21 -0700 Subject: [PATCH 40/63] Pull WaitForEvent to DispatchQueue These methods exclusively use members defined in DispatchQueue, and thus belong there. --- autowiring/CoreThread.h | 33 --------------- autowiring/DispatchQueue.h | 33 +++++++++++++++ src/autowiring/CoreThread.cpp | 71 ------------------------------- src/autowiring/DispatchQueue.cpp | 72 ++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 104 deletions(-) diff --git a/autowiring/CoreThread.h b/autowiring/CoreThread.h index 03ebfa2a6..a64323880 100644 --- a/autowiring/CoreThread.h +++ b/autowiring/CoreThread.h @@ -45,39 +45,6 @@ class CoreThread: virtual void DoRunLoopCleanup(std::shared_ptr&& ctxt, std::shared_ptr&& refTracker) override; public: - /// \internal - /// - /// Waits until a lambda function is ready to run in this thread's dispatch queue, - /// dispatches the function, and then returns. - /// - void WaitForEvent(void); - - /// \internal - /// - /// Waits until a lambda function in the dispatch queue is ready to run or the specified - /// time period elapses, whichever comes first. - /// - /// - /// False if the timeout period elapsed before an event could be dispatched, true otherwise - /// - bool WaitForEvent(std::chrono::milliseconds milliseconds); - - /// \internal - /// - /// Waits until a lambda function in the dispatch queue is ready to run or the specified - /// time is reached, whichever comes first. - /// - /// - /// False if the timeout period elapsed before an event could be dispatched, true otherwise - /// - bool WaitForEvent(std::chrono::steady_clock::time_point wakeTime); - - /// \internal - /// - /// An unsafe variant of WaitForEvent - /// - bool WaitForEventUnsafe(std::unique_lock& lk, std::chrono::steady_clock::time_point wakeTime); - /// \internal /// /// Called automatically to begin core thread execution. diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index f9cef3d71..24a925f53 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -147,6 +147,39 @@ class DispatchQueue { /// The total number of events dispatched int DispatchAllEvents(void); + /// \internal + /// + /// Waits until a lambda function is ready to run in this thread's dispatch queue, + /// dispatches the function, and then returns. + /// + void WaitForEvent(void); + + /// \internal + /// + /// Waits until a lambda function in the dispatch queue is ready to run or the specified + /// time period elapses, whichever comes first. + /// + /// + /// False if the timeout period elapsed before an event could be dispatched, true otherwise + /// + bool WaitForEvent(std::chrono::milliseconds milliseconds); + + /// \internal + /// + /// Waits until a lambda function in the dispatch queue is ready to run or the specified + /// time is reached, whichever comes first. + /// + /// + /// False if the timeout period elapsed before an event could be dispatched, true otherwise + /// + bool WaitForEvent(std::chrono::steady_clock::time_point wakeTime); + + /// \internal + /// + /// An unsafe variant of WaitForEvent + /// + bool WaitForEventUnsafe(std::unique_lock& lk, std::chrono::steady_clock::time_point wakeTime); + public: /// /// Explicit overload for already-constructed dispatch thunk types diff --git a/src/autowiring/CoreThread.cpp b/src/autowiring/CoreThread.cpp index 2c857e1b2..36a4f1f92 100644 --- a/src/autowiring/CoreThread.cpp +++ b/src/autowiring/CoreThread.cpp @@ -26,77 +26,6 @@ void CoreThread::DoRunLoopCleanup(std::shared_ptr&& ctxt, std::shar BasicThread::DoRunLoopCleanup(std::move(ctxt), std::move(refTracker)); } -void CoreThread::WaitForEvent(void) { - std::unique_lock lk(m_dispatchLock); - if(m_aborted) - throw dispatch_aborted_exception(); - - // Unconditional delay: - m_queueUpdated.wait(lk, [this] () -> bool { - if(m_aborted) - throw dispatch_aborted_exception(); - - return - // We will need to transition out if the delay queue receives any items: - !this->m_delayedQueue.empty() || - - // We also transition out if the dispatch queue has any events: - !this->m_dispatchQueue.empty(); - }); - - if(m_dispatchQueue.empty()) { - // The delay queue has items but the dispatch queue does not, we need to switch - // to the suggested sleep timeout variant: - WaitForEventUnsafe(lk, m_delayedQueue.top().GetReadyTime()); - } else { - // We have an event, we can just hop over to this variant: - DispatchEventUnsafe(lk); - } -} - -bool CoreThread::WaitForEvent(std::chrono::milliseconds milliseconds) { - return WaitForEvent(std::chrono::steady_clock::now() + milliseconds); -} - -bool CoreThread::WaitForEvent(std::chrono::steady_clock::time_point wakeTime) { - if(wakeTime == std::chrono::steady_clock::time_point::max()) { - // Maximal wait--we can optimize by using the zero-arguments version - return WaitForEvent(), true; - } - - std::unique_lock lk(m_dispatchLock); - return WaitForEventUnsafe(lk, wakeTime); -} - -bool CoreThread::WaitForEventUnsafe(std::unique_lock& lk, std::chrono::steady_clock::time_point wakeTime) { - if(m_aborted) - throw dispatch_aborted_exception(); - - while(m_dispatchQueue.empty()) { - // Derive a wakeup time using the high precision timer: - wakeTime = SuggestSoonestWakeupTimeUnsafe(wakeTime); - - // Now we wait, either for the timeout to elapse or for the dispatch queue itself to - // transition to the "aborted" state. - std::cv_status status = m_queueUpdated.wait_until(lk, wakeTime); - - // Short-circuit if the queue was aborted - if(m_aborted) - throw dispatch_aborted_exception(); - - if (PromoteReadyDispatchersUnsafe()) - // Dispatcher is ready to run! Exit our loop and dispatch an event - break; - - if(status == std::cv_status::timeout) - // Can't proceed, queue is empty and nobody is ready to be run - return false; - } - - DispatchEventUnsafe(lk); - return true; -} - void CoreThread::Run() { while(!ShouldStop()) WaitForEvent(); diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index a9c8e6f9d..da73d188d 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -76,6 +76,78 @@ void DispatchQueue::Abort(void) { m_queueUpdated.notify_all(); } +void DispatchQueue::WaitForEvent(void) { + std::unique_lock lk(m_dispatchLock); + if (m_aborted) + throw dispatch_aborted_exception(); + + // Unconditional delay: + m_queueUpdated.wait(lk, [this]() -> bool { + if (m_aborted) + throw dispatch_aborted_exception(); + + return + // We will need to transition out if the delay queue receives any items: + !this->m_delayedQueue.empty() || + + // We also transition out if the dispatch queue has any events: + !this->m_dispatchQueue.empty(); + }); + + if (m_dispatchQueue.empty()) { + // The delay queue has items but the dispatch queue does not, we need to switch + // to the suggested sleep timeout variant: + WaitForEventUnsafe(lk, m_delayedQueue.top().GetReadyTime()); + } + else { + // We have an event, we can just hop over to this variant: + DispatchEventUnsafe(lk); + } +} + +bool DispatchQueue::WaitForEvent(std::chrono::milliseconds milliseconds) { + return WaitForEvent(std::chrono::steady_clock::now() + milliseconds); +} + +bool DispatchQueue::WaitForEvent(std::chrono::steady_clock::time_point wakeTime) { + if (wakeTime == std::chrono::steady_clock::time_point::max()) { + // Maximal wait--we can optimize by using the zero-arguments version + return WaitForEvent(), true; + } + + std::unique_lock lk(m_dispatchLock); + return WaitForEventUnsafe(lk, wakeTime); +} + +bool DispatchQueue::WaitForEventUnsafe(std::unique_lock& lk, std::chrono::steady_clock::time_point wakeTime) { + if (m_aborted) + throw dispatch_aborted_exception(); + + while (m_dispatchQueue.empty()) { + // Derive a wakeup time using the high precision timer: + wakeTime = SuggestSoonestWakeupTimeUnsafe(wakeTime); + + // Now we wait, either for the timeout to elapse or for the dispatch queue itself to + // transition to the "aborted" state. + std::cv_status status = m_queueUpdated.wait_until(lk, wakeTime); + + // Short-circuit if the queue was aborted + if (m_aborted) + throw dispatch_aborted_exception(); + + if (PromoteReadyDispatchersUnsafe()) + // Dispatcher is ready to run! Exit our loop and dispatch an event + break; + + if (status == std::cv_status::timeout) + // Can't proceed, queue is empty and nobody is ready to be run + return false; + } + + DispatchEventUnsafe(lk); + return true; +} + bool DispatchQueue::DispatchEvent(void) { std::unique_lock lk(m_dispatchLock); From f882f71a8478848446ca34da4ee8aff38f007e5e Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 30 Mar 2015 23:53:48 -0700 Subject: [PATCH 41/63] Add Barrier functionality Users can now infer progress on a `DispatchQueue` by making use of the `DispatchQueue::Barrier`. This allows users to replace this: std::mutex lock; std::condition_variable cv; bool cond = false; *q += [&] { std::lock_guard lk(lock); cond = true; cv.notify_all(); }; std::unique_lock lk(lock); cv.wait_for(lk, std::chrono::seconds(5), [&] { return cond; }); With this: q.Barrier(std::chrono::seconds(5)); `DispatchQueue::Barrier` also has improved performance because it does not require the user construct an explicit condition variable and mutex every time barrier support is required. --- autowiring/DispatchQueue.h | 11 +++++++++++ src/autowiring/DispatchQueue.cpp | 10 ++++++++++ src/autowiring/test/DispatchQueueTest.cpp | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index 24a925f53..5c279eda0 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -186,6 +186,17 @@ class DispatchQueue { /// void AddExisting(DispatchThunkBase* pBase); + /// + /// Blocks until all dispatchers on the DispatchQueue at the time of the call have been dispatched + /// + /// The maximum amount of time to wait + /// + /// This method does not cause any dispatchers to run. If the underlying dispatch queue does not have an event loop + /// operating on it, this method will deadlock. It is an error for the party responsible for driving the dispatch queue + /// via WaitForEvent or DispatchAllEvents unless that party first delegates the responsibility elsewhere. + /// + bool Barrier(std::chrono::nanoseconds timeout); + /// /// Recommends a point in time to wake up to check for events /// diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index da73d188d..630fb4213 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -179,6 +179,16 @@ void DispatchQueue::AddExisting(DispatchThunkBase* pBase) { OnPended(std::move(lk)); } +bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { + // Set up the lambda: + auto complete = std::make_shared(false); + *this += [complete] { *complete = true; }; + + // Obtain the lock, wait until our variable is satisfied, which might be right away: + std::unique_lock lk(m_dispatchLock); + return m_queueUpdated.wait_for(lk, timeout, [&] {return *complete; }); +} + std::chrono::steady_clock::time_point DispatchQueue::SuggestSoonestWakeupTimeUnsafe(std::chrono::steady_clock::time_point latestTime) const { return diff --git a/src/autowiring/test/DispatchQueueTest.cpp b/src/autowiring/test/DispatchQueueTest.cpp index d2fe730be..8b0f1f47b 100644 --- a/src/autowiring/test/DispatchQueueTest.cpp +++ b/src/autowiring/test/DispatchQueueTest.cpp @@ -2,6 +2,7 @@ #include "stdafx.h" #include #include +#include using namespace std; @@ -67,4 +68,22 @@ TEST_F(DispatchQueueTest, PathologicalStartAndStop){ ASSERT_TRUE(t4->WaitFor(std::chrono::seconds(10))); } +TEST_F(DispatchQueueTest, Waypoint) { + AutoCurrentContext()->Initiate(); + AutoRequired ct; + // Barrier on an empty dispatch queue should return right away: + ASSERT_TRUE(ct->Barrier(std::chrono::seconds(5))) << "Barrier on an empty dispatch queue did not return immediately as expected"; + + // Attach a lambda that will wait until the promise is complete: + std::promise barrier; + *ct += [&] { + auto f = barrier.get_future(); + f.wait(); + }; + ASSERT_FALSE(ct->Barrier(std::chrono::milliseconds(1))) << "Barrier returned even though a dispatcher was not complete"; + barrier.set_value(true); + + // Now we should be able to complete: + ASSERT_TRUE(ct->Barrier(std::chrono::seconds(5))) << "Barrier did not return even though a dispatcher should have completed"; +} From 9ecd37f7c113ced25226939d1fbf60d4ab5f010b Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Tue, 31 Mar 2015 19:52:18 -0700 Subject: [PATCH 42/63] Fix the name of the Barrier test --- src/autowiring/test/DispatchQueueTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/autowiring/test/DispatchQueueTest.cpp b/src/autowiring/test/DispatchQueueTest.cpp index 8b0f1f47b..3ba503b3d 100644 --- a/src/autowiring/test/DispatchQueueTest.cpp +++ b/src/autowiring/test/DispatchQueueTest.cpp @@ -68,7 +68,7 @@ TEST_F(DispatchQueueTest, PathologicalStartAndStop){ ASSERT_TRUE(t4->WaitFor(std::chrono::seconds(10))); } -TEST_F(DispatchQueueTest, Waypoint) { +TEST_F(DispatchQueueTest, Barrier) { AutoCurrentContext()->Initiate(); AutoRequired ct; From b40d0e9b371dad0b231eee70fc5a7c74227c55dd Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 1 Apr 2015 22:34:12 -0700 Subject: [PATCH 43/63] Fix behavior of Barrier when DispatchQueue::Abort is called If a user calls Abort, any active calls to Barrier must throw exceptions if they cannot successfully complete. --- autowiring/DispatchQueue.h | 3 ++ src/autowiring/DispatchQueue.cpp | 6 +++- src/autowiring/test/DispatchQueueTest.cpp | 43 +++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index 5c279eda0..f4c43ce42 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -194,6 +194,9 @@ class DispatchQueue { /// This method does not cause any dispatchers to run. If the underlying dispatch queue does not have an event loop /// operating on it, this method will deadlock. It is an error for the party responsible for driving the dispatch queue /// via WaitForEvent or DispatchAllEvents unless that party first delegates the responsibility elsewhere. + /// + /// If DispatchQueue::Abort() is called before the dispatcher has been completed, this method will throw an exception. + /// If a dispatcher on the underlying DispatchQueue throws an exception, this method will also throw an exception. /// bool Barrier(std::chrono::nanoseconds timeout); diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index 630fb4213..5cfe6625d 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -2,6 +2,7 @@ #include "stdafx.h" #include "DispatchQueue.h" #include "at_exit.h" +#include "autowiring_error.h" #include dispatch_aborted_exception::dispatch_aborted_exception(void){} @@ -186,7 +187,10 @@ bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { // Obtain the lock, wait until our variable is satisfied, which might be right away: std::unique_lock lk(m_dispatchLock); - return m_queueUpdated.wait_for(lk, timeout, [&] {return *complete; }); + bool rv = m_queueUpdated.wait_for(lk, timeout, [&] { return m_aborted || *complete; }); + if (m_aborted) + throw autowiring_error("Dispatch queue was aborted while a barrier was invoked"); + return rv; } std::chrono::steady_clock::time_point diff --git a/src/autowiring/test/DispatchQueueTest.cpp b/src/autowiring/test/DispatchQueueTest.cpp index 3ba503b3d..b07211835 100644 --- a/src/autowiring/test/DispatchQueueTest.cpp +++ b/src/autowiring/test/DispatchQueueTest.cpp @@ -87,3 +87,46 @@ TEST_F(DispatchQueueTest, Barrier) { // Now we should be able to complete: ASSERT_TRUE(ct->Barrier(std::chrono::seconds(5))) << "Barrier did not return even though a dispatcher should have completed"; } + +struct BarrierMonitor { + // Standard continuation behavior: + std::mutex lock; + std::condition_variable cv; + bool done; +}; + +TEST_F(DispatchQueueTest, BarrierWithAbort) { + AutoCurrentContext()->Initiate(); + AutoRequired ct; + + // Hold our initial lockdown: + auto b = std::make_shared(); + std::unique_lock lk(b->lock); + + // This dispatch entry will delay until we're ready for it to continue: + *ct += [b] { + std::lock_guard lk(b->lock); + }; + + // Launch something that will barrier: + auto exception = std::make_shared(false); + auto f = std::async( + std::launch::async, + [=] { + try { + ct->Barrier(std::chrono::seconds(5)); + } + catch (autowiring_error&) { + *exception = true; + } + } + ); + + // Delay for long enough for the barrier to be reached: + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + + // Now abandon the queue, this should cause the async thread to quit: + ct->Abort(); + ASSERT_EQ(std::future_status::ready, f.wait_for(std::chrono::seconds(5))) << "Barrier did not abort fast enough"; + ASSERT_TRUE(*exception) << "Exception should have been thrown inside the Barrier call"; +} \ No newline at end of file From cf94f83976ab4245b6cfdd31357140b059c3e789 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 1 Apr 2015 22:46:02 -0700 Subject: [PATCH 44/63] Add timeout-free Barrier variant This `Barrier` variant is identical to the timed version, except that it does not time out. --- autowiring/DispatchQueue.h | 5 +++++ src/autowiring/DispatchQueue.cpp | 15 +++++++++++++++ src/autowiring/test/DispatchQueueTest.cpp | 2 +- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index f4c43ce42..c024699e7 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -200,6 +200,11 @@ class DispatchQueue { /// bool Barrier(std::chrono::nanoseconds timeout); + /// + /// Identical to the timed version of Barrier, but does not time out + /// + void Barrier(void); + /// /// Recommends a point in time to wake up to check for events /// diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index 5cfe6625d..6ac978a95 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -193,6 +193,21 @@ bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { return rv; } +void DispatchQueue::Barrier(void) { + // Set up the lambda: + bool complete = false; + *this += [&] { complete = true; }; + + // Obtain the lock, wait until our variable is satisfied, which might be right away: + std::unique_lock lk(m_dispatchLock); + m_queueUpdated.wait(lk, [&] { return m_aborted || complete; }); + if (m_aborted) + // At this point, the dispatch queue MUST be completely run down. We have no outstanding references + // to our stack-allocated "complete" variable. Furthermore, after m_aborted is true, no further + // dispatchers are permitted to be run. + throw autowiring_error("Dispatch queue was aborted while a barrier was invoked"); +} + std::chrono::steady_clock::time_point DispatchQueue::SuggestSoonestWakeupTimeUnsafe(std::chrono::steady_clock::time_point latestTime) const { return diff --git a/src/autowiring/test/DispatchQueueTest.cpp b/src/autowiring/test/DispatchQueueTest.cpp index b07211835..19faea4c3 100644 --- a/src/autowiring/test/DispatchQueueTest.cpp +++ b/src/autowiring/test/DispatchQueueTest.cpp @@ -129,4 +129,4 @@ TEST_F(DispatchQueueTest, BarrierWithAbort) { ct->Abort(); ASSERT_EQ(std::future_status::ready, f.wait_for(std::chrono::seconds(5))) << "Barrier did not abort fast enough"; ASSERT_TRUE(*exception) << "Exception should have been thrown inside the Barrier call"; -} \ No newline at end of file +} From 108c2111e0738a55297d43ec6ca3a6100378b49d Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 2 Apr 2015 11:32:03 -0700 Subject: [PATCH 45/63] Eliminate empty test Perhaps we intended to fill this out at some point. For now, however, it's empty and therefore must be removed. --- src/autowiring/test/ExceptionFilterTest.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/autowiring/test/ExceptionFilterTest.cpp b/src/autowiring/test/ExceptionFilterTest.cpp index 782e56541..1e36db4de 100644 --- a/src/autowiring/test/ExceptionFilterTest.cpp +++ b/src/autowiring/test/ExceptionFilterTest.cpp @@ -237,7 +237,3 @@ TEST_F(ExceptionFilterTest, VerifySimpleConfinement) { // Verify that the child scope was terminated as expected: EXPECT_TRUE(child->IsShutdown()) << "An event recipient in a child scope threw an exception and the child context was not correctly terminated"; } - -TEST_F(ExceptionFilterTest, NoRecursiveShutdowns) { - -} From 07ac8f4bf2bc5d154aa6d390ff02524d3cc3dbfb Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 2 Apr 2015 11:30:36 -0700 Subject: [PATCH 46/63] Clarify a documentation note about ExceptionFilter This is to ensure that users are aware that this is not a filter in the proper sense, but actually a Listener. The current misunderstanding as to this class' purpose should be taken as an indicator that the class may have an inappropriate name. --- autowiring/ExceptionFilter.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/autowiring/ExceptionFilter.h b/autowiring/ExceptionFilter.h index 312fba13a..1f6e4ca0e 100644 --- a/autowiring/ExceptionFilter.h +++ b/autowiring/ExceptionFilter.h @@ -15,6 +15,7 @@ class CoreObject; /// filter the passed exception. Generally, the filtration technique should be written /// as follows: /// +/// \code /// try { /// throw; /// } catch(custom_type_1& t1) { @@ -22,17 +23,21 @@ class CoreObject; /// } catch(custom_type_2& t2) { /// ...handling code... /// } +/// \endcode /// /// Alternatively, this strategy may be used: /// +/// \code /// try { /// throw; /// } catch(custom_type_1&) { /// } catch(custom_type_2&) { /// } /// ...handling code... +/// \endcode /// /// Filtration methods may safely allow any unhandled rethrows to percolate to the caller. +/// Autowiring will ignore any exceptions thrown by a filter method. /// /// Unhandled exceptions thrown in a context will cause that context to be torn down. By /// the time the exception filter is called, teardown of the context originating the @@ -59,6 +64,8 @@ class ExceptionFilter /// prevent certain exceptions from being unhandled. /// /// Implementors can use "throw" with no arguments to trigger a rethrow of the originating exception. + /// + /// Exceptions thrown by this method are silently ignored by Autowiring. /// virtual void Filter(void) {}; @@ -68,6 +75,8 @@ class ExceptionFilter /// The target of the call /// /// Implementors can use "throw" with no arguments to trigger a rethrow of the originating exception. + /// + /// Exceptions thrown by this method are silently ignored by Autowiring. /// virtual void Filter(const JunctionBoxBase* pJunctionBox, CoreObject* pRecipient) {} }; From a263370c6e54d57b735ac537e3107cab5e07c57c Mon Sep 17 00:00:00 2001 From: Jimmy Nguyen Date: Thu, 2 Apr 2015 12:34:13 -0700 Subject: [PATCH 47/63] dispatch_aborted_exception should inherit from autowiring_error --- autowiring/DispatchQueue.h | 5 +++-- src/autowiring/BasicThread.cpp | 3 ++- src/autowiring/DispatchQueue.cpp | 13 ++++++++----- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index c024699e7..ad99ee2ed 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -1,5 +1,6 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once +#include "autowiring_error.h" #include "DispatchThunk.h" #include #include @@ -14,10 +15,10 @@ class DispatchQueue; /// Thrown when a dispatch operation was aborted /// class dispatch_aborted_exception: - public std::exception + public autowiring_error { public: - dispatch_aborted_exception(void); + dispatch_aborted_exception(const std::string& what); virtual ~dispatch_aborted_exception(void); }; diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index b4e85894a..723dbd6a6 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -2,6 +2,7 @@ #include "stdafx.h" #include "BasicThread.h" #include "Autowired.h" +#include "autowiring_error.h" #include "BasicThreadStateBlock.h" #include "ContextEnumerator.h" #include "fast_pointer_cast.h" @@ -109,7 +110,7 @@ void BasicThread::WaitForStateUpdate(const std::function& fn) { } ); if(ShouldStop()) - throw dispatch_aborted_exception(); + throw dispatch_aborted_exception("Thread was stopped before the function returned true"); } void BasicThread::PerformStatusUpdate(const std::function& fn) { diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index 6ac978a95..a2f1653ff 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -5,7 +5,10 @@ #include "autowiring_error.h" #include -dispatch_aborted_exception::dispatch_aborted_exception(void){} +dispatch_aborted_exception::dispatch_aborted_exception(const std::string& what): + autowiring_error(what) +{} + dispatch_aborted_exception::~dispatch_aborted_exception(void){} DispatchQueue::DispatchQueue(void): @@ -80,12 +83,12 @@ void DispatchQueue::Abort(void) { void DispatchQueue::WaitForEvent(void) { std::unique_lock lk(m_dispatchLock); if (m_aborted) - throw dispatch_aborted_exception(); + throw dispatch_aborted_exception("Dispatch queue was aborted while waiting for an event"); // Unconditional delay: m_queueUpdated.wait(lk, [this]() -> bool { if (m_aborted) - throw dispatch_aborted_exception(); + throw dispatch_aborted_exception("Dispatch queue was aborted while waiting for an event"); return // We will need to transition out if the delay queue receives any items: @@ -122,7 +125,7 @@ bool DispatchQueue::WaitForEvent(std::chrono::steady_clock::time_point wakeTime) bool DispatchQueue::WaitForEventUnsafe(std::unique_lock& lk, std::chrono::steady_clock::time_point wakeTime) { if (m_aborted) - throw dispatch_aborted_exception(); + throw dispatch_aborted_exception("Dispatch queue was aborted while waiting for an event"); while (m_dispatchQueue.empty()) { // Derive a wakeup time using the high precision timer: @@ -134,7 +137,7 @@ bool DispatchQueue::WaitForEventUnsafe(std::unique_lock& lk, std::ch // Short-circuit if the queue was aborted if (m_aborted) - throw dispatch_aborted_exception(); + throw dispatch_aborted_exception("Dispatch queue was aborted while waiting for an event"); if (PromoteReadyDispatchersUnsafe()) // Dispatcher is ready to run! Exit our loop and dispatch an event From 73e0f1ef69392cb95d0bb15651bdc296dda4b48f Mon Sep 17 00:00:00 2001 From: Jimmy Nguyen Date: Thu, 2 Apr 2015 13:00:08 -0700 Subject: [PATCH 48/63] DispatchQueue::Barrier(*) functions should throw dispatch_aborted_exception --- src/autowiring/DispatchQueue.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index a2f1653ff..bd1cff919 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -2,7 +2,6 @@ #include "stdafx.h" #include "DispatchQueue.h" #include "at_exit.h" -#include "autowiring_error.h" #include dispatch_aborted_exception::dispatch_aborted_exception(const std::string& what): @@ -192,7 +191,7 @@ bool DispatchQueue::Barrier(std::chrono::nanoseconds timeout) { std::unique_lock lk(m_dispatchLock); bool rv = m_queueUpdated.wait_for(lk, timeout, [&] { return m_aborted || *complete; }); if (m_aborted) - throw autowiring_error("Dispatch queue was aborted while a barrier was invoked"); + throw dispatch_aborted_exception("Dispatch queue was aborted while a barrier was invoked"); return rv; } @@ -208,7 +207,7 @@ void DispatchQueue::Barrier(void) { // At this point, the dispatch queue MUST be completely run down. We have no outstanding references // to our stack-allocated "complete" variable. Furthermore, after m_aborted is true, no further // dispatchers are permitted to be run. - throw autowiring_error("Dispatch queue was aborted while a barrier was invoked"); + throw dispatch_aborted_exception("Dispatch queue was aborted while a barrier was invoked"); } std::chrono::steady_clock::time_point From 992bff308141314d4e20cf324a66ec3a67f27162 Mon Sep 17 00:00:00 2001 From: Jimmy Nguyen Date: Thu, 2 Apr 2015 13:10:07 -0700 Subject: [PATCH 49/63] Tweaking the error strings to differentiate between exception prior to or during WaitForEvent() --- src/autowiring/DispatchQueue.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index bd1cff919..07841cdb2 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -82,7 +82,7 @@ void DispatchQueue::Abort(void) { void DispatchQueue::WaitForEvent(void) { std::unique_lock lk(m_dispatchLock); if (m_aborted) - throw dispatch_aborted_exception("Dispatch queue was aborted while waiting for an event"); + throw dispatch_aborted_exception("Dispatch queue was aborted prior to waiting for an event"); // Unconditional delay: m_queueUpdated.wait(lk, [this]() -> bool { @@ -124,7 +124,7 @@ bool DispatchQueue::WaitForEvent(std::chrono::steady_clock::time_point wakeTime) bool DispatchQueue::WaitForEventUnsafe(std::unique_lock& lk, std::chrono::steady_clock::time_point wakeTime) { if (m_aborted) - throw dispatch_aborted_exception("Dispatch queue was aborted while waiting for an event"); + throw dispatch_aborted_exception("Dispatch queue was aborted prior to waiting for an event"); while (m_dispatchQueue.empty()) { // Derive a wakeup time using the high precision timer: From 96aa81f0126138bcc7d7ecaec5d4f3c32dc159d6 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 1 Apr 2015 23:12:41 -0700 Subject: [PATCH 50/63] Add an interface support query ability to CoreContext This allows users to get all implementors of a particular interface in a context. This method returns an empty vector if it is called before the context is initiated. --- autowiring/CoreContext.h | 33 ++++++++++ autowiring/JunctionBox.h | 80 ++++++++++++++++++++++++- autowiring/JunctionBoxBase.h | 6 ++ autowiring/JunctionBoxEntry.h | 13 ++-- src/autowiring/CoreContext.cpp | 7 +++ src/autowiring/test/CoreContextTest.cpp | 30 ++++++++++ 6 files changed, 160 insertions(+), 9 deletions(-) diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index 3ef0505f5..f1d3b4ed7 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -750,6 +750,39 @@ class CoreContext: FindByType(ptr); return ptr != nullptr; } + + /// + /// Runtime variant of All + /// + /// + /// This instance does not cause a correct instantiation of the underlying junction box. Users are cautioned against + /// using this method directly unless they are able to ensure a proper entry is made into the type registry. + /// + /// It is an error to call this method on an unregistered type. + /// + JunctionBoxBase& All(const std::type_info& ti) const; + + /// + /// Returns all members of and snoopers on this context which implement the specified interface + /// + /// + /// This method makes use of the JunctionBox subsystem, and thus is extremely efficient. The underlying system is + /// memoized, and new entries automatically update any existing memos, which gives this routine O(n) efficiency, where + /// n is the number of types in this context that implement the specified interface. + /// + /// Note that the junction box will also contain members of child contexts that implement the specified interface, and + /// instances that are snooping this context. + /// + /// This method's result will be an empty vector if the context is not currently initiated. + /// + template + JunctionBox& All(void) { + // Type registration, needed to ensure our junction box actually exists + (void) RegEvent::r; + + // Simple coercive transfer: + return static_cast&>(All(typeid(T))); + } template std::shared_ptr DEPRECATED(Construct(Args&&... args), "'Construct' is deprecated, use 'Inject' instead"); diff --git a/autowiring/JunctionBox.h b/autowiring/JunctionBox.h index 8bb03f78c..45d25594c 100644 --- a/autowiring/JunctionBox.h +++ b/autowiring/JunctionBox.h @@ -43,9 +43,83 @@ class JunctionBox: volatile int m_numberOfDeletions; public: - /// - /// Convenience method allowing consumers to quickly determine whether any listeners exist - /// + class iterator { + public: + iterator(void) {} + + iterator(JunctionBox* pParent) : + pParent(pParent), + q(pParent->m_st.end()), + deleteCount(pParent->m_numberOfDeletions) + {} + + iterator(JunctionBox* pParent, typename t_listenerSet::iterator q) : + pParent(pParent), + q(q), + currentEvent(*q), + deleteCount(pParent->m_numberOfDeletions) + {} + + typedef std::forward_iterator_tag iterator_category; + + private: + JunctionBox* pParent = nullptr; + typename t_listenerSet::iterator q; + JunctionBoxEntry currentEvent; + int deleteCount; + + public: + // Required operator overloads: + T& operator*(void) const { return *currentEvent.m_ptr; } + T* operator->(void) const { return currentEvent.m_ptr.get(); } + bool operator==(const iterator& rhs) const { return q == rhs.q; } + bool operator!=(const iterator& rhs) const { return q != rhs.q; } + bool operator<(const iterator& rhs) const { return q < rhs.q; } + + iterator operator++(void) { + // Need to hold this here to prevent deletion from occuring while the lock is held + std::shared_ptr old = currentEvent.m_ptr; + std::lock_guard lk(pParent->m_lock); + + // Increment iterator correctly even if it's been invalidated + if (deleteCount == pParent->m_numberOfDeletions) + ++q; + else { + q = pParent->m_st.upper_bound(currentEvent); + deleteCount = pParent->m_numberOfDeletions; + } + + // Only update if we aren't at the end: + currentEvent = + q == pParent->m_st.end() ? + JunctionBoxEntry() : + *q; + return *this; + } + + iterator operator++(int) { + iterator prior = *this; + *this++; + return prior; + } + }; + + iterator begin(void) { + std::lock_guard lk(m_lock); + return iterator(this, m_st.begin()); + } + + iterator end(void) { + std::lock_guard lk(m_lock); + return iterator(this); + } + + size_t size(void) const { + std::lock_guard lk(m_lock); + return m_st.size(); + } + + // JunctionBoxBase overrides: bool HasListeners(void) const override { return (std::lock_guard)m_lock, !m_st.empty(); } diff --git a/autowiring/JunctionBoxBase.h b/autowiring/JunctionBoxBase.h index d942b6de4..87aca21f9 100644 --- a/autowiring/JunctionBoxBase.h +++ b/autowiring/JunctionBoxBase.h @@ -52,6 +52,12 @@ class JunctionBoxBase { bool IsInitiated(void) const {return m_isInitiated;} void Initiate(void) {m_isInitiated=true;} + /// + /// Convenience method allowing consumers to quickly determine whether any listeners exist + /// + /// + /// True if at least one listener has been registered + /// virtual bool HasListeners(void) const = 0; // Event attachment and detachment pure virtuals diff --git a/autowiring/JunctionBoxEntry.h b/autowiring/JunctionBoxEntry.h index 6dde5e5da..38bc66fe8 100644 --- a/autowiring/JunctionBoxEntry.h +++ b/autowiring/JunctionBoxEntry.h @@ -6,11 +6,15 @@ class CoreContext; struct JunctionBoxEntryBase { + JunctionBoxEntryBase(void) : + m_owner(nullptr) + {} + JunctionBoxEntryBase(CoreContext* owner) : m_owner(owner) {} - CoreContext* const m_owner; + CoreContext* m_owner; }; /// @@ -20,6 +24,8 @@ template struct JunctionBoxEntry: JunctionBoxEntryBase { + JunctionBoxEntry(void) {} + JunctionBoxEntry(CoreContext* owner, std::shared_ptr ptr) : JunctionBoxEntryBase(owner), m_ptr(ptr) @@ -27,11 +33,6 @@ struct JunctionBoxEntry: std::shared_ptr m_ptr; - JunctionBoxEntry& operator=(const JunctionBoxEntry& rhs) { - // This shouldn't be used. non-c++11 containers require this... - throw std::runtime_error("Can't copy a JunctionBoxEntry"); - } - bool operator==(const JunctionBoxEntry& rhs) const { return m_ptr == rhs.m_ptr; } diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index 72909cb32..57a436745 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -630,6 +630,13 @@ void CoreContext::AddBolt(const std::shared_ptr& pBase) { m_nameListeners[typeid(void)].push_back(pBase.get()); } +JunctionBoxBase& CoreContext::All(const std::type_info& ti) const { + auto jb = m_junctionBoxManager->Get(ti); + if (!jb) + throw autowiring_error("Attempted to obtain a junction box which has not been declared"); + return *jb; +} + void CoreContext::BuildCurrentState(void) { AutoGlobalContext glbl; glbl->Invoke(&AutowiringEvents::NewContext)(*this); diff --git a/src/autowiring/test/CoreContextTest.cpp b/src/autowiring/test/CoreContextTest.cpp index fada6dfec..4f3be7560 100644 --- a/src/autowiring/test/CoreContextTest.cpp +++ b/src/autowiring/test/CoreContextTest.cpp @@ -434,3 +434,33 @@ TEST_F(CoreContextTest, AppropriateShutdownInterleave) { ASSERT_TRUE(ctxtOuter->Wait(std::chrono::seconds(5))) << "Outer context did not tear down in a timely fashion"; ASSERT_TRUE(ctxtInner->Wait(std::chrono::seconds(5))) << "Inner context did not tear down in a timely fashion"; } + +class MyClassForAllBase {}; + +template +class MyClassForAll: + public MyClassForAllBase +{ +}; + +TEST_F(CoreContextTest, All) { + AutoCurrentContext ctxt; + ctxt->Initiate(); + + AutoRequired> c1; + AutoRequired> c2; + + // Sanity check first: + auto& allMembers = ctxt->All(); + ASSERT_LE(2UL, allMembers.size()) << "All members not correctly identified by call to all"; + + bool found1; + bool found2; + for (auto& cur : allMembers) { + found1 = &cur == c1.get(); + found2 = &cur == c2.get(); + } + + ASSERT_TRUE(found1) << "Failed to find MyClassForAll<1> via its ContextMember interface"; + ASSERT_TRUE(found2) << "Failed to find MyClassForAll<2> via its ContextMember interface"; +} From 41bed2ddedbc1592425750b9f1799a20231b2095 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Fri, 3 Apr 2015 10:56:04 -0700 Subject: [PATCH 51/63] Fix test logic --- autowiring/CoreContext.h | 2 +- src/autowiring/test/CoreContextTest.cpp | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index f1d3b4ed7..4130771c0 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -773,7 +773,7 @@ class CoreContext: /// Note that the junction box will also contain members of child contexts that implement the specified interface, and /// instances that are snooping this context. /// - /// This method's result will be an empty vector if the context is not currently initiated. + /// This method's result will be an empty iterable if the context is not currently initiated. /// template JunctionBox& All(void) { diff --git a/src/autowiring/test/CoreContextTest.cpp b/src/autowiring/test/CoreContextTest.cpp index 4f3be7560..287ae18e7 100644 --- a/src/autowiring/test/CoreContextTest.cpp +++ b/src/autowiring/test/CoreContextTest.cpp @@ -454,11 +454,12 @@ TEST_F(CoreContextTest, All) { auto& allMembers = ctxt->All(); ASSERT_LE(2UL, allMembers.size()) << "All members not correctly identified by call to all"; - bool found1; - bool found2; + // Check all instances found + bool found1 = false; + bool found2 = false; for (auto& cur : allMembers) { - found1 = &cur == c1.get(); - found2 = &cur == c2.get(); + found1 |= &cur == c1.get(); + found2 |= &cur == c2.get(); } ASSERT_TRUE(found1) << "Failed to find MyClassForAll<1> via its ContextMember interface"; From 90a7a088eac6ca5fc08f13ca34c2fc465dacf267 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 6 Apr 2015 17:52:39 -0700 Subject: [PATCH 52/63] Split multi-decorate tests into their own source file --- .../test/AutoFilterFunctionTest.cpp | 20 +++++- .../test/AutoFilterMultiDecorateTest.cpp | 66 +++++++++++++++++++ src/autowiring/test/AutoPacketFactoryTest.cpp | 59 +---------------- src/autowiring/test/CMakeLists.txt | 1 + 4 files changed, 87 insertions(+), 59 deletions(-) create mode 100644 src/autowiring/test/AutoFilterMultiDecorateTest.cpp diff --git a/src/autowiring/test/AutoFilterFunctionTest.cpp b/src/autowiring/test/AutoFilterFunctionTest.cpp index dfd033fc5..fdf84cbbd 100644 --- a/src/autowiring/test/AutoFilterFunctionTest.cpp +++ b/src/autowiring/test/AutoFilterFunctionTest.cpp @@ -125,7 +125,6 @@ TEST_F(AutoFilterFunctionalTest, ObservingFunctionTest) { ASSERT_TRUE(*called) << "Receive-only filter attached to a const packet image was not correctly called"; } - TEST_F(AutoFilterFunctionalTest, TripleFunctionTest) { AutoRequired factory; @@ -151,3 +150,22 @@ TEST_F(AutoFilterFunctionalTest, TripleFunctionTest) { ASSERT_TRUE(*called1) << "Final observer method was not invoked as expected"; ASSERT_TRUE(*called2) << "Two-argument observer method not invoked as expected"; } + +TEST_F(AutoFilterFunctionalTest, MultiPostHocIntroductionTest) { + AutoCurrentContext()->Initiate(); + AutoRequired factory; + + int called = 0; + + *factory += [&called](int& out) { out = called++; }; + *factory += [&called](int& out) { out = called++; }; + + // Add a gather step on the packet: + auto packet = factory->NewPacket(); + *packet += [&called](const int* vals []){ + ASSERT_NE(nullptr, vals); + called++; + }; + + ASSERT_EQ(3, called) << "Not all lambda functions were called as expected"; +} diff --git a/src/autowiring/test/AutoFilterMultiDecorateTest.cpp b/src/autowiring/test/AutoFilterMultiDecorateTest.cpp new file mode 100644 index 000000000..b26ff1d74 --- /dev/null +++ b/src/autowiring/test/AutoFilterMultiDecorateTest.cpp @@ -0,0 +1,66 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include +#include CHRONO_HEADER +#include THREAD_HEADER + +class AutoFilterMultiDecorateTest: + public testing::Test +{ +public: + AutoFilterMultiDecorateTest(void) { + AutoCurrentContext()->Initiate(); + } + + AutoRequired factory; +}; + +TEST_F(AutoFilterMultiDecorateTest, EnumerateDecorationsTest) { + auto sample = [](const int* vals []) {}; + AutoFilterDescriptor desc(sample); + + size_t i = 0; + for (auto* pCur = desc.GetAutoFilterInput(); *pCur; pCur++) + i++; + + ASSERT_EQ(1, i) << "AutoFilterDescriptor parsed an incorrect number of arguments from a lambda"; +} + +TEST_F(AutoFilterMultiDecorateTest, MultiDecorateTest) { + int called = 0; + + *factory += [&called](int& out) { out = called++; }; + *factory += [&called](int& out) { out = called++; }; + *factory += [&called](const int* vals []) { + ASSERT_NE(nullptr, vals); + called++; + + // Guarantee that values were added in the expected order + int i; + for (i = 0; vals[i]; i++) + ASSERT_EQ(i, *(vals[i])) << "Incorrect values were added to the packet"; + + // Verify we got the number of values out that we wanted to get out + ASSERT_EQ(2, i) << "The wrong number of values were added to the packet"; + }; + ASSERT_EQ(0, called) << "Lambda functions were called before expected"; + + auto packet = factory->NewPacket(); + ASSERT_EQ(3, called) << "Not all lambda functions were called as expected"; +} + +TEST_F(AutoFilterMultiDecorateTest, MultiPostHocIntroductionTest) { + int called = 0; + + *factory += [&called](int& out) { out = called++; }; + *factory += [&called](int& out) { out = called++; }; + + // Add a gather step on the packet: + auto packet = factory->NewPacket(); + *packet += [&called](const int* vals []){ + ASSERT_NE(nullptr, vals); + called++; + }; + + ASSERT_EQ(3, called) << "Not all lambda functions were called as expected"; +} diff --git a/src/autowiring/test/AutoPacketFactoryTest.cpp b/src/autowiring/test/AutoPacketFactoryTest.cpp index 4b3e50ca3..a6940c8ee 100644 --- a/src/autowiring/test/AutoPacketFactoryTest.cpp +++ b/src/autowiring/test/AutoPacketFactoryTest.cpp @@ -199,63 +199,6 @@ TEST_F(AutoPacketFactoryTest, AddSubscriberTest) { ASSERT_TRUE(second_called) << "Subscriber added with operator+= never called"; } -TEST_F(AutoPacketFactoryTest, EnumerateDecorationsTest) { - auto sample = [](const int* vals []) {}; - AutoFilterDescriptor desc(sample); - - size_t i = 0; - for (auto* pCur = desc.GetAutoFilterInput(); *pCur; pCur++) - i++; - - ASSERT_EQ(1, i) << "AutoFilterDescriptor parsed an incorrect number of arguments from a lambda"; -} - -TEST_F(AutoPacketFactoryTest, MultiDecorateTest) { - AutoCurrentContext ctxt; - AutoRequired factory(ctxt); - ctxt->Initiate(); - - int called = 0; - - *factory += [&called] (int& out) { out = called++; }; - *factory += [&called] (int& out) { out = called++; }; - *factory += [&called] (const int* vals[]) { - ASSERT_NE(nullptr, vals); - called++; - - // Guarantee that values were added in the expected order - int i; - for (i = 0; vals[i]; i++) - ASSERT_EQ(i, *(vals[i])) << "Incorrect values were added to the packet"; - - // Verify we got the number of values out that we wanted to get out - ASSERT_EQ(2, i) << "The wrong number of values were added to the packet"; - }; - ASSERT_EQ(0, called) << "Lambda functions were called before expected"; - - auto packet = factory->NewPacket(); - ASSERT_EQ(3, called) << "Not all lambda functions were called as expected"; -} - -TEST_F(AutoPacketFactoryTest, MultiPostHocIntroductionTest) { - AutoCurrentContext()->Initiate(); - AutoRequired factory; - - int called = 0; - - *factory += [&called](int& out) { out = called++; }; - *factory += [&called](int& out) { out = called++; }; - - // Add a gather step on the packet: - auto packet = factory->NewPacket(); - *packet += [&called](const int* vals []){ - ASSERT_NE(nullptr, vals); - called++; - }; - - ASSERT_EQ(3, called) << "Not all lambda functions were called as expected"; -} - TEST_F(AutoPacketFactoryTest, CanRemoveAddedLambda) { AutoCurrentContext()->Initiate(); AutoRequired factory; @@ -267,4 +210,4 @@ TEST_F(AutoPacketFactoryTest, CanRemoveAddedLambda) { ASSERT_TRUE(packet1->Has()) << "First packet did not posess expected decoration"; ASSERT_FALSE(packet2->Has()) << "Decoration present even after all filters were removed from a factory"; -} \ No newline at end of file +} diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index 6c5a1753c..c590d49b7 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -8,6 +8,7 @@ set(AutowiringTest_SRCS AutoFilterCollapseRulesTest.cpp AutoFilterDiagnosticsTest.cpp AutoFilterFunctionTest.cpp + AutoFilterMultiDecorateTest.cpp AutoFilterSequencing.cpp AutoFilterTest.cpp AutoInjectableTest.cpp From 3085e232258b1769edb1d26d1394453bdd76a47c Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Tue, 7 Apr 2015 11:33:47 -0700 Subject: [PATCH 53/63] Move dispatch_aborted_exception to its own file, add default ctor Adding an exception explanation behavior to `dispatch_aborted_exception` is nice, but it still must be possible to default-construct this type. Also provide it its own header. --- autowiring/DispatchQueue.h | 14 +------------- autowiring/dispatch_aborted_exception.h | 16 ++++++++++++++++ src/autowiring/CMakeLists.txt | 2 ++ src/autowiring/DispatchQueue.cpp | 6 ------ src/autowiring/dispatch_aborted_exception.cpp | 13 +++++++++++++ 5 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 autowiring/dispatch_aborted_exception.h create mode 100644 src/autowiring/dispatch_aborted_exception.cpp diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index ad99ee2ed..bd273d7f9 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -1,6 +1,6 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once -#include "autowiring_error.h" +#include "dispatch_aborted_exception.h" #include "DispatchThunk.h" #include #include @@ -10,18 +10,6 @@ class DispatchQueue; -/// \internal -/// -/// Thrown when a dispatch operation was aborted -/// -class dispatch_aborted_exception: - public autowiring_error -{ -public: - dispatch_aborted_exception(const std::string& what); - virtual ~dispatch_aborted_exception(void); -}; - /// /// This is an asynchronous queue of zero-argument functions /// diff --git a/autowiring/dispatch_aborted_exception.h b/autowiring/dispatch_aborted_exception.h new file mode 100644 index 000000000..d0ee7080a --- /dev/null +++ b/autowiring/dispatch_aborted_exception.h @@ -0,0 +1,16 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#pragma once +#include "autowiring_error.h" + +/// \internal +/// +/// Thrown when a dispatch operation was aborted +/// +class dispatch_aborted_exception: + public autowiring_error +{ +public: + dispatch_aborted_exception(void); + dispatch_aborted_exception(const std::string& what); + virtual ~dispatch_aborted_exception(void); +}; diff --git a/src/autowiring/CMakeLists.txt b/src/autowiring/CMakeLists.txt index 1c2bdc550..572b3e5b2 100644 --- a/src/autowiring/CMakeLists.txt +++ b/src/autowiring/CMakeLists.txt @@ -100,6 +100,8 @@ set(Autowiring_SRCS demangle.cpp demangle.h Deserialize.h + dispatch_aborted_exception.h + dispatch_aborted_exception.cpp DispatchQueue.cpp DispatchQueue.h DispatchThunk.h diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index 07841cdb2..ec143ce1b 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -4,12 +4,6 @@ #include "at_exit.h" #include -dispatch_aborted_exception::dispatch_aborted_exception(const std::string& what): - autowiring_error(what) -{} - -dispatch_aborted_exception::~dispatch_aborted_exception(void){} - DispatchQueue::DispatchQueue(void): m_dispatchCap(1024), m_aborted(false) diff --git a/src/autowiring/dispatch_aborted_exception.cpp b/src/autowiring/dispatch_aborted_exception.cpp new file mode 100644 index 000000000..9730993e7 --- /dev/null +++ b/src/autowiring/dispatch_aborted_exception.cpp @@ -0,0 +1,13 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include "dispatch_aborted_exception.h" + +dispatch_aborted_exception::dispatch_aborted_exception(void) : + autowiring_error("Dispatch queue aborted") +{} + +dispatch_aborted_exception::dispatch_aborted_exception(const std::string& what) : + autowiring_error(what) +{} + +dispatch_aborted_exception::~dispatch_aborted_exception(void){} From 0f8a22a82e2c866fc7f9861cb0d82d6de91a648a Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 9 Apr 2015 02:11:44 -0700 Subject: [PATCH 54/63] CoreContext::NotifyWhenAutowired is not actually internal It is expected that users will invoke this routine in order to add notification lambdas without having to expressly declare an Autowired slot. Thus, we should allow users a way to do this directly. --- autowiring/CoreContext.h | 1 - 1 file changed, 1 deletion(-) diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index 4130771c0..78a05df71 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -1185,7 +1185,6 @@ class CoreContext: } }; - /// \internal /// /// Adds a post-attachment listener in this context for a particular autowired member. /// There is no guarantee for the context in which the listener will be called. From af002c732d1a6258fa9a62e5ea8f6145856fbfb7 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 9 Apr 2015 15:27:05 -0700 Subject: [PATCH 55/63] Remove unused LIBCXX preprocessor conditionals --- autowiring/DecorationDisposition.h | 4 ---- src/autonet/AutoNetServerImpl.hpp | 5 ----- 2 files changed, 9 deletions(-) diff --git a/autowiring/DecorationDisposition.h b/autowiring/DecorationDisposition.h index d3ad221fa..31e8edc5a 100644 --- a/autowiring/DecorationDisposition.h +++ b/autowiring/DecorationDisposition.h @@ -49,11 +49,7 @@ namespace std { return key.tshift + (key.is_shared ? 0x80000 : 0x70000) + -#if AUTOWIRING_USE_LIBCXX key.ti->hash_code(); -#else - std::type_index(*key.ti).hash_code(); -#endif } }; } diff --git a/src/autonet/AutoNetServerImpl.hpp b/src/autonet/AutoNetServerImpl.hpp index c8b98dd3b..5f4ab9d97 100644 --- a/src/autonet/AutoNetServerImpl.hpp +++ b/src/autonet/AutoNetServerImpl.hpp @@ -9,11 +9,6 @@ #include SYSTEM_ERROR_HEADER #include ARRAY_HEADER -#if !AUTOWIRING_USE_LIBCXX - // No initializer lists on libstdc - #define BOOST_NO_CXX11_HDR_INITIALIZER_LIST -#endif - struct CoreObjectDescriptor; struct TypeIdentifierBase; From 989aab0df501285bc7c3166ea519977652c2c410 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 9 Apr 2015 17:11:37 -0700 Subject: [PATCH 56/63] Always use visibility hidden Requested by @gittyupagain. Fixes problems when linking multiple libraries that use autowiring --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fa25903a8..610b45fee 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,9 +52,9 @@ endif() message(STATUS "Compiler version ${CMAKE_CXX_COMPILER_VERSION}") -# Always use c++11 compiler +# Always use c++11 compiler with hidden visibility if(NOT WIN32) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -fvisibility=hidden") endif() # Clang needs special additional flags to build with C++11 From 965c67e48da8491fb4f3156dc36a4d2a26dd7ed8 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 16 Apr 2015 13:32:25 -0700 Subject: [PATCH 57/63] Fix Android build Forgot to include a header required to get Autowiring building on Android --- src/autowiring/test/DispatchQueueTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/autowiring/test/DispatchQueueTest.cpp b/src/autowiring/test/DispatchQueueTest.cpp index 19faea4c3..18157b0a5 100644 --- a/src/autowiring/test/DispatchQueueTest.cpp +++ b/src/autowiring/test/DispatchQueueTest.cpp @@ -2,7 +2,8 @@ #include "stdafx.h" #include #include -#include +#include +#include FUTURE_HEADER using namespace std; From e965d7da2f96271c871dd3e5e9cb2865ca2b99a1 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Thu, 16 Apr 2015 19:16:55 -0700 Subject: [PATCH 58/63] Added a test exposing an invalid memory access error that needs to be protected against. --- src/autowiring/test/AutoSignalTest.cpp | 59 ++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/autowiring/test/AutoSignalTest.cpp b/src/autowiring/test/AutoSignalTest.cpp index fbe946125..bc3f9b516 100644 --- a/src/autowiring/test/AutoSignalTest.cpp +++ b/src/autowiring/test/AutoSignalTest.cpp @@ -68,6 +68,65 @@ TEST_F(AutoSignalTest, SignalWithAutowiring) { ASSERT_FALSE(handler_called) << "A handler was unexpectedly called after it should have been destroyed"; } +struct RaisesASignalDerived : public RaisesASignal { + +}; + +struct ContainsRaises { + ContainsRaises() : count(0) { + ras(&RaisesASignal::signal) += [this](int v) { + count++; + }; + } + + ~ContainsRaises() { + static int i = 0; + i++; + } + Autowired ras; + int count; +}; + +TEST_F(AutoSignalTest, ConstructorAutowiredRegistration) { + bool handler_called = false; + int val = 202; + + { + AutoCurrentContext rootContext; + auto ctxt = rootContext->Create(); + CurrentContextPusher pshr(ctxt); + (void)pshr; + { + AutoRequired cRas; + + //Autowired ras; + Autowired rasDerived; + + // Register a signal handler: + rasDerived(&RaisesASignal::signal) += [&](int v) { + handler_called = true; + val = v; + }; + + // Inject type type after the signal has been registered + AutoRequired(); + + // Now raise the signal, see what happens: + rasDerived->signal(55); + + // Verify that the handler got called with the correct value: + ASSERT_TRUE(handler_called) << "Signal handler was not invoked"; + ASSERT_EQ(55, val) << "Signal handler not called with the correct parameter as expected"; + } + + // Raise the signal again, this should not cause anything to break: + Autowired ras; + handler_called = false; + ras->signal(99); + ASSERT_FALSE(handler_called) << "A handler was unexpectedly called after it should have been destroyed"; + } +} + TEST_F(AutoSignalTest, MultipleSlotsTest) { autowiring::signal signal; From 8eae66faac2f068ff4d2a245e7b49ed79ba2b7f1 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 17 Apr 2015 02:06:30 -0700 Subject: [PATCH 59/63] Remove irrelevant contributors from contributors list --- CONTRIBUTORS.txt | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 09b0a5b45..79ebbaad4 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -117,38 +117,3 @@ ====================================================== -1. Jared Deckard -Github account: deckar01 -Email: jared.deckard@gmail.com - -2. Stu Kabakoff -Github account: sakabako -Email: sakabako@gmail.com - -3. Yoshihiro Iwanaga -Github account: iwanaga -Email: iwanaga.blackie@gmail.com - -4. ARJUNKUMAR KRISHNAMOORTHY -Github account: tk120404 -Email: Arjunkumartk@gmail.com - -5. Rob Witoff -Github account: witoff -Email: leap@pspct.com - -6. Richard Pearson -Github account: catdevnull -Email: github@catdevnull.co.uk - -7. Ben Nortier -Github account: bjnortier -Email: bjnortier@gmail.com - -8. Andrew Kennedy -Github account: akenn -Email: andrew@akenn.org - -9. Victor Norgren -Github account: logotype -Email: victor@logotype.se From 49c834f85e9e206f435051ec375791b6ec3c51af Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 17 Apr 2015 11:09:01 -0700 Subject: [PATCH 60/63] Fixed the problem - autowired fields that use the autowiredField(&Class::signal) += syntax will now keep a shared pointer to the signal table to ensure correct teardown order. --- autowiring/Autowired.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/autowiring/Autowired.h b/autowiring/Autowired.h index fbd411eff..fd7c1e22a 100644 --- a/autowiring/Autowired.h +++ b/autowiring/Autowired.h @@ -160,6 +160,8 @@ class Autowired: // The set of all nodes that will have to be unlinked when this field is torn down std::vector m_unlinkEntries; + //Required to keep the registration table alive untill after we're dead. + std::shared_ptr m_registrationTable; public: operator const std::shared_ptr&(void) const { @@ -210,6 +212,10 @@ class Autowired: auto ctxt = AutowirableSlot::GetContext(); if (!ctxt) throw std::runtime_error("Attempted to attach a signal to an Autowired field in a context that was already destroyed"); + + if (!m_registrationTable) + m_registrationTable = ctxt->template Inject(); + return {*this, ctxt->RelayForType(handlerActual)}; } From 356b434152881606ef171c7392d280e690fb658c Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 20 Apr 2015 09:22:33 -0700 Subject: [PATCH 61/63] Eliminate unneeded copy and assignment constructors Default behavior is OK in both of these cases--no need to carry around an unneeded duplicate --- autowiring/AutoFilterDescriptor.h | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/autowiring/AutoFilterDescriptor.h b/autowiring/AutoFilterDescriptor.h index ddfd2b67f..030ef5f58 100644 --- a/autowiring/AutoFilterDescriptor.h +++ b/autowiring/AutoFilterDescriptor.h @@ -25,14 +25,8 @@ struct AutoFilterDescriptorStub { m_pCall(nullptr) {} - AutoFilterDescriptorStub(const AutoFilterDescriptorStub& rhs) : - m_pType(rhs.m_pType), - m_pArgs(rhs.m_pArgs), - m_deferred(rhs.m_deferred), - m_arity(rhs.m_arity), - m_requiredCount(rhs.m_requiredCount), - m_pCall(rhs.m_pCall) - {} + AutoFilterDescriptorStub(const AutoFilterDescriptorStub&) = default; + AutoFilterDescriptorStub& operator=(const AutoFilterDescriptorStub&) = default; /// /// Constructs a new packet subscriber entry based on the specified call extractor and call pointer @@ -137,18 +131,8 @@ struct AutoFilterDescriptor: AutoFilterDescriptorStub { AutoFilterDescriptor(void) {} - - AutoFilterDescriptor(const AutoFilterDescriptor& rhs) : - AutoFilterDescriptorStub(rhs), - m_autoFilter(rhs.m_autoFilter) - {} - - AutoFilterDescriptor& operator=(const AutoFilterDescriptor& rhs) { - AutoFilterDescriptorStub::operator = (rhs); - m_autoFilter = rhs.m_autoFilter; - return *this; - } - + AutoFilterDescriptor(const AutoFilterDescriptor&) = default; + AutoFilterDescriptor& operator=(const AutoFilterDescriptor&) = default; AutoFilterDescriptor(AutoFilterDescriptor&& rhs) : AutoFilterDescriptorStub(std::move(rhs)), m_autoFilter(std::move(rhs.m_autoFilter)) From f9de2673001d3363b6baef49c6a821a0b21fff38 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 20 Apr 2015 06:05:42 -0700 Subject: [PATCH 62/63] Add an altitude concept to describe filter priorities Altitude is now declared on an AutoFilter as a static member of that class, as so: ```C++ class MyFilter { static const autowiring::altitude altitude = autowiring::altitude::Instrumentation; }; ``` The remainder of the AutoFilter declaration is unchanged. By default, AutoFilters marked `Deferred` will run at a higher altitude than other filters. Currently, altitudes can only be easily declared on class-level AutoFilters. --- autowiring/AutoFilterDescriptor.h | 30 +++++-- autowiring/AutoPacketFactory.h | 4 +- autowiring/altitude.h | 87 +++++++++++++++++++ src/autowiring/AutoPacket.cpp | 1 + src/autowiring/AutoPacketGraph.cpp | 1 + src/autowiring/CMakeLists.txt | 1 + .../test/AutoFilterAltitudeTest.cpp | 53 +++++++++++ src/autowiring/test/CMakeLists.txt | 1 + src/autowiring/test/ContextEnumeratorTest.cpp | 1 + 9 files changed, 169 insertions(+), 10 deletions(-) create mode 100644 autowiring/altitude.h create mode 100644 src/autowiring/test/AutoFilterAltitudeTest.cpp diff --git a/autowiring/AutoFilterDescriptor.h b/autowiring/AutoFilterDescriptor.h index 030ef5f58..d8d2ef16b 100644 --- a/autowiring/AutoFilterDescriptor.h +++ b/autowiring/AutoFilterDescriptor.h @@ -1,6 +1,7 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once #include "AnySharedPointer.h" +#include "altitude.h" #include "auto_arg.h" #include "AutoFilterDescriptorInput.h" #include "CallExtractor.h" @@ -18,6 +19,7 @@ class Deferred; struct AutoFilterDescriptorStub { AutoFilterDescriptorStub(void) : m_pType(nullptr), + m_altitude(autowiring::altitude::Standard), m_pArgs(nullptr), m_deferred(false), m_arity(0), @@ -40,8 +42,9 @@ struct AutoFilterDescriptorStub { /// is required to carry information about the type of the proper member function to be called; t_extractedCall is /// required to be instantiated by the caller and point to the AutoFilter proxy routine. /// - AutoFilterDescriptorStub(const std::type_info* pType, const AutoFilterDescriptorInput* pArgs, bool deferred, t_extractedCall pCall) : + AutoFilterDescriptorStub(const std::type_info* pType, autowiring::altitude altitude, const AutoFilterDescriptorInput* pArgs, bool deferred, t_extractedCall pCall) : m_pType(pType), + m_altitude(altitude), m_pArgs(pArgs), m_deferred(deferred), m_arity(0), @@ -61,6 +64,9 @@ struct AutoFilterDescriptorStub { // Type of the subscriber itself const std::type_info* m_pType; + // Altitude--controls when the filter gets called + autowiring::altitude m_altitude; + // This subscriber's argument types // NOTE: This is a reference to a static generated list, // therefore it MUST be const and MUST be shallow-copied. @@ -87,6 +93,7 @@ struct AutoFilterDescriptorStub { public: // Accessor methods: + autowiring::altitude GetAltitude(void) const { return m_altitude; } const std::type_info* GetType() const { return m_pType; } size_t GetArity(void) const { return m_arity; } size_t GetRequiredCount(void) const { return m_requiredCount; } @@ -162,6 +169,10 @@ struct AutoFilterDescriptor: std::static_pointer_cast::type>(subscriber) ), &typeid(T), + autowiring::altitude_of< + T, + CallExtractor::deferred ? autowiring::altitude::Dispatch : autowiring::altitude::Standard + >::value, Decompose::template Enumerate::types, CallExtractor::deferred, &CallExtractor::template Call<&T::AutoFilter> @@ -179,6 +190,7 @@ struct AutoFilterDescriptor: AutoFilterDescriptor( AnySharedPointer(std::make_shared(std::forward(fn))), &typeid(Fn), + autowiring::altitude::Standard, CallExtractor::template Enumerate::types, false, &CallExtractor::template Call<&Fn::operator()> @@ -203,13 +215,13 @@ struct AutoFilterDescriptor: /// /// The caller is responsible for decomposing the desired routine into the target AutoFilter call /// - AutoFilterDescriptor(const AnySharedPointer& autoFilter, const std::type_info* pType, const AutoFilterDescriptorInput* pArgs, bool deferred, t_extractedCall pCall) : - AutoFilterDescriptorStub(pType, pArgs, deferred, pCall), + AutoFilterDescriptor(const AnySharedPointer& autoFilter, const std::type_info* pType, autowiring::altitude altitude, const AutoFilterDescriptorInput* pArgs, bool deferred, t_extractedCall pCall) : + AutoFilterDescriptorStub(pType, altitude, pArgs, deferred, pCall), m_autoFilter(autoFilter) {} template - AutoFilterDescriptor(RetType(*pfn)(Args...)): + AutoFilterDescriptor(RetType(*pfn)(Args...), autowiring::altitude altitude = autowiring::altitude::Standard) : AutoFilterDescriptor( // Token shared pointer, used to provide a pointer to pfn because we can't // capture it in a template processing context. Hopefully this can be changed @@ -223,7 +235,7 @@ struct AutoFilterDescriptor: // The remainder is fairly straightforward &typeid(pfn), - + altitude, CallExtractor::template Enumerate::types, false, CallExtractor::Call @@ -262,9 +274,11 @@ struct AutoFilterDescriptor: /// Default for std library sorting of unique elements /// bool operator<(const AutoFilterDescriptor& rhs) const { - if (m_pCall < rhs.m_pCall) - return true; - return m_autoFilter < rhs.m_autoFilter; + // This filter is "logically prior" to the right-hand side if this filter has a HIGHER altitude + // than the one on the right-hand side + return + std::tie(m_altitude, m_pCall, m_autoFilter) < + std::tie(rhs.m_altitude, rhs.m_pCall, rhs.m_autoFilter); } /// diff --git a/autowiring/AutoPacketFactory.h b/autowiring/AutoPacketFactory.h index b9178ff18..459d35f41 100644 --- a/autowiring/AutoPacketFactory.h +++ b/autowiring/AutoPacketFactory.h @@ -7,7 +7,7 @@ #include "TypeRegistry.h" #include CHRONO_HEADER #include TYPE_TRAITS_HEADER -#include STL_UNORDERED_SET +#include class AutoPacketFactory; class DispatchQueue; @@ -41,7 +41,7 @@ class AutoPacketFactory: std::shared_ptr m_nextPacket; // Collection of known subscribers - typedef std::unordered_set> t_autoFilterSet; + typedef std::set t_autoFilterSet; t_autoFilterSet m_autoFilters; // Accumulators used to compute statistics about AutoPacket lifespan. diff --git a/autowiring/altitude.h b/autowiring/altitude.h new file mode 100644 index 000000000..9a53ac7d9 --- /dev/null +++ b/autowiring/altitude.h @@ -0,0 +1,87 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#pragma once + +namespace autowiring { + +/// +/// Defines the altitude enumeration concept for AutoFilter instances +/// +/// +/// A filter altitude is an indicator to the AutoFilter scheduler about when a particular filter +/// should be scheduled to receive control. Altitude is a hard requirement, but is subject to +/// a number of stipulations as to when it applies: +/// +/// 1) If two autofilters are both candidates to be run at the same time, the filter with the +/// higher altitude will be run first. +/// 2) If both filters have the same altitude, an arbitrary filter will be selected. +/// 3) When the current filter returns control (IE, its AutoFilter routine returns), the next +/// filter will be run. +/// 4) Deferred AutoFilters are a special case. A deferred AutoFilter is considered to have +/// returned control as soon as its execution has been scheduled; generally this happens very +/// fast. +/// 5) Altitudes only provide an order-of-execution guarantee if NO deferred AutoFilters have been +/// declared in the network. +/// +enum class altitude { + // Highest altitude level. Reserved for temporary debug logic and other nonpermanent code that + // must run before all other filter levels + Highest = 0x9000, + + // Instrumentation level, for use with instrumentation code. Instrumentation code often needs to + // observe the inputs to its AutoFilter before any other code has an opportunity to observe it, + // because this code needs information about + Instrumentation = 0x8000, + + // Default altitude for Deferred autofilters. Deferred autofilters are guaranteed to return very + // quickly, even though they may do a lot of work, because they do not tie up the main thread. + Dispatch = 0x7000, + + // Asynchronous filters are designed to be run with a higher priority than standard filters, + // but are still expected to complete very quickly. Because their speedy behavior is implemented + // by the filter, and not guaranteed by Autowiring, asynchronous filters are considered to + // have a lower priority than Deferred filters. + // + // It is expected that AutoFilters which are marked as Asynchronous will do the majority of their + // work in an std::async or other similar call. + Asynchronous = 0x6000, + + // The realtime altitude is a higher-than-normal altitude which may have some tight timing requirements + // but does not run in a separate thread. Realtime filters run after deferred filters have been + // scheduled to run. + Realtime = 0x5000, + + // Default altitude range. Unless otherwise specified, or the filter is marked Deferred, filters + // will normally execute at this priority level. + Standard = 0x4000, + + // Altitude indicator for filters with no hard timing requirements. This is a convenient place to put + // filters that may have extensive CPU usage requirements, or which are not strongly impacted by timing. + // Analytics and diagnostics are typically suitable for execution at the passive level. + Passive = 0x3000, + + // Lowest altitude level. Reserved for temporary debug logic and other nonpermanent code that + // must run after all other filter levels. + Lowest = 0x2000 +}; + +inline altitude operator+(altitude alt, int v) { + return (altitude) ((int) alt + v); +} + +/// +/// Extracts the altitude of type T, if declared, or infers it if not +/// +/// The outer type of the AutoFilter +/// The default to be used if one is not provied by T +template +struct altitude_of { + template + static std::integral_constant select(U*); + + template + static std::integral_constant select(...); + + static const altitude value = decltype(select(nullptr))::value; +}; + +} \ No newline at end of file diff --git a/src/autowiring/AutoPacket.cpp b/src/autowiring/AutoPacket.cpp index 18ce30c67..04a02733f 100644 --- a/src/autowiring/AutoPacket.cpp +++ b/src/autowiring/AutoPacket.cpp @@ -424,6 +424,7 @@ bool AutoPacket::Wait(std::condition_variable& cv, const AutoFilterDescriptorInp stub, AutoFilterDescriptorStub( &typeid(AutoPacketFactory), + autowiring::altitude::Dispatch, inputs, false, [] (const AnySharedPointer& obj, AutoPacket&) { diff --git a/src/autowiring/AutoPacketGraph.cpp b/src/autowiring/AutoPacketGraph.cpp index 087b7cd95..bfac398bd 100644 --- a/src/autowiring/AutoPacketGraph.cpp +++ b/src/autowiring/AutoPacketGraph.cpp @@ -10,6 +10,7 @@ #include #include #include FUNCTIONAL_HEADER +#include STL_UNORDERED_SET AutoPacketGraph::AutoPacketGraph() { } diff --git a/src/autowiring/CMakeLists.txt b/src/autowiring/CMakeLists.txt index 572b3e5b2..238472e92 100644 --- a/src/autowiring/CMakeLists.txt +++ b/src/autowiring/CMakeLists.txt @@ -14,6 +14,7 @@ set(Autowiring_SRCS AnySharedPointer.h atomic_object.h at_exit.h + altitude.h auto_id.h auto_future.h auto_signal.h diff --git a/src/autowiring/test/AutoFilterAltitudeTest.cpp b/src/autowiring/test/AutoFilterAltitudeTest.cpp new file mode 100644 index 000000000..a6504b527 --- /dev/null +++ b/src/autowiring/test/AutoFilterAltitudeTest.cpp @@ -0,0 +1,53 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include + +class AutoFilterAltitudeTest: + public testing::Test +{}; + +struct AltitudeValue {}; + +struct AltitudeMonotonicCounter { + std::atomic order{0}; +}; + +template +struct HasProfilingAltitude { + static const autowiring::altitude altitude = A; + + AutoRequired ctr; + int order = -1; + + void AutoFilter(const AltitudeValue& val) { + order = ++ctr->order; + } +}; + +TEST_F(AutoFilterAltitudeTest, AltitudeDetection) { + AutoFilterDescriptor desc(std::make_shared>()); + ASSERT_EQ(autowiring::altitude::Highest, desc.GetAltitude()) << "Filter altitude was not correctly inferred"; +} + +TEST_F(AutoFilterAltitudeTest, StandardAltitudeArrangement) { + AutoCurrentContext()->Initiate(); + + AutoRequired> alt3; + AutoRequired> alt1; + AutoRequired> alt2; + AutoRequired> alt4; + AutoRequired> alt5; + AutoRequired> alt0; + + AutoRequired factory; + auto packet = factory->NewPacket(); + packet->Decorate(AltitudeValue{}); + + // Now we verify things got invoked in the right order: + ASSERT_EQ(1, alt0->order); + ASSERT_EQ(2, alt1->order); + ASSERT_EQ(3, alt2->order); + ASSERT_EQ(4, alt3->order); + ASSERT_EQ(5, alt4->order); + ASSERT_EQ(6, alt5->order); +} \ No newline at end of file diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index c590d49b7..a3a970941 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -5,6 +5,7 @@ set(AutowiringTest_SRCS AutoConfigListingTest.cpp AutoConfigParserTest.cpp AutoConstructTest.cpp + AutoFilterAltitudeTest.cpp AutoFilterCollapseRulesTest.cpp AutoFilterDiagnosticsTest.cpp AutoFilterFunctionTest.cpp diff --git a/src/autowiring/test/ContextEnumeratorTest.cpp b/src/autowiring/test/ContextEnumeratorTest.cpp index 9cc7b5feb..12441e72e 100644 --- a/src/autowiring/test/ContextEnumeratorTest.cpp +++ b/src/autowiring/test/ContextEnumeratorTest.cpp @@ -3,6 +3,7 @@ #include #include #include MEMORY_HEADER +#include STL_UNORDERED_SET class ContextEnumeratorTest: public testing::Test From 570d3bc9e5169cc5b9a8213df5d1fabd58b6dd17 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Mon, 20 Apr 2015 10:31:59 -0700 Subject: [PATCH 63/63] Allow altitude to be easily specified when using lambda autofilters Altitudes can be specified using the comma notation, similar to how delays are specified for dispatchers: ```C++ AutoRequired factory; *factory += autowiring::altitude::Highest, [] (const MyDecoration&) {}; ``` --- autowiring/AutoFilterDescriptor.h | 4 +-- autowiring/AutoPacketFactory.h | 19 ++++++++++++ .../test/AutoFilterAltitudeTest.cpp | 29 ++++++++++++++++++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/autowiring/AutoFilterDescriptor.h b/autowiring/AutoFilterDescriptor.h index d8d2ef16b..d0e596242 100644 --- a/autowiring/AutoFilterDescriptor.h +++ b/autowiring/AutoFilterDescriptor.h @@ -186,11 +186,11 @@ struct AutoFilterDescriptor: /// Recipients added in this way cannot receive piped data, since they are anonymous. /// template - AutoFilterDescriptor(Fn fn): + AutoFilterDescriptor(Fn fn, autowiring::altitude altitude = autowiring::altitude::Standard): AutoFilterDescriptor( AnySharedPointer(std::make_shared(std::forward(fn))), &typeid(Fn), - autowiring::altitude::Standard, + altitude, CallExtractor::template Enumerate::types, false, &CallExtractor::template Call<&Fn::operator()> diff --git a/autowiring/AutoPacketFactory.h b/autowiring/AutoPacketFactory.h index 459d35f41..ed12d85c1 100644 --- a/autowiring/AutoPacketFactory.h +++ b/autowiring/AutoPacketFactory.h @@ -119,6 +119,25 @@ class AutoPacketFactory: /// void RemoveSubscriber(const AutoFilterDescriptor& autoFilter); + struct AutoPacketFactoryExpression { + AutoPacketFactoryExpression(AutoPacketFactory& factory, autowiring::altitude altitude): + factory(factory), + altitude(altitude) + {} + + AutoPacketFactory& factory; + autowiring::altitude altitude; + + template + AutoFilterDescriptor operator,(Fx&& fx) { + return factory.AddSubscriber(AutoFilterDescriptor(std::forward(fx), altitude)); + } + }; + + AutoPacketFactoryExpression operator+=(autowiring::altitude altitude) { + return AutoPacketFactoryExpression(*this, altitude); + } + /// /// Convienance overload of operator+= to add a subscriber from a lambda /// diff --git a/src/autowiring/test/AutoFilterAltitudeTest.cpp b/src/autowiring/test/AutoFilterAltitudeTest.cpp index a6504b527..255ac0a9f 100644 --- a/src/autowiring/test/AutoFilterAltitudeTest.cpp +++ b/src/autowiring/test/AutoFilterAltitudeTest.cpp @@ -50,4 +50,31 @@ TEST_F(AutoFilterAltitudeTest, StandardAltitudeArrangement) { ASSERT_EQ(4, alt3->order); ASSERT_EQ(5, alt4->order); ASSERT_EQ(6, alt5->order); -} \ No newline at end of file +} + +TEST_F(AutoFilterAltitudeTest, LambdaAltitudesOnFactory) { + AutoCurrentContext()->Initiate(); + AutoRequired factory; + + int seq = 0; + int ctr[9]; + + for (size_t i = 0; i < 9; i++) { + ctr[i] = -1; + *factory += (autowiring::altitude)i, [&, i] { + ctr[i] = ++seq; + }; + } + + // Generate a packet and trip the assignments: + auto packet = factory->NewPacket(); + ASSERT_EQ(9, ctr[0]); + ASSERT_EQ(8, ctr[1]); + ASSERT_EQ(7, ctr[2]); + ASSERT_EQ(6, ctr[3]); + ASSERT_EQ(5, ctr[4]); + ASSERT_EQ(4, ctr[5]); + ASSERT_EQ(3, ctr[6]); + ASSERT_EQ(2, ctr[7]); + ASSERT_EQ(1, ctr[8]); +}