Simplify LuaParameterDictionary. (#132)

cartographer/common/lua_parameter_dictionary.cc

@@ -147,43 +147,19 @@ void GetArrayValues(lua_State* L, const std::function<void()>& pop_value) {
 
 std::unique_ptr<LuaParameterDictionary>
 LuaParameterDictionary::NonReferenceCounted(
-    const string& code, std::unique_ptr<FileResolver> file_resolver,
+    const string& code, std::unique_ptr<FileResolver> file_resolver) {
-    StateExtensionFunction state_extension_function) {
   return std::unique_ptr<LuaParameterDictionary>(new LuaParameterDictionary(
-      code, ReferenceCount::NO, std::move(file_resolver),
+      code, ReferenceCount::NO, std::move(file_resolver)));
-      state_extension_function));
-}
-
-std::unique_ptr<LuaParameterDictionary> LuaParameterDictionary::Partial(
-    const string& code, const string& key,
-    std::unique_ptr<FileResolver> file_resolver,
-    StateExtensionFunction state_extension_function) {
-  auto parameter_dictionary =
-      std::unique_ptr<LuaParameterDictionary>(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;
 }
 
 LuaParameterDictionary::LuaParameterDictionary(
-    const string& code, std::unique_ptr<FileResolver> file_resolver,
+    const string& code, std::unique_ptr<FileResolver> file_resolver)
-    StateExtensionFunction state_extension_function)
     : LuaParameterDictionary(code, ReferenceCount::YES,
-                             std::move(file_resolver),
+                             std::move(file_resolver)) {}
-                             state_extension_function) {}
 
 LuaParameterDictionary::LuaParameterDictionary(
     const string& code, ReferenceCount reference_count,
-    std::unique_ptr<FileResolver> file_resolver,
+    std::unique_ptr<FileResolver> file_resolver)
-    StateExtensionFunction state_extension_function)
     : 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));

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<void(lua_State*)>;
-
 // 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<FileResolver> file_resolver,
+                         std::unique_ptr<FileResolver> file_resolver);
-                         StateExtensionFunction state_extension_functon);
 
   LuaParameterDictionary(const LuaParameterDictionary&) = delete;
   LuaParameterDictionary& operator=(const LuaParameterDictionary&) = delete;
 
   // Constructs a LuaParameterDictionary without reference counting.
   static std::unique_ptr<LuaParameterDictionary> NonReferenceCounted(
-      const string& code, std::unique_ptr<FileResolver> file_resolver,
+      const string& code, std::unique_ptr<FileResolver> 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<LuaParameterDictionary> Partial(
-      const string& code, const string& key,
-      std::unique_ptr<FileResolver> file_resolver,
-      StateExtensionFunction state_extension_functon);
 
   ~LuaParameterDictionary();
 
@@ -96,8 +82,7 @@ class LuaParameterDictionary {
  private:
   enum class ReferenceCount { YES, NO };
   LuaParameterDictionary(const string& code, ReferenceCount reference_count,
-                         std::unique_ptr<FileResolver> file_resolver,
+                         std::unique_ptr<FileResolver> file_resolver);
-                         StateExtensionFunction state_extension_function);
 
   // For GetDictionary().
   LuaParameterDictionary(lua_State* L, ReferenceCount reference_count,

cartographer/common/lua_parameter_dictionary_test.cc

@@ -31,15 +31,7 @@ namespace {
 std::unique_ptr<LuaParameterDictionary> MakeNonReferenceCounted(
     const string& code) {
   return LuaParameterDictionary::NonReferenceCounted(
-      code, common::make_unique<DummyFileResolver>(),
+      code, common::make_unique<DummyFileResolver>());
-      nullptr /* state_extension_function */);
-}
-
-std::unique_ptr<LuaParameterDictionary> MakePartial(const string& code,
-                                                    const string& key) {
-  return LuaParameterDictionary::Partial(
-      code, key, common::make_unique<DummyFileResolver>(),
-      nullptr /* state_extension_function */);
 }
 
 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

cartographer/common/lua_parameter_dictionary_test_helpers.h

@@ -48,8 +48,7 @@ class DummyFileResolver : public FileResolver {
 
 std::unique_ptr<LuaParameterDictionary> MakeDictionary(const string& code) {
   return common::make_unique<LuaParameterDictionary>(
-      code, std::unique_ptr<DummyFileResolver>(new DummyFileResolver()),
+      code, common::make_unique<DummyFileResolver>());
-      nullptr /* state_extension_function */);
 }
 
 }  // namespace common

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<PointsBatch> 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;
       }