Skip to content

Commit

Permalink
Fix issue with namespace captured variables as rvalues
Browse files Browse the repository at this point in the history
  • Loading branch information
kunitoki committed Sep 15, 2023
1 parent 79775a8 commit feca450
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 28 deletions.
63 changes: 35 additions & 28 deletions Source/LuaBridge/detail/Namespace.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,21 @@ class Registrar
{
}

Registrar(const Registrar& rhs)
Registrar(const Registrar& rhs) = delete;

Registrar(Registrar&& rhs)
: L(rhs.L)
, m_stackSize(std::exchange(rhs.m_stackSize, 0))
, m_skipStackPops(std::exchange(rhs.m_skipStackPops, 0))
{
}

Registrar& operator=(const Registrar& rhs)
Registrar& operator=(const Registrar& rhs) = delete;

Registrar& operator=(Registrar&& rhs)
{
m_stackSize = rhs.m_stackSize;
m_skipStackPops = rhs.m_skipStackPops;
m_stackSize = std::exchange(rhs.m_stackSize, 0);
m_skipStackPops = std::exchange(rhs.m_skipStackPops, 0);

return *this;
}
Expand All @@ -87,8 +91,8 @@ class Registrar
}

lua_State* const L = nullptr;
int mutable m_stackSize = 0;
int mutable m_skipStackPops = 0;
int m_stackSize = 0;
int m_skipStackPops = 0;
};

} // namespace detail
Expand Down Expand Up @@ -146,8 +150,8 @@ class Namespace : public detail::Registrar
class ClassBase : public detail::Registrar
{
public:
explicit ClassBase(Namespace& parent)
: Registrar(parent)
explicit ClassBase(Namespace parent)
: Registrar(std::move(parent))
{
}

Expand Down Expand Up @@ -290,8 +294,8 @@ class Namespace : public detail::Registrar
* @param parent A parent namespace object.
* @param options Class options.
*/
Class(const char* name, Namespace& parent, Options options)
: ClassBase(parent)
Class(const char* name, Namespace parent, Options options)
: ClassBase(std::move(parent))
{
LUABRIDGE_ASSERT(name != nullptr);
LUABRIDGE_ASSERT(lua_istable(L, -1)); // Stack: namespace table (ns)
Expand Down Expand Up @@ -366,8 +370,8 @@ class Namespace : public detail::Registrar
* @param parent A parent namespace object.
* @param staticKey Key where the class is stored.
*/
Class(const char* name, Namespace& parent, const void* const staticKey, Options options)
: ClassBase(parent)
Class(const char* name, Namespace parent, const void* const staticKey, Options options)
: ClassBase(std::move(parent))
{
LUABRIDGE_ASSERT(name != nullptr);
LUABRIDGE_ASSERT(lua_istable(L, -1)); // Stack: namespace table (ns)
Expand Down Expand Up @@ -443,7 +447,8 @@ class Namespace : public detail::Registrar

m_stackSize -= 3;
lua_pop(L, 3);
return Namespace(*this);

return Namespace(std::move(*this));
}

//=========================================================================================
Expand Down Expand Up @@ -1400,8 +1405,8 @@ class Namespace : public detail::Registrar
class Table : public detail::Registrar
{
public:
explicit Table(const char* name, Namespace& parent)
: Registrar(parent)
explicit Table(const char* name, Namespace parent)
: Registrar(std::move(parent))
{
lua_newtable(L); // Stack: ns, table (tb)
lua_pushvalue(L, -1); // Stack: ns, tb, tb
Expand Down Expand Up @@ -1452,7 +1457,8 @@ class Namespace : public detail::Registrar

m_stackSize -= 2;
lua_pop(L, 2);
return Namespace(*this);

return Namespace(std::move(*this));
}
};

Expand Down Expand Up @@ -1521,8 +1527,8 @@ class Namespace : public detail::Registrar
*
* @pre The parent namespace is at the top of the Lua stack.
*/
Namespace(const char* name, Namespace& parent, Options options)
: Registrar(parent)
Namespace(const char* name, Namespace parent, Options options)
: Registrar(std::move(parent))
{
LUABRIDGE_ASSERT(name != nullptr);
LUABRIDGE_ASSERT(lua_istable(L, -1)); // Stack: parent namespace (pns)
Expand Down Expand Up @@ -1573,13 +1579,13 @@ class Namespace : public detail::Registrar
*
* @param child A child class registration object.
*/
explicit Namespace(ClassBase& child)
: Registrar(child)
explicit Namespace(ClassBase child)
: Registrar(std::move(child))
{
}

explicit Namespace(Table& child)
: Registrar(child)
explicit Namespace(Table child)
: Registrar(std::move(child))
{
}

Expand Down Expand Up @@ -1627,7 +1633,7 @@ class Namespace : public detail::Registrar
Namespace beginNamespace(const char* name, Options options = defaultOptions)
{
assertIsActive();
return Namespace(name, *this, options);
return Namespace(name, std::move(*this), options);
}

//=============================================================================================
Expand All @@ -1644,13 +1650,14 @@ class Namespace : public detail::Registrar
{
throw_or_assert<std::logic_error>("endNamespace() called on global namespace");

return Namespace(*this);
return Namespace(std::move(*this));
}

LUABRIDGE_ASSERT(m_stackSize > 1);
--m_stackSize;
lua_pop(L, 1);
return Namespace(*this);

return Namespace(std::move(*this));
}

//=============================================================================================
Expand Down Expand Up @@ -2004,7 +2011,7 @@ class Namespace : public detail::Registrar
Table beginTable(const char* name)
{
assertIsActive();
return Table(name, *this);
return Table(name, std::move(*this));
}

//=============================================================================================
Expand All @@ -2020,7 +2027,7 @@ class Namespace : public detail::Registrar
Class<T> beginClass(const char* name, Options options = defaultOptions)
{
assertIsActive();
return Class<T>(name, *this, options);
return Class<T>(name, std::move(*this), options);
}

//=============================================================================================
Expand All @@ -2038,7 +2045,7 @@ class Namespace : public detail::Registrar
Class<Derived> deriveClass(const char* name, Options options = defaultOptions)
{
assertIsActive();
return Class<Derived>(name, *this, detail::getStaticRegistryKey<Base>(), options);
return Class<Derived>(name, std::move(*this), detail::getStaticRegistryKey<Base>(), options);
}
};

Expand Down
15 changes: 15 additions & 0 deletions Tests/Source/NamespaceTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,21 @@ TEST_F(NamespaceTests, IndexAccessByNonStringShouldNotCrash)
EXPECT_TRUE(result().isNil());
}

TEST_F(NamespaceTests, NamespaceAsRValueReferenceShouldWork)
{
auto ns = luabridge::getGlobalNamespace(L);

ns = ns.beginNamespace("test");

auto cls = ns.beginClass<SystemDestroyer>("SystemDestroyer");
ns = cls.endClass();

ns.endNamespace();

runLua("result = test[SystemDestroyer]");
EXPECT_TRUE(result().isNil());
}

#ifdef _M_IX86 // Windows 32bit only

namespace {
Expand Down

0 comments on commit feca450

Please sign in to comment.