From 4522fc49ad56081e89ac635a5022837990306fd5 Mon Sep 17 00:00:00 2001 From: Holger Rapp Date: Thu, 17 Nov 2016 17:58:51 +0100 Subject: [PATCH] Simplify LuaParameterDictionary. (#132) --- .../common/lua_parameter_dictionary.cc | 37 +++---------------- .../common/lua_parameter_dictionary.h | 21 ++--------- .../common/lua_parameter_dictionary_test.cc | 24 +----------- .../lua_parameter_dictionary_test_helpers.h | 3 +- cartographer/io/xray_points_processor.cc | 4 +- 5 files changed, 12 insertions(+), 77 deletions(-) diff --git a/cartographer/common/lua_parameter_dictionary.cc b/cartographer/common/lua_parameter_dictionary.cc index 6fca85a..4db29dd 100644 --- a/cartographer/common/lua_parameter_dictionary.cc +++ b/cartographer/common/lua_parameter_dictionary.cc @@ -147,43 +147,19 @@ void GetArrayValues(lua_State* L, const std::function& pop_value) { std::unique_ptr LuaParameterDictionary::NonReferenceCounted( - const string& code, std::unique_ptr file_resolver, - StateExtensionFunction state_extension_function) { + const string& code, std::unique_ptr file_resolver) { return std::unique_ptr(new LuaParameterDictionary( - code, ReferenceCount::NO, std::move(file_resolver), - state_extension_function)); -} - -std::unique_ptr LuaParameterDictionary::Partial( - const string& code, const string& key, - std::unique_ptr file_resolver, - StateExtensionFunction state_extension_function) { - auto parameter_dictionary = - std::unique_ptr(new LuaParameterDictionary( - code, std::move(file_resolver), state_extension_function)); - // This replaces the table at the top of the stack with the table at 'key'. - auto& L = parameter_dictionary->L_; - - const string lua_code = "local table=...; return table." + key; - CheckForLuaErrors(L, luaL_loadstring(L, lua_code.c_str())); - lua_pushvalue(L, -2); // S: table, function, table - lua_remove(L, -3); // S: function, table - CheckForLuaErrors(L, lua_pcall(L, 1, 1, 0)); - CheckTableIsAtTopOfStack(L); - return parameter_dictionary; + code, ReferenceCount::NO, std::move(file_resolver))); } LuaParameterDictionary::LuaParameterDictionary( - const string& code, std::unique_ptr file_resolver, - StateExtensionFunction state_extension_function) + const string& code, std::unique_ptr file_resolver) : LuaParameterDictionary(code, ReferenceCount::YES, - std::move(file_resolver), - state_extension_function) {} + std::move(file_resolver)) {} LuaParameterDictionary::LuaParameterDictionary( const string& code, ReferenceCount reference_count, - std::unique_ptr file_resolver, - StateExtensionFunction state_extension_function) + std::unique_ptr file_resolver) : L_(luaL_newstate()), index_into_reference_table_(-1), file_resolver_(std::move(file_resolver)), @@ -196,9 +172,6 @@ LuaParameterDictionary::LuaParameterDictionary( lua_register(L_, "choose", LuaChoose); lua_register(L_, "include", LuaInclude); lua_register(L_, "read", LuaRead); - if (state_extension_function) { - state_extension_function(L_); - } CheckForLuaErrors(L_, luaL_loadstring(L_, code.c_str())); CheckForLuaErrors(L_, lua_pcall(L_, 0, 1, 0)); diff --git a/cartographer/common/lua_parameter_dictionary.h b/cartographer/common/lua_parameter_dictionary.h index 207b0be..871a687 100644 --- a/cartographer/common/lua_parameter_dictionary.h +++ b/cartographer/common/lua_parameter_dictionary.h @@ -38,33 +38,19 @@ class FileResolver { virtual string GetFileContentOrDie(const string& basename) = 0; }; -// A function that adds new Lua functions to a state. Used to extend the Lua -// configuration in custom contexts. -using StateExtensionFunction = std::function; - // A parameter dictionary that gets loaded from Lua code. class LuaParameterDictionary { public: // Constructs the dictionary from a Lua Table specification. LuaParameterDictionary(const string& code, - std::unique_ptr file_resolver, - StateExtensionFunction state_extension_functon); + std::unique_ptr file_resolver); LuaParameterDictionary(const LuaParameterDictionary&) = delete; LuaParameterDictionary& operator=(const LuaParameterDictionary&) = delete; // Constructs a LuaParameterDictionary without reference counting. static std::unique_ptr NonReferenceCounted( - const string& code, std::unique_ptr file_resolver, - StateExtensionFunction state_extension_functon); - - // Constructs a partial LuaParameterDictionary by extracting the dictionary - // with 'key' from 'code' where 'key' refers to an arbitrarily deep dictionary - // (e.g. "a.b.c"). - static std::unique_ptr Partial( - const string& code, const string& key, - std::unique_ptr file_resolver, - StateExtensionFunction state_extension_functon); + const string& code, std::unique_ptr file_resolver); ~LuaParameterDictionary(); @@ -96,8 +82,7 @@ class LuaParameterDictionary { private: enum class ReferenceCount { YES, NO }; LuaParameterDictionary(const string& code, ReferenceCount reference_count, - std::unique_ptr file_resolver, - StateExtensionFunction state_extension_function); + std::unique_ptr file_resolver); // For GetDictionary(). LuaParameterDictionary(lua_State* L, ReferenceCount reference_count, diff --git a/cartographer/common/lua_parameter_dictionary_test.cc b/cartographer/common/lua_parameter_dictionary_test.cc index b7d686c..7d1493c 100644 --- a/cartographer/common/lua_parameter_dictionary_test.cc +++ b/cartographer/common/lua_parameter_dictionary_test.cc @@ -31,15 +31,7 @@ namespace { std::unique_ptr MakeNonReferenceCounted( const string& code) { return LuaParameterDictionary::NonReferenceCounted( - code, common::make_unique(), - nullptr /* state_extension_function */); -} - -std::unique_ptr MakePartial(const string& code, - const string& key) { - return LuaParameterDictionary::Partial( - code, key, common::make_unique(), - nullptr /* state_extension_function */); + code, common::make_unique()); } class LuaParameterDictionaryTest : public ::testing::Test { @@ -226,20 +218,6 @@ TEST_F(LuaParameterDictionaryTest, TestChooseInvalidArgument) { "condition is not a boolean value."); } -TEST_F(LuaParameterDictionaryTest, Partial) { - auto partial_dictionary = - MakePartial("return { blah = { blue = { red = 200 } } }", "blah.blue"); - EXPECT_EQ(200, partial_dictionary->GetInt("red")); -} - -TEST_F(LuaParameterDictionaryTest, PartialIsReferenceCounted) { - auto partial_dictionary = - MakePartial("return { blah = { blue = { red = 200 } } }", "blah.blue"); - ASSERT_DEATH(partial_dictionary.reset(), - ".*Key 'red' was used the wrong number of times..*"); - partial_dictionary->GetInt("red"); -} - } // namespace } // namespace common } // namespace cartographer diff --git a/cartographer/common/lua_parameter_dictionary_test_helpers.h b/cartographer/common/lua_parameter_dictionary_test_helpers.h index 6d269a8..278b139 100644 --- a/cartographer/common/lua_parameter_dictionary_test_helpers.h +++ b/cartographer/common/lua_parameter_dictionary_test_helpers.h @@ -48,8 +48,7 @@ class DummyFileResolver : public FileResolver { std::unique_ptr MakeDictionary(const string& code) { return common::make_unique( - code, std::unique_ptr(new DummyFileResolver()), - nullptr /* state_extension_function */); + code, common::make_unique()); } } // namespace common diff --git a/cartographer/io/xray_points_processor.cc b/cartographer/io/xray_points_processor.cc index d05c895..59f833b 100644 --- a/cartographer/io/xray_points_processor.cc +++ b/cartographer/io/xray_points_processor.cc @@ -143,7 +143,7 @@ XRayPointsProcessor::XRayPointsProcessor( floors_(floors), output_filename_(output_filename), transform_(transform) { - for (int i = 0; i < (floors_.empty() ? 1 : floors.size()); ++i) { + for (size_t i = 0; i < (floors_.empty() ? 1 : floors.size()); ++i) { voxels_.emplace_back(voxel_size, Eigen::Vector3f::Zero()); } } @@ -169,7 +169,7 @@ void XRayPointsProcessor::Process(std::unique_ptr batch) { CHECK_EQ(voxels_.size(), 1); Insert(*batch, transform_, &voxels_[0]); } else { - for (int i = 0; i < floors_.size(); ++i) { + for (size_t i = 0; i < floors_.size(); ++i) { if (!ContainedIn(batch->time, floors_[i].timespans)) { continue; }